From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
---|---|
To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | John Naylor <john(dot)naylor(at)enterprisedb(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, John Naylor <jcnaylor(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: optimize lookups in snapshot [sub]xip arrays |
Date: | 2022-08-08 23:07:16 |
Message-ID: | 20220808230716.GB1393216@nathanxps13 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Aug 08, 2022 at 12:56:28PM +0530, Bharath Rupireddy wrote:
> 1) pg_lfind32 - why just uint32? If it's not possible to define
> functions for char, unsigned char, int16, uint16, int32, int64, uint64
> and so on, can we add a few comments around that? Also, the comments
> can talk about if the base type or element data type of array or data
> type of key matters to use pg_lfind32.
I figured that we'd add functions for other types when needed. I
considered making the new function generic by adding an argument for the
element size. Then, we could branch to optimized routines based on the
element size (e.g., pg_lfind() would call pg_lfind32() if the element size
was 4 bytes). However, that seemed like more complexity than is required,
and it's probably nice to avoid the extra branching.
> 2) I think this is not just for the remaining elements but also for
> non-USE_SSE2 cases. Also, please specify in which cases we reach here
> for USE_SSE2 cases.
> + /* Process the remaining elements the slow way. */
Well, in the non-SSE2 case, all of the elements are remaining at this
point. :)
> 3) Can pg_lfind32 return the index of the key found, for instance to
> use it for setting/resetting the found element in the array?
As discussed upthread, only returning whether the element is present in the
array is slightly faster. If we ever needed a version that returned the
address of the matching element, we could reevaluate whether this small
boost was worth creating a separate function or if we should just modify
pg_lfind32() to be a tad slower. I don't think we need to address that
now, though.
> 4) Can we, right away, use this API to replace linear search, say, in
> SimpleLruReadPage_ReadOnly(), ATExecAttachPartitionIdx(),
> AfterTriggerSetState()? I'm sure I might be missing other places, but
> can we replace the possible found areas with the new function?
I had found a few eligible linear searches earlier [0], but I haven't done
any performance analysis that proved such changes were worthwhile. While
substituting linear searches with pg_lfind32() is probably an improvement
in most cases, I think we ought to demonstrate the benefits for each one.
[0] https://postgr.es/m/20220802221301.GA742739%40nathanxps13
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2022-08-08 23:13:53 | Re: conchuela doesn't like gnu_printf anymore |
Previous Message | Thomas Munro | 2022-08-08 22:44:55 | Re: Checking pgwin32_is_junction() errors |