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
