[benasher44]

pg_query_protobuf_to_nodes unpacks its input with protobuf-c's default allocator, which mallocs once per message and frees via protobuf_c_message_free_unpacked — a walk over every field of every message's descriptor looking for pointers. The Node descriptor has 271 fields and every value in a parse tree is a Node, so ~80% of pg_query_deparse_protobuf is inside protobuf-c: ~45% of samples in free_unpacked, ~34% in unpack. The deparse walk itself is 6–8%.

The function has one caller, pg_query_deparse_protobuf_opts, which always runs it inside a pg_query memory context — the one _readRawStmt already pallocs the Node tree into. So this passes a ProtobufCAllocator backed by palloc and drops the free call. MemoryContextDelete reclaims the structs with everything else.

Measured on darwin-arm64, clang -O2, 20k iterations of pg_query_deparse_protobuf over a pre-parsed tree:

statement	before	after
SELECT a, b FROM t WHERE c = 1	5.93 µs	2.90 µs	2.0×
join + correlated subquery + GROUP BY	18.32 µs	8.37 µs	2.2×
100-column projection	116.98 µs	53.45 µs	2.2×
INSERT … ON CONFLICT DO UPDATE	6.58 µs	3.14 µs	2.1×

Peak memory during the call goes up. The unpacked structs stay live through the deparse instead of being freed before it, and protobuf-c's per-message scratch (a 34-byte required-fields bitmap for Node) is retained instead of freed inside unpack. On a 3 MB parse tree, peak RSS for the call goes from +25.4 MB to +38.7 MB, released when the call returns as before.

This also fixes a leak: an elog(ERROR) from _readNode used to longjmp past free_unpacked. The structs are now context-owned, so the existing PG_CATCH reclaims them.

[lfittl]

@benasher44 Thanks for the contribution, worth evaluating!

My current plan is to move to ubp (see #349), which does its own arena allocations (still with malloc though), and is a better maintained Protobuf parsing library that also has better handling for deeply nested structs (fixing some OSS-fuzz reported issues).

Would you be interested in running a benchmark comparison between this PR and that PR? Would be good to know if palloc buys us anything if we switch to the ubp arena allocator.

[benasher44]

Posted the benchmark in #349!

[lfittl]

@benasher44 I just merged the upb support, so I believe this PR is no longer needed - I'll go ahead and close it. In case you notice anything amiss with the upb changes (e.g. with the way the depth limit works now), please let us know in a separate issue. Thanks for your testing!