Re: [PATCH] Halve peak memory allocation for toast_flatten_tuple

Hi, Johannes!

> Peak memory allocated inside toast_flatten_tuple():
>
> cols plain stored master patched cut
> ---- -------- --------- ---------- ---------- ------
> 1 96 MB 28 MB 57 MB 57 MB 0.0%
> 2 96 MB 28 MB 57 MB 43 MB 25.0%
> 4 96 MB 28 MB 57 MB 36 MB 37.5%
> 8 96 MB 28 MB 57 MB 32 MB 43.7%
> 8 896 MB 265 MB 530 MB 298 MB 43.7%
>
> That is your claim confirmed, to three decimals: master is exactly 2x the
> tuple in all five cases, and v1 is exactly 1x plus the one detoasted value
> it is holding.
>
> Whole-backend peak RSS:
>
> cols plain stored master patched cut
> ---- -------- --------- ---------- ---------- ------
> 1 96 MB 28 MB 89 MB 88 MB 0.4%
> 2 96 MB 28 MB 88 MB 83 MB 6.4%
> 4 96 MB 28 MB 89 MB 75 MB 15.0%
> 8 96 MB 28 MB 88 MB 72 MB 18.8%
> 8 896 MB 265 MB 803 MB 580 MB 27.8%
>
> One small thing on the code, since I was reading it anyway.
>
> toast_flatten_tuple() now open-codes what heap_form_tuple() does: the
> hasnull scan, len and hoff, heap_compute_data_size(), the palloc0 and the
> HeapTupleHeaderSet* calls. It has to, because it needs the allocation and
> the fill separated.
>
> You already split fill_val() and heap_fill_tuple() into _attr variants with
> thin wrappers. The same move one level up would avoid the duplication
> in toast_flatten_tuple():
> heap_form_tuple() becomes a wrapper around heap_form_tuple_attr(), which
> takes skip_copy and attr_data_out. toast_flatten_tuple() could then call
> that and drop the block entirely.
>
> Worth it for the single source of truth, I think.

Thank you very much for your independent verification!

I also split heap_form_tuple_attr encapsulation to avoid code
duplication, as you suggested. See attached patch v2.

Kind regards,
Pavel Borisov
Supabase