Repository navigation
perf(topk): sort the extract with a radix sort - #379
pavanmanishd wants to merge 1 commit into
Conversation
The capped extract quickselects the rerank pool to 3x the cap and then sorts that prefix with qsort, one function-pointer comparison per step. A four-pass LSD radix sort over the float distances replaces it: stable, exact, and several times faster, taking roughly 40% off the posting phase at LIMIT 5000, where the prefix is about 95k entries. It uses a grow-only scratch buffer from the top-K's memory context. The key transform canonicalizes -0.0 to +0.0 so equal distances keep their arrival order instead of ranking one below the other, and sorts NaN last. One expected output moves: the extension suite pins a tie at LIMIT 3 on default_items, where the order is now deterministic but, after the capped path's quickselect, not necessarily arrival order. The figure above comes from a build with timescale#369 applied, which this branch does not contain: timescale#369 is approved and pending merge, and the two changes are independent, so this needs either the same base or a re-measurement before it lands.
|
@pavanmanishd I am not going to accept this change as is. Why?
|
|
@erimatnor I disagree that these are random changes. Each targets a hotspot I found by profiling, and I measured each one before opening it. They came out of the same investigation as #369 and #373, both of which you merged. At LIMIT 5000 on a 2M-row, 128-dim table, You're right that the argument belongs in an issue before review, so I've opened #384 with the full profile and before/after table, linked this PR and #380 to it, and moved both back to draft. If you think large-LIMIT performance isn't worth this code, I'll close them. If you'd rather judge them with your regression framework, I'll re-run them on it when it lands. |
3 participants
Refs #384.
Replaces the comparator sort in the extract with a four-pass LSD radix sort over the float distances. The capped extract quickselects the rerank pool to 3x the cap and sorts that prefix — about 95k entries at LIMIT 5000 — where
qsortpays a function-pointer comparison per step; the radix sort is stable and exact and takes roughly 40% off the posting phase. A grow-only scratch buffer from the top-K's memory context holds the ping-pong.Equal distances now come out deterministic, and in arrival order except after the capped path's quickselect, which is not stable;
-0.0is keyed as+0.0so the two do not rank one below the other, and NaN sorts last. One expected output moves: the extension suite pins a tie at LIMIT 3, where the order changes but stays deterministic.The figures come from a build with #369 applied, which has since merged; #384 has the measurements.