-
Notifications
You must be signed in to change notification settings - Fork 166
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
[Minor] Refactor parse
Function
#110
Conversation
…t of the expected base32 character set into its own function
No objection, but I'll observe that the existing code will short-circuit in the |
Oh yep. It looks like they're a bit different, added the benchstat output below.
Benchmarking master ParseStrict
Bechmarks after the refactor
|
Am I reading it correctly that this change is 22% more CPU and 18% less throughput i.e. strictly worse? |
Yeah, looks like this refactor made it less performant. |
Refactor
parse
function if all the characters in a base32 encoded ULID are part of the expected base32 character set into its own function.Since the if condition checks if one of the entries is
0xFF
and then returns true forErrInvalidCharacters
I refactored the break into a for loop that exits true early if the entry is0xFF