-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
src: use validate_ascii_with_errors for buffer.isAscii #61122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
It has better performance
1f3f083 to
e78cdd1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #61122 +/- ##
==========================================
- Coverage 88.53% 88.52% -0.02%
==========================================
Files 703 703
Lines 208546 208547 +1
Branches 40217 40217
==========================================
- Hits 184634 184607 -27
- Misses 15926 15949 +23
- Partials 7986 7991 +5
🚀 New features to boost your workflow:
|
lemire
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the ping.
gurgunday
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Tracking: #61041
Don't expect
buffer.isAsciicommon-case to be ASCII, as that's exposed as a public API for checks.The error version has better performance on non-ascii as it returns early, which could easily be e.g. >10x.
Refs: #46271 (comment),
But somehow the
_errorsversion is also faster on ASCII, which was supposed to be a common case forvalidate_ascii?I'm not sure what's up, might be a simdutf bug? cc @lemire
On another note: v8 is also likely using
validate_asciiincorrectlybuffer.isAsciiperf:main:
PR