perf(gzip): index BytesView directly in crc32_update - #388
Closed
mizchi wants to merge 1 commit into
Closed
Conversation
crc32_update iterates the input via 'for byte in chunk' where chunk is a BytesView. That desugars through BytesView::iter + Iter::next, which allocates an iterator closure and pays a virtual dispatch per byte. A gzip_roundtrip callgrind profile attributes ~16% of total instructions to that single loop (BytesView::iter 9.80% + Iter::next 6.07%), even though the loop body is a couple of arithmetic ops and a table lookup. Pull out the backing Bytes + start offset + length once and index the raw Bytes directly. Bytes[i] is intrinsic and inlined. gzip_roundtrip bench (native, 3-run median): baseline: 178 ms patched : 162 ms (-9.0%)
Collaborator
|
For builtin, array-like data structure, |
Collaborator
|
It seems that |
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
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 proposal is superseded by #475, merged on July 3, 2026. That change replaced the byte-at-a-time CRC32 loop with a slicing-by-eight implementation in 007b3ab.
The original May proposal replaced iterator-based byte traversal with direct indexing. Current main already processes eight bytes per iteration and indexes the remaining tail directly, so porting the old loop would replace the newer algorithm. The original benchmark numbers no longer describe the implementation on main.
The current native release gzip and gzip-internal suites pass 39/39 with moonc
v0.10.12+1634b282e(2026-09-07):moon test src/internal/gzip_internal src/gzip --target native --releaseClosing this obsolete proposal in favor of the implementation already merged in #475.