-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
[mypyc] Add efficient librt.base64.b64decode #20263
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
Conversation
This comment has been minimized.
This comment has been minimized.
mypyc/lib-rt/librt_base64.c
Outdated
| return ((c >= 'A' && c <= 'Z') | (c >= 'a' && c <= 'z') | | ||
| (c >= '0' && c <= '9') | (c == '+') | (c == '/') | (allow_padding && c == '=')); |
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.
is there a reason to mix logical && and bitwise |?
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.
No particularly good reason, mainly to highlight that these don't need branches at runtime (we don't want many mispredicted branches). I think this was from ChatGPT output and I thought it was okay in this use case. The semantics are identical so it's probably better to use && consistently though.
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.
I did some experiments, and using || might help compilers generate faster code, so I will use it.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
The performance can be 10x faster than stdlib if input is valid base64, or if input has extra non-base64 characters only at the end of input. Similar to the base64 encode implementation I added recently, this uses SIMD instructions when available.
The implementation first tries to decode the input optimistically assuming valid base64. If this fails, we'll perform a slow path with a preprocessing step that removes extra characters, and we'll perform a strict base64 decode on the cleaned up input.
The semantics aren't 100% compatible with stdlib. First, we raise ValueError on invalid padding instead of
binascii.Error, since I don't want a runtime dependency on the unrelated abinasciimodule. This needs to be documented, but stdlib can already raise ValueError on other conditions, so the deviation is not huge. Also, some invalid inputs are checked more strictly for padding violations. The stdlib implementation has some mysterious behaviors with invalid inputs that didn't seem worth replicating.The function only accepts a single ASCII str or bytes argument for now, since that seems to be by the far the most common use case. The stdlib function also accepts buffer objects and a
validateargument.The slow path is still somewhat faster than stdlib (on the order of 1.3x to 2x for longer inputs), at least if the input is much smaller than L1 cache size.
Got the initial fast path implementation from ChatGPT, but did a bunch of manual edits afterwards and reviewed carefully.