Skip to content

Commit 2576dcf

Browse files
committed
Revert refactoring of hex code to src/common/
This is a combined revert of the following commits: - c3826f8, a refactoring piece that moved the hex decoding code to src/common/. This code was cleaned up by aef8948, as it originally included no overflow checks in the same way as the base64 routines in src/common/ used by SCRAM, making it unsafe for its purpose. - aef8948, a more advanced refactoring of the hex encoding/decoding code to src/common/ that added sanity checks on the result buffer for hex decoding and encoding. As reported by Hans Buschmann, those overflow checks are expensive, and it is possible to see a performance drop in the decoding/encoding of bytea or LOs the longer they are. Simple SQLs working on large bytea values show a clear difference in perf profile. - ccf4e27, a cleanup made possible by aef8948. The reverts of all those commits bring back the performance of hex decoding and encoding back to what it was in ~13. Fow now and post-beta3, this is the simplest option. Reported-by: Hans Buschmann Discussion: https://postgr.es/m/1629039545467.80333@nidsa.net Backpatch-through: 14
1 parent 2313dda commit 2576dcf

File tree

9 files changed

+127
-304
lines changed

9 files changed

+127
-304
lines changed

src/backend/replication/backup_manifest.c

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@
1313
#include "postgres.h"
1414

1515
#include "access/timeline.h"
16-
#include "common/hex.h"
1716
#include "libpq/libpq.h"
1817
#include "libpq/pqformat.h"
1918
#include "mb/pg_wchar.h"
2019
#include "replication/backup_manifest.h"
20+
#include "utils/builtins.h"
2121
#include "utils/json.h"
2222

2323
static void AppendStringToManifest(backup_manifest_info *manifest, char *s);
@@ -150,12 +150,10 @@ AddFileToBackupManifest(backup_manifest_info *manifest, const char *spcoid,
150150
}
151151
else
152152
{
153-
uint64 dstlen = pg_hex_enc_len(pathlen);
154-
155153
appendStringInfoString(&buf, "{ \"Encoded-Path\": \"");
156-
enlargeStringInfo(&buf, dstlen);
157-
buf.len += pg_hex_encode(pathname, pathlen,
158-
&buf.data[buf.len], dstlen);
154+
enlargeStringInfo(&buf, 2 * pathlen);
155+
buf.len += hex_encode(pathname, pathlen,
156+
&buf.data[buf.len]);
159157
appendStringInfoString(&buf, "\", ");
160158
}
161159

@@ -178,7 +176,6 @@ AddFileToBackupManifest(backup_manifest_info *manifest, const char *spcoid,
178176
{
179177
uint8 checksumbuf[PG_CHECKSUM_MAX_LENGTH];
180178
int checksumlen;
181-
uint64 dstlen;
182179

183180
checksumlen = pg_checksum_final(checksum_ctx, checksumbuf);
184181
if (checksumlen < 0)
@@ -188,10 +185,9 @@ AddFileToBackupManifest(backup_manifest_info *manifest, const char *spcoid,
188185
appendStringInfo(&buf,
189186
", \"Checksum-Algorithm\": \"%s\", \"Checksum\": \"",
190187
pg_checksum_type_name(checksum_ctx->type));
191-
dstlen = pg_hex_enc_len(checksumlen);
192-
enlargeStringInfo(&buf, dstlen);
193-
buf.len += pg_hex_encode((char *) checksumbuf, checksumlen,
194-
&buf.data[buf.len], dstlen);
188+
enlargeStringInfo(&buf, 2 * checksumlen);
189+
buf.len += hex_encode((char *) checksumbuf, checksumlen,
190+
&buf.data[buf.len]);
195191
appendStringInfoChar(&buf, '"');
196192
}
197193

@@ -311,9 +307,8 @@ SendBackupManifest(backup_manifest_info *manifest)
311307
{
312308
StringInfoData protobuf;
313309
uint8 checksumbuf[PG_SHA256_DIGEST_LENGTH];
314-
char *checksumstringbuf;
310+
char checksumstringbuf[PG_SHA256_DIGEST_STRING_LENGTH];
315311
size_t manifest_bytes_done = 0;
316-
uint64 dstlen;
317312

318313
if (!IsManifestEnabled(manifest))
319314
return;
@@ -334,11 +329,10 @@ SendBackupManifest(backup_manifest_info *manifest)
334329
sizeof(checksumbuf)) < 0)
335330
elog(ERROR, "failed to finalize checksum of backup manifest");
336331
AppendStringToManifest(manifest, "\"Manifest-Checksum\": \"");
337-
dstlen = pg_hex_enc_len(sizeof(checksumbuf));
338-
checksumstringbuf = palloc0(dstlen + 1); /* includes \0 */
339-
pg_hex_encode((char *) checksumbuf, sizeof(checksumbuf),
340-
checksumstringbuf, dstlen);
341-
checksumstringbuf[dstlen] = '\0';
332+
333+
hex_encode((char *) checksumbuf, sizeof checksumbuf, checksumstringbuf);
334+
checksumstringbuf[PG_SHA256_DIGEST_STRING_LENGTH - 1] = '\0';
335+
342336
AppendStringToManifest(manifest, checksumstringbuf);
343337
AppendStringToManifest(manifest, "\"}\n");
344338

0 commit comments

Comments
 (0)