On Fri, Jan 20, 2023 at 11:02 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> Yeah. I think the analysis looks good, but I'll do some testing next
> week with the aim of getting it committed. Looks like it now needs
> Meson changes, but I'll look after that as my penance.
Here's an updated version that I'm testing...
Changes to the main patch:
* Adjust a few comments
* pgindent
* Explained a bit more in the commit message
I'm wondering about this bit in rebin_segment():
+ if (segment_map->header == NULL)
+ return;
Why would we be rebinning an uninitialised/unused segment? Does
something in your DSA-client code (I guess you have an extension?) hit
this case? The tests certainly don't; I'm not sure how the case could
be reached.
Changes to the test:
* Update copyright year
* Size -> size_t
* pgindent
* Add Meson glue
* Re-alphabetise the makefile
* Make sure we get BGWH_STOPPED while waiting for bgworkers to exit
* Background worker main function return type is fixed (void)
* results[1] -> results[FLEXIBLE_ARRAY_MEMBER]
* getpid() -> MyProcPid
I wonder if this code would be easier to understand, but not
materially less efficient, if we re-binned eagerly when allocating
too, so the bin is always correct/optimal. Checking fpm_largest()
again after allocating should be cheap, I guess (it just reads a
member variable that we already paid the cost of maintaining). We
don't really seem to amortise much, we just transfer the rebinning
work to the next caller to consider the segment. I haven't tried out
that theory though.