Resolution - #17
Merged
Merged
Conversation
frankmcsherry
added a commit
that referenced
this pull request
Sep 2, 2026
Bumps the corgi pin from #16 to #18 (`de0f2ac`), which also picks up #17. Four API changes, all of which delete code at the call site: * `corgi::hash` returns the ids (`Vec<u64>`) rather than a `Value` that every caller here unwrapped in the same expression. Four sites lose `.into_u64(..).unwrap()`. * `arrange::hash_rows` is gone — it was a second, disagreeing implementation of the same fold (it mixed leaf width into its seed, so it called a `u8 5` and a `u64 5` different values, which the canonical hash does not). The two round-trip test calls become `corgi::hash`. * `Bounds::Offsets` holds an `Arc`, so a `Value` clone no longer copies a list's partition. Construction goes through `Bounds::offsets`, and the three places that hand-rolled a `Vec<usize>` of end offsets by matching both variants now call `Bounds::to_vec`, which is public and does it. * `Value::Sum` carries one `Tags` — the lane assignment — instead of a tag column and an offset column, with a two-word `Const` form for the one-tag case. `signed_order_view` moves the assignment through untouched; `untranscode` reads `tag_at`/`offset_at` per row. And one adoption beyond compiling: `sort_consolidate` was building the two `i`/`i+1` index columns to ask for an adjacent comparison. `compare_adjacent` names the pattern instead, so the index columns are not built and corgi reads both sides densely. That is on the chunk-merge path. Claude-Session: https://claude.ai/code/session_01PPtSwqXTuH4wefbQd2F8pA Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces a fairly substantial shift to high-resolution collection streams. Collections are now streams of triples
(data, time, diff)where thetimeis what is new. Rather than have each(data, diff)inherit thetimeof the containing message, they can (and must) each have their own time, which must be greater or equal to that of the containing message.This introduces non-trivial complexity in
arrangeandgroup, though with some further optimization the performance should get closer to what it was for batch computation. What is good is that one can now run updates each at distinct times at substantially higher throughputs, and with performance that increases with additional workers.To recover performance for less high-resolution streams, I anticipate some specialized code and implementations. But, I like this version that makes sure that the general case works without falling over, and can recover performance in specialized cases with effort.
One outstanding issue is that
grouphas a running time which is quadratic in the number of distinct times per key. This isn't new, in that if you put as many updates at distinct times into old versions you would have an even worse problem, but you are now encouraged to do this sort of thing and the perf needs to get shaken out. I expect that to improve soon, but I didn't want it to block this version.