-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Implement bytes.startswith in mypyc #20387
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
72f9ae5 to
2064d9e
Compare
e8c65fe to
1185d2f
Compare
for more information, see https://pre-commit.ci
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
mypyc/lib-rt/bytes_ops.c
Outdated
| const char *self_buf = PyBytes_AS_STRING(self); | ||
| const char *subobj_buf = PyBytes_AS_STRING(subobj); | ||
|
|
||
| if (subobj_len == 0) { |
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.
maybe this if check should go above the 2 PyBytes_AS_STRING lines? We can exit without those calls if the check returns true
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.
Good call, updated. I split the checks around each PyBytes_GET_SIZE call a bit further to optimize for the empty-arg case. Probably won't save a ton but I don't think it makes it unreadable
| # Test empty cases | ||
| assert test.startswith(b'') | ||
| assert b''.startswith(b'') | ||
| assert not b''.startswith(test) |
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.
Test with bytearray 1) as the receiver object and 2) the argument. This way we will also test the slow path.
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.
Added a few checks to cover those as well
Implements
bytes.startswithin mypy. Potentially could be more efficient without relying onmemcmpbut not sure.Tested with the following benchmark code, which shows a ~6.3x performance improvement compared to standard Python:
Output: