Hi,
On 03/12/2025 03:08, Peter Geoghegan wrote:
Coming back to this patch now, after several months of work on index
prefetching.
Nice, welcome back! It would be great to wrap this up.
Tomas Vondra suggested that I keep killedItems as a separate
allocation (as it is on master), while using a Bitmapset to represent
killedItems (unlike on master, where it is represented using a simple
array). This has all of the same advantages as my previous approach,
but doesn't have the aforementioned disadvantages within _bt_killitems
(plus we no longer need to change BTScanPosItem in any way).
Good one!
- * Don't bother advancing the outermost loop's int iterator to
- * avoid processing killed items that relate to the same
- * offnum/posting list tuple. This micro-optimization hardly
- * seems worth it. (Further iterations of the outermost loop
- * will fail to match on this same posting list's first heap
- * TID instead, so we'll advance to the next offnum/index
- * tuple pretty quickly.)
+ * Don't advance itemIndex for outermost loop, no matter how
+ * nextIndex was advanced. It's possible that items whose
+ * TIDs weren't matched in posting list can still be killed
+ * (there might be a later tuple whose TID is a match).
Hmm, as far as I can tell, the old version of the comment seems more
accurate. If I understand correctly, it's still safe to do the
micro-optimization, but we choose to not do it because we expect the
speedup will not be worth the increased complexity / risk of introducing
bugs.
--
Thanks,
Mircea Cadariu
