LantaoJin commented on PR #16794: URL: https://github.com/apache/lucene/pull/16794#issuecomment-6094611388
Thanks @mayya-sharipova for the review! The four inline comments are addressed, and I measured the barrier question. **Are the barriers what limits the speedup?** Not much, it turns out. I logged the builder’s own InfoStream “effective concurrency” (cumulative worker time / wall time of `build()`) during the luceneutil force merge: cohere-v3 1024d, 1M docs, M 16, beam width 100, 8 merge workers, every version force-merging a copy of the same multi-segment index (float32: 8 segments, 7-bit: 4 segments). I also prototyped two variants with fewer barriers: - `overlap`: graph g’s non-join-set nodes and graph g+1’s join set run in one phase, so G+1 barriers instead of 2G. - `perGraph`: your suggestion, one task per joined graph (largest first), each on its own worker, so no barrier between graphs. | variant | float32 force merge | effective concurrency (of 8) | 7-bit force merge | effective concurrency (of 8) | |---|---|---|---|---| | main (full rebuild of the non-base graphs) | 192.2 s | 7.92x | 228.5 s | | | this PR | 143.9 s | 7.75x | 193.6 s | 7.59x | | overlap | 142.6 s | 7.84x | 192.0 s | 7.70x | | perGraph | 276.0 s | 3.57x | 650.5 s | 1.27x | (The 7-bit number for main comes from an earlier run on the same input.) - The barriers cost about 2% of worker time for float32 and 4% for 7-bit. `overlap` gets back about half of that, around 1% of force merge time, which doesn't seem worth the extra code to me. - `perGraph` is 1.9x (float32) and 3.4x (7-bit) slower than this PR, because of load imbalance: with 7 joined graphs of different sizes, or only 3, most of the 8 workers have nothing to do for most of the merge. - Recall/visited is the same for all three variants, within 0.003 at every fanout from 0 to 400. What bounds the speedup is the work saved, rather than synchronization. For float32, cumulative worker time drops from 1328.6 s to 925.8 s (1.44x), graph build wall time from 167.7 s to 119.5 s (1.40x), and about 24 s of each force merge is spent outside the graph build in both versions, which brings the end-to-end gain to 1.34x. **So I'd keep the current design**. Open to switch to `overlap` if you think the extra 1% is worth it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
