Simpler ASCII printable length #16
Conversation
Don't need the SWAR. Overall, both mixed and ASCII-only benchmarks are much faster. Surprising! Presumably because the new function does not require an 8-byte boundaries and therefore applies to more texts.
There was a problem hiding this comment.
Pull request overview
This PR simplifies the ASCII optimization by replacing the SWAR (SIMD Within A Register) approach with simple loop-based iteration, removing the unsafe package dependency. The new implementation is faster for ASCII text, simpler to understand, and works on any length of printable ASCII (not just 8+ bytes).
Changes:
- Replaced SWAR-based
printableASCIILengthandprintableASCIILengthBytesfunctions with a single generic function using simple loop iteration - Removed
unsafepackage import and all unsafe pointer operations - Updated
String()andBytes()methods to use the simplified optimization that no longer requires 8+ bytes to apply - Added comprehensive test cases for control characters, alternating ASCII/non-ASCII sequences, and edge cases
- Updated benchmark results showing significant performance improvements (3.3x faster for ASCII, 1.2x faster for mixed content)
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| width.go | Simplified printableASCIILength to use generic simple loop instead of SWAR; removed unsafe import; unified string and byte slice handling; updated comments |
| width_test.go | Added new test cases for control characters and alternating sequences; updated test expectations to match new behavior (returns exact count instead of -1 for short strings); updated function calls to use unified generic function |
| README.md | Updated benchmark results; removed section about internal implementation details; minor formatting improvements |
| comparison/README.md | Updated benchmark results to reflect performance improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Previous ASCII optimization #14 used SWAR. This one simplifies to just loops, no
unsafe. Appears to be quite a bit faster — 10x over the previous tagged release, and 3x over SWAR — for short ASCII strings anyway. For very long strings (say hundreds of contiguous ASCII bytes), SWAR might pull ahead but I prefer the simplicity of this approach.This approach does not require 8 bytes of ASCII to apply, as SWAR does. It now applies to any run of ASCII — so it will do better on short or mixed text, more often.