Scan _isAscii through a byteArrayViewVarHandle instead of Unsafe.getLong - #15
Merged
Merged
Conversation
Since JDK 24 (JEP 498) every sun.misc.Unsafe memory-access call performs an acquire-load, taxing the 8-byte scan that string materialization runs on every >=8 char string. The VarHandle view compiles to plain unfenced loads, and it is one less use of an API on the removal track. No effect on the existing benchmarks - their only materialized string is 3 chars, which never enters the 8-byte loop.
There was a problem hiding this comment.
Pull request overview
This PR updates LightProto’s string materialization fast path to avoid sun.misc.Unsafe.getLong fences on JDK 24+ by switching ASCII scanning in _isAscii to use a MethodHandles.byteArrayViewVarHandle(long[].class, nativeOrder()) for plain (unfenced) 8-byte loads, and removes the now-unused MH_GET_LONG MethodHandle plumbing.
Changes:
- Replace the
_isAscii8-byte scan implementation fromUnsafe.getLong(viaMH_GET_LONG) to aVarHandlebyte[]→long view using plainget. - Remove
MH_GET_LONGdeclarations/initialization now that it’s unused. - Simplify
_isAsciiby removing the MethodHandleinvokeExacttry/catch wrapper that was only needed forThrowable.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
String materialization (
readString) decides between the zero-copy LATIN1 fast path and full UTF-8 decoding by scanning the copied bytes with_isAscii, 8 bytes at a time viasun.misc.Unsafe.getLong. Since JDK 24 (JEP 498) everysun.misc.Unsafememory-access call executesbeforeMemoryAccess()with an acquire-load, so the scan pays a fence per 8 bytes on every materialized string of 8+ chars — and those calls are also on the eventual-removal track.Changes
_isAsciinow scans through aMethodHandles.byteArrayViewVarHandle(long[].class, nativeOrder())view: plain unfenced 8-byte loads on a supported API. The0x8080808080808080mask test is byte-order independent. The now-unusedMH_GET_LONGplumbing is removed.Results
No effect on the existing benchmarks — the only string they materialize is 3 chars, which never enters the 8-byte loop. On a temporary benchmark materializing a 57-char topic-style name (
persistent://my-tenant/my-namespace/my-topic-partition-42, parse +getName(), 3 forks per side, JDK 26):Unsafe.getLongscanLonger strings benefit proportionally more. This is also one step of the planned migration off
sun.misc.Unsafememory-access methods before their removal.286 tests green.