Skip to content

JAVA-5789 Implements batch ByteBuf::indexOf #1630

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

Closed
wants to merge 2 commits into from

Conversation

franz1981
Copy link

Fixes JAVA-5789

@franz1981
Copy link
Author

Similarly to #1629 this have some large performance impact to existing users 🙏

@vbabanin
Copy link
Member

vbabanin commented May 9, 2025

Hi @franz1981,

As replied on the associated ticket JAVA-5789, thank you again for your contribution and for taking the time to explore performance improvements.

I really liked your suggestion and the direction it suggested. However, we ended up implementing an alternative SWAR-based solution as part of JAVA-5842. This approach resulted in approximately ~10% better performance according to our benchmarks. For reference, our benchmarks are based on the framework described in the benchmarking specification and implemented in the driver-benchmarks module.

Given this, I’ll go ahead and close this PR as superseded by the implementation in JAVA-5842.

@vbabanin vbabanin closed this May 9, 2025
@franz1981
Copy link
Author

franz1981 commented May 10, 2025

Hi @vbabanin

I've taken a quick look at the SWAR approach you used there and it is sounds - but for the Netty use case I still suggest to leverage what is offered by ByteBuf::indexOf since within Netty I could have used bound and accessibility check free (and without position change too!) getLong - which directly become as simple as a "mov" in x86.
In term of absolute performance increase, IDK if the original benchmark exercise branch misses and if the new SWAR implementation is as effective as the one I mentioned in Netty - which, accordingly to my past measurements, deliver a good 30% boost alone in Http 1.1 decoding (it is used for both line and headed name/values parsing).

Said that, many thanks for the consideration and keep the good work ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants