Aleksander Alekseev <a.alekseev@postgrespro.ru> writes:
> Oops, wrong patches - here are correct ones.
This is certainly not doing what I had in mind for communication
between ResourceOwnerGetAny and ResourceOwnerRelease, because the
latter pays zero attention to "lastidx", but instead does a blind
hash search regardless.
In general, I'm suspicious of the impact of this patch on "normal"
cases with not-very-large resource arrays. It might be hard to
measure that because in such cases resowner.c is not a bottleneck;
but that doesn't mean I want to give up performance in cases that
perform well today. You could probably set up a test harness that
exercises ResourceOwnerAdd/Release/etc in isolation and get good
timings for them that way, to confirm or disprove whether there's
a performance loss for small arrays.
I still suspect it would be advantageous to have two different operating
modes depending on the size of an individual resource array, and not
bother with hashing until you got to more than a couple dozen entries.
Given that you're rehashing anyway when you enlarge the array, this
wouldn't cost anything except a few more lines of code, ISTM --- and
you already have a net code savings because of the refactoring.
Also, there are defined ways to convert between Datum format and
other formats (DatumGetPointer() etc). This code isn't using 'em.
regards, tom lane
