-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[JAVA-4816] perf: improving performance of ObjectId parseHexString() #1034
[JAVA-4816] perf: improving performance of ObjectId parseHexString() #1034
Conversation
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.
Hi @eonwhite thanks for the contribution. Can you do a couple of things to move this forward:
- Open an issue in the JAVA project of MongoDB's Jira, and then reference the issue number in the PR description (see the CONTRIBUTING guide for details). Please attach your performance tests as well.
- Run
./gradlew bson:check
to make sure all static and runtime checks are passing
Thanks @jyemin -- have resolved issues and verified that tests pass. |
Hi @eonwhite I pushed a commit with a few small improvements to your PR. PTAL and then I can merge it. |
Thanks again for the contribution @eonwhite |
JAVA-4816 Co-authored-by: Jeff Yemin <jeff.yemin@mongodb.com>
JIRA Ticket: https://jira.mongodb.org/browse/JAVA-4816
The current implementation of
ObjectId
parseHexString
is somewhat slow, because it makes a total of 12 calls toInteger.parseInt
, which is not the fastest to begin with. This imposes some overhead to allObjectId(string)
calls, which can really add up at scale, for example when deserializing a large amount of ObjectId data via Jackson.Replacing this implementation with a hand-rolled version which converts each part of the hex string to a byte. This in my rough testing is about 5x faster than the current implementation of
parseHexString
.Tests (on my M1 MacBook Pro):
On master:
On branch: