Skip to content

VarHandle string encoder #7701

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

VarHandle string encoder #7701

wants to merge 3 commits into from

Conversation

trask
Copy link
Member

@trask trask commented Sep 23, 2025 •
edited
Loading

Note that unlike the UnsafeStringEncoder, the VarHandleStringEncoder requires --add-opens=java.base/java.lang=ALL-UNNAMED since the VarHandles need access to String internals.

Generally users won't do this and so won't get the VarHandle implementation, but the Java agent is able to automatically open these modules (see ModuleOpener.java in that repository).

Java 17 Results

StringMarshalBenchmark Results

PR Branch Results

Benchmark Method Configuration Score Units
marshalAsciiStringStateful FallbackStringEncoder, stringSize=512 0.093 ±0.002 us/op
marshalAsciiStringStateful UnsafeStringEncoder, stringSize=512 0.093 ±0.001 us/op
marshalAsciiStringStateful VarHandleStringEncoder, stringSize=512 0.094 ±0.001 us/op
marshalAsciiStringStateless FallbackStringEncoder, stringSize=512 1.269 ±0.005 us/op
marshalAsciiStringStateless UnsafeStringEncoder, stringSize=512 0.127 ±0.002 us/op
marshalAsciiStringStateless VarHandleStringEncoder, stringSize=512 0.215 ±0.001 us/op
marshalLatin1StringStateful FallbackStringEncoder, stringSize=512 0.805 ±0.006 us/op
marshalLatin1StringStateful UnsafeStringEncoder, stringSize=512 0.801 ±0.009 us/op
marshalLatin1StringStateful VarHandleStringEncoder, stringSize=512 1.737 ±1.025 us/op
marshalLatin1StringStateless FallbackStringEncoder, stringSize=512 2.456 ±0.024 us/op
marshalLatin1StringStateless UnsafeStringEncoder, stringSize=512 2.080 ±0.011 us/op
marshalLatin1StringStateless VarHandleStringEncoder, stringSize=512 1.890 ±0.007 us/op
marshalUnicodeStringStateful FallbackStringEncoder, stringSize=512 1.395 ±0.006 us/op
marshalUnicodeStringStateful UnsafeStringEncoder, stringSize=512 1.381 ±0.008 us/op
marshalUnicodeStringStateful VarHandleStringEncoder, stringSize=512 1.390 ±0.004 us/op
marshalUnicodeStringStateless FallbackStringEncoder, stringSize=512 2.755 ±0.003 us/op
marshalUnicodeStringStateless UnsafeStringEncoder, stringSize=512 2.942 ±0.009 us/op
marshalUnicodeStringStateless VarHandleStringEncoder, stringSize=512 2.848 ±0.011 us/op
marshalAsciiStringStateful FallbackStringEncoder, stringSize=512 0.095 ±0.000 us/op
marshalAsciiStringStateful UnsafeStringEncoder, stringSize=512 0.095 ±0.000 us/op
marshalAsciiStringStateful VarHandleStringEncoder, stringSize=512 0.095 ±0.001 us/op
marshalAsciiStringStateless FallbackStringEncoder, stringSize=512 1.287 ±0.005 us/op
marshalAsciiStringStateless UnsafeStringEncoder, stringSize=512 0.127 ±0.002 us/op
marshalAsciiStringStateless VarHandleStringEncoder, stringSize=512 0.213 ±0.002 us/op
marshalLatin1StringStateful FallbackStringEncoder, stringSize=512 0.801 ±0.010 us/op
marshalLatin1StringStateful UnsafeStringEncoder, stringSize=512 0.800 ±0.011 us/op
marshalLatin1StringStateful VarHandleStringEncoder, stringSize=512 0.799 ±0.012 us/op
marshalLatin1StringStateless FallbackStringEncoder, stringSize=512 2.452 ±0.026 us/op
marshalLatin1StringStateless UnsafeStringEncoder, stringSize=512 2.079 ±0.009 us/op
marshalLatin1StringStateless VarHandleStringEncoder, stringSize=512 1.974 ±0.007 us/op
marshalUnicodeStringStateful FallbackStringEncoder, stringSize=512 1.393 ±0.007 us/op
marshalUnicodeStringStateful UnsafeStringEncoder, stringSize=512 1.386 ±0.006 us/op
marshalUnicodeStringStateful VarHandleStringEncoder, stringSize=512 1.384 ±0.008 us/op
marshalUnicodeStringStateless FallbackStringEncoder, stringSize=512 2.757 ±0.006 us/op
marshalUnicodeStringStateless UnsafeStringEncoder, stringSize=512 2.934 ±0.007 us/op
marshalUnicodeStringStateless VarHandleStringEncoder, stringSize=512 2.847 ±0.005 us/op

Main Branch Results

Benchmark Method Configuration Score Units
marshalAsciiStringStateful stringSize=512 0.094 ±0.001 us/op
marshalAsciiStringStatelessSafe stringSize=512 1.280 ±0.009 us/op
marshalAsciiStringStatelessUnsafe stringSize=512 0.125 ±0.002 us/op
marshalLatin1StringStateful stringSize=512 0.803 ±0.012 us/op
marshalLatin1StringStatelessSafe stringSize=512 2.494 ±0.045 us/op
marshalLatin1StringStatelessUnsafe stringSize=512 2.242 ±0.008 us/op
marshalUnicodeStringStateful stringSize=512 1.389 ±0.009 us/op
marshalUnicodeStringStatelessSafe stringSize=512 2.905 ±0.004 us/op
marshalUnicodeStringStatelessUnsafe stringSize=512 2.803 ±0.007 us/op
marshalAsciiStringStateful stringSize=512 0.094 ±0.001 us/op
marshalAsciiStringStatelessSafe stringSize=512 1.284 ±0.009 us/op
marshalAsciiStringStatelessUnsafe stringSize=512 0.128 ±0.002 us/op
marshalLatin1StringStateful stringSize=512 0.804 ±0.015 us/op
marshalLatin1StringStatelessSafe stringSize=512 2.497 ±0.016 us/op
marshalLatin1StringStatelessUnsafe stringSize=512 1.876 ±0.002 us/op
marshalUnicodeStringStateful stringSize=512 1.382 ±0.004 us/op
marshalUnicodeStringStatelessSafe stringSize=512 2.907 ±0.004 us/op
marshalUnicodeStringStatelessUnsafe stringSize=512 2.803 ±0.009 us/op

Java 24 Results

StringMarshalBenchmark Results

PR Branch Results

Benchmark Method Configuration Score Units
marshalAsciiStringStateful FallbackStringEncoder, stringSize=512 0.109 ±0.006 us/op
marshalAsciiStringStateful UnsafeStringEncoder, stringSize=512 0.108 ±0.001 us/op
marshalAsciiStringStateful VarHandleStringEncoder, stringSize=512 0.107 ±0.000 us/op
marshalAsciiStringStateless FallbackStringEncoder, stringSize=512 1.056 ±0.001 us/op
marshalAsciiStringStateless UnsafeStringEncoder, stringSize=512 0.119 ±0.001 us/op
marshalAsciiStringStateless VarHandleStringEncoder, stringSize=512 0.198 ±0.001 us/op
marshalLatin1StringStateful FallbackStringEncoder, stringSize=512 0.808 ±0.011 us/op
marshalLatin1StringStateful UnsafeStringEncoder, stringSize=512 0.797 ±0.007 us/op
marshalLatin1StringStateful VarHandleStringEncoder, stringSize=512 0.801 ±0.006 us/op
marshalLatin1StringStateless FallbackStringEncoder, stringSize=512 2.031 ±0.001 us/op
marshalLatin1StringStateless UnsafeStringEncoder, stringSize=512 1.826 ±0.003 us/op
marshalLatin1StringStateless VarHandleStringEncoder, stringSize=512 2.470 ±0.039 us/op
marshalUnicodeStringStateful FallbackStringEncoder, stringSize=512 1.319 ±0.010 us/op
marshalUnicodeStringStateful UnsafeStringEncoder, stringSize=512 1.324 ±0.006 us/op
marshalUnicodeStringStateful VarHandleStringEncoder, stringSize=512 1.329 ±0.011 us/op
marshalUnicodeStringStateless FallbackStringEncoder, stringSize=512 2.866 ±0.026 us/op
marshalUnicodeStringStateless UnsafeStringEncoder, stringSize=512 3.254 ±0.006 us/op
marshalUnicodeStringStateless VarHandleStringEncoder, stringSize=512 3.273 ±0.004 us/op

Main Branch Results

Benchmark Method Configuration Score Units
marshalAsciiStringStateful stringSize=512 0.109 ±0.001 us/op
marshalAsciiStringStatelessSafe stringSize=512 1.050 ±0.001 us/op
marshalAsciiStringStatelessUnsafe stringSize=512 1.051 ±0.003 us/op
marshalLatin1StringStateful stringSize=512 0.800 ±0.008 us/op
marshalLatin1StringStatelessSafe stringSize=512 2.031 ±0.003 us/op
marshalLatin1StringStatelessUnsafe stringSize=512 2.038 ±0.002 us/op
marshalUnicodeStringStateful stringSize=512 1.320 ±0.009 us/op
marshalUnicodeStringStatelessSafe stringSize=512 3.209 ±0.008 us/op
marshalUnicodeStringStatelessUnsafe stringSize=512 3.238 ±0.005 us/op

@trask trask force-pushed the varhandle-string-encoder branch from db27d02 to 780fafd Compare September 24, 2025 01:56
@codecov Codecov
Copy link

codecov bot commented Sep 24, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 79.08497% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.09%. Comparing base (1e763b2) to head (c5bd6de).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...exporter/internal/marshal/StringEncoderHolder.java 50.00% 10 Missing and 4 partials ⚠️
...exporter/internal/marshal/UnsafeStringEncoder.java 61.29% 8 Missing and 4 partials ⚠️
...ry/exporter/internal/marshal/MarshalerContext.java 62.50% 3 Missing ⚠️
...porter/internal/marshal/FallbackStringEncoder.java 96.42% 1 Missing and 1 partial ⚠️
...porter/internal/marshal/AbstractStringEncoder.java 96.29% 0 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (79.08%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #7701      +/-   ##
============================================
- Coverage     90.12%   90.09%   -0.03%     
- Complexity     7187     7191       +4     
============================================
  Files           814      817       +3     
  Lines         21700    21731      +31     
  Branches       2123     2122       -1     
============================================
+ Hits          19557    19579      +22     
- Misses         1477     1488      +11     
+ Partials        666      664       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@trask trask force-pushed the varhandle-string-encoder branch 8 times, most recently from b9e69c2 to ff9ae14 Compare September 25, 2025 02:43
trask
trask commented Sep 25, 2025
View reviewed changes
...s/common/src/main/java/io/opentelemetry/exporter/internal/marshal/AbstractStringEncoder.java
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
abstract class AbstractStringEncoder implements StringEncoder {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code in this class is just moved (cut-and-paste) from StatelessMarshalerUtil

...s/common/src/main/java/io/opentelemetry/exporter/internal/marshal/FallbackStringEncoder.java
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
final class FallbackStringEncoder implements StringEncoder {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code in this class is just moved (cut-and-paste) from StatelessMarshalerUtil

@trask trask force-pushed the varhandle-string-encoder branch from ff9ae14 to c5bd6de Compare September 25, 2025 03:49
@trask trask marked this pull request as ready for review September 25, 2025 03:49
@trask trask requested a review from a team as a code owner September 25, 2025 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
run benchmarks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@trask