Re: type cache cleanup improvements

On Tue, Oct 22, 2024 at 6:10 PM Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> wrote:
> On Tue, 22 Oct 2024 at 11:34, Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
>>
>> On Mon, Oct 21, 2024 at 2:30 PM Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
>> >
>> > On Mon, Oct 21, 2024 at 1:16 PM Dagfinn Ilmari Mannsåker
>> > <ilmari(at)ilmari(dot)org> wrote:
>> > > Alexander Korotkov <aekorotkov(at)gmail(dot)com> writes:
>> > >
>> > > > On Mon, Oct 21, 2024 at 8:40 AM Andrei Lepikhov <lepihov(at)gmail(dot)com> wrote:
>> > > >>
>> > > >> On 21/10/2024 06:32, Dagfinn Ilmari Mannsåker wrote:
>> > > >> > Alexander Korotkov <aekorotkov(at)gmail(dot)com> writes:
>> > > >> >
>> > > >> >> +static Oid *in_progress_list;
>> > > >> >> +static int in_progress_list_len;
>> > > >> >> +static int in_progress_list_maxlen;
>> > > >> >
>> > > >> > Is there any particular reason not to use pg_list.h for this?
>> > > >> Sure. The type cache lookup has to be as much optimal as possible.
>> > > >> Using an array and relating sequential access to it, we avoid memory
>> > > >> allocations and deallocations 99.9% of the time. Also, quick access to
>> > > >> the single element (which we will have in real life almost all of the
>> > > >> time) is much faster than employing list machinery.
>> > >
>> > > Lists are actually dynamically resized arrays these days (see commit
>> > > 1cff1b95ab6ddae32faa3efe0d95a820dbfdc164), not linked lists, so
>> > > accessing arbitrary elements is O(1), not O(n). Just like this patch,
>> > > the size is doubled (starting at 16) whenever array is full.
>> > >
>> > > > +1,
>> > > > List with zero elements has to be NIL. That means continuous
>> > > > allocations/deallocations.
>> > >
>> > > This however is a valid point (unless we keep a dummy zeroth element to
>> > > avoid it, which is even uglier than open-coding the array extension
>> > > logic), so objection withdrawn.
>> >
>> > OK, thank you!
>> >
>> > The attached revision fixes EXTRA_INSTALL in
>> > src/test/modules/typcache/Makefile. Spotted off-list by Arthur
>> > Zakirov.
>>
>> I've re-checked that regression tests pass with
>> -DCLOBBER_CACHE_ALWAYS. Also did some grammar corrections for
>> comments and commit message. I'm going to push this if no objections.
>
> Thank you for working on this patch!
> Looked through the patchset once more.
>
> Patch 0001 (minor): "in the last" -> "after everything else" or "after other TypeCacheEntry contents"
>
> Patch 0002 looks ready to me.

Thank you, Pavel! 0001 revised according to your suggestion.

------
Regards,
Alexander Korotkov
Supabase