Masks for ascii lookups #11
Closed
Conversation
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
A tiny little bit faster, but smaller in memory
There was a problem hiding this comment.
Pull request overview
This PR optimizes ASCII character lookup tables by replacing two 256-byte arrays with a compact bitmask representation using four uint64 values (32 bytes total), achieving a 16× memory reduction. The optimization maintains the same functionality through bit manipulation operations instead of direct array indexing.
Key Changes
- Replaced
asciiWidthsandasciiPropertieslookup arrays with a singleasciiWidthsbitmask array of 4 uint64s - Implemented
asciiWidth()andasciiProperty()functions that use bit shifting and masking to extract values from the bitmask - Updated all call sites in
width.goto use the new functions instead of array lookups - Added comprehensive test coverage for the new bitmask-based lookup functions
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tables.go | Replaces lookup arrays with compact bitmask representation and implements bit manipulation functions for value extraction |
| tables_test.go | Adds comprehensive test coverage for the new bitmask functions, including edge cases and enum invariant validation |
| width.go | Updates all call sites to use new asciiWidth() and asciiProperty() functions; minor variable extraction refactoring |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
Author
|
Too complicated, simplified in #12 . |
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.
asciiWidthswas a 256-byte (int8) array for fast lookups. This PR uses a more compact representation, 256 bits as 4uint64’s. In turn, theasciiWidthfunc masks and shifts to extract the same values as the array lookup.asciiPropertyfunc does the same, but the inverse, because the_Defaultenum happens to be 0 and_Zero_Widthhappens to be 1. TheasciiPropertieslookup array was eliminated, leaving only theasciiWidthsintegers.So, 2 × 256 bytes → 32 bytes of table storage, a 16× reduction. A bit more cache-friendly, one hopes.
Performance looks about the same, maybe faster, but likely within any margin of error.