> On Feb 2, 2026, at 21:09, Tatsuya Kawata <[email protected]> wrote: > > Hi, > > Thank you for the detailed explanation! Your explanation helped me understand > the design much better. > I hope my understanding is now on the right track. > > I tested v3 both approaches: > > 1. Ilia's proposal with corrected increment and <= condition: > if (was_count1 && j <= firstcount1) > firstcount1++; > > 2. The original patch with while loop: > while (use_hash && firstcount1 < track_cnt && > track[firstcount1].count > 1) > firstcount1++; > > I verified the following cases and both approaches produced correct > track array values after the loop completed: > > Case 1: c1_cursor == match_index > c1_cursor points to a singleton, that singleton is matched again, > bubble-up occurs, then a new value arrives triggering eviction. > > Case 2: c1_cursor < match_index > c1_cursor is in the earlier part of the singleton region, > and a singleton further back is matched. > > Case 3: c1_cursor > match_index > c1_cursor has advanced past match_index due to previous evictions, > and an earlier singleton is matched. > > Both approaches seem to work correctly. The code reduction from 1 is minimal, > so either approach should be fine. > I believe the while loop exists to handle potential edge cases, > though in typical scenarios firstcount1 would only increment once per match > (since one singleton is promoted at a time). > > Overall, the patch looks good to me.
Hi Tatsuya, Thank you for the detailed testing and for validating those c1_cursor/match_index cases. I agree with your conclusion that both variants behave correctly, and that the code reduction from the single-step approach is small. On firstcount1: in the typical case it should advance by one when a singleton is promoted. I kept the loop-style adjustment mainly as an invariant repair step in hash mode — after bubble-up, it simply advances firstcount1 until it again points to the first singleton (or track_cnt). That makes the update less dependent on subtle index relationships and is a bit more robust against potential corner cases (or future tweaks to the reordering), while still being cheap since firstcount1 only moves forward and is bounded by track_cnt/track_max. That said, if other community members would prefer the simpler one-step update for readability, I’m happy to switch. -- Best regards, Chengpeng Yan
