Skip to content

fix(core): compatibility with JDK 26 - #99

Open
jerrinot wants to merge 7 commits into
mainfrom
jh_jdk26
Open

fix(core): compatibility with JDK 26#99
jerrinot wants to merge 7 commits into
mainfrom
jh_jdk26

Conversation

@jerrinot

@jerrinot jerrinot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Applications running the client on JDK 26 crash with IllegalAccessError the first time they send a double that needs the slow formatting path, such as 1e300, 4.9e-324 or Double.MAX_VALUE. Common values like 0.1 or 123456.789 are unaffected, so the failure shows up late and only for some rows. Building the client from source on JDK 26 fails outright.

Cause: JDK 26 (JDK-8366017) made jdk.internal.math.FDBigInteger, which the client's double formatter borrows for its bignum arithmetic, package-private. No module export can make a non-public class reachable.

Fix: the Java 9+ bridge now binds the eight FDBigInteger methods it needs through method handles resolved once at class-init, instead of naming the class in source. Existing --add-exports plumbing and the reflective module export become unnecessary and are removed. There is no performance cost: JMH on JDK 25 shows identical ns/op and B/op to the previous direct calls.

Supported runtimes are unchanged: Java 8 through 26. The shipping JDK 8 artifact was verified on Java 8, 25 and 26; builds and tests were also run on JDK 11, 17, 25 and 26. A JMH benchmark for the double formatter is added under core/src/test.

Fixes #96

Benchmark

DoubleFormatBenchmark (JMH 1.37, added in this PR) run against a client jar built from main (direct FDBigInteger calls) and one from this branch (method handles), same JDK. Each shape is a fixed pool of 1024 doubles cycled per call; the slow-path share was measured by running the pool against the pre-fix jar on JDK 26, where the bignum path throws and the fast path does not.

Machine: AMD Ryzen 9 9950X, Linux, Corretto 25.0.2. Timing: 2 forks x 5 warmup x 5 measurement iterations of 1 s.

shape slow-path share main ns/op this PR ns/op delta
simple (1.0, 123456.789, ...) 0% 20.5 ± 0.6 20.1 ± 0.2 -1.7%
unit (nextDouble()) 0.2% 71.3 ± 0.3 72.5 ± 0.9 +1.7%
scaled (nextDouble() * 1e6) 0% 86.2 ± 0.3 82.2 ± 6.8 -4.7%
wide (longBitsToDouble(random)) 95% 342.2 ± 1.9 343.2 ± 3.5 +0.3%
extreme (1e300, 4.9e-324, MAX_VALUE, ...) 100% 267.8 ± 3.0 273.7 ± 3.3 +2.2%

All deltas are within the run-to-run error. The slow-path rows (wide, extreme) are the ones that exercise the new bridge on every call.

Allocation per call (-prof gc, gc.alloc.rate.norm, 1 fork):

shape main B/op this PR B/op
unit 1.172 1.172
wide 615.104 615.104
extreme 640.002 640.002

Identical to the byte. The handles are static final and invoked with invokeExact on exact primitive signatures, so nothing boxes and the JIT inlines them to direct calls. The slow path's ~600 B/op is the JDK's FDBigInteger arithmetic plus the four FdBig wrappers, unchanged from before.

To reproduce, after mvn -DskipTests package:

java -cp core/target/questdb-client-*-tests.jar:core/target/questdb-client-*[!s].jar:<jmh-core>:<jopt-simple>:<commons-math3> \
     org.openjdk.jmh.Main DoubleFormatBenchmark

JDK 27 (compact object headers)

The 27-ea smoke job initially failed at runtime even though the mechanism is sound. Root cause: JEP 450 compact object headers are enabled by default in JDK 27, shrinking the object header from 12 to 8 bytes and moving AccessibleObject.override from offset 12 to 8. Unsafe hard-coded 12/16, so the override write landed inside the header and setAccessible() silently did nothing. Disabling the feature (-XX:-UseCompactObjectHeaders) made the unchanged jar pass, confirming the cause.

Fix: Unsafe.AccessibleObject_override_fieldOffset() now measures the first-field boundary (via a one-field probe class) instead of hard-coding it. override sits at that boundary in every layout, so this tracks compact (8), compressed (12), uncompressed (16) and 32-bit (8) automatically. Being in the shared source, it also repairs every other Unsafe.makeAccessible call site under compact headers.

Verified: the JDK 8-built jar formats all slow-path doubles on JDK 8, 11, 17, 25, 26 and 27-ea (both +/-UseCompactObjectHeaders). The mrjar-smoke 27-ea job is now a green forward canary (non-blocking, since EA is a moving target). The only remaining JDK-27+ risk is the eventual removal of sun.misc.Unsafe; the durable answer then is a self-contained bignum with no JDK-internal access.

Applications running the client on JDK 26 crash with IllegalAccessError
the first time they send a double that needs the slow formatting path,
such as 1e300, 4.9e-324 or Double.MAX_VALUE. Common values like 0.1 or
123456.789 are unaffected, so the failure shows up late and only for
some rows. Building the client from source on JDK 26 fails outright.

Cause: JDK 26 (JDK-8366017) made jdk.internal.math.FDBigInteger, which
the client's double formatter borrows for its bignum arithmetic,
package-private. No module export can make a non-public class reachable.

Fix: the Java 9+ bridge now binds the eight FDBigInteger methods it needs
through method handles resolved once at class-init, instead of naming the
class in source. Existing --add-exports plumbing and the reflective module
export become unnecessary and are removed. There is no performance cost:
JMH on JDK 25 shows identical ns/op and B/op to the previous direct calls.

Supported runtimes are unchanged: Java 8 through 26. The shipping JDK 8
artifact was verified on Java 8, 25 and 26; builds and tests were also
run on JDK 11, 17, 25 and 26. A JMH benchmark for the double formatter
is added under core/src/test.

Fixes #96
@jerrinot jerrinot added the bug Something isn't working label Sep 11, 2026
jerrinot and others added 6 commits September 11, 2026 18:27
Issue #96 passed every existing check because the JDK 8-built jar was only
ever executed on JDK 8, 11 and 25, and source was only compiled on 8 and
25. The MRJAR smoke job is now a matrix over the JDKs the jar must run on
(25, 26, plus a non-blocking 27-ea early warning) and the compile/javadoc
smoke covers 25 and 26. The check names for JDK 25 are unchanged.

JarPackagingIT also accepts QUESTDB_SMOKE_JDKS (path-separated JDK homes)
so a developer with several JDKs installed gets the same cross-runtime
check locally from `mvn install`.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S8vZt12JbC8o8rGyHYTPrE
Reproduced on Temurin 27+35: the Unsafe write to AccessibleObject.override
no longer takes effect, so isAccessible() stays false and Lookup.unreflect
cannot reach the package-private FDBigInteger. This defeats both this bridge
and the old --add-exports export hack (same primitive). Only a launch-time
--add-opens works there, which a library cannot impose on consumers; the
durable JDK 27+ fix is a self-contained bignum. The non-blocking mrjar-smoke
27-ea CI job tracks this.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S8vZt12JbC8o8rGyHYTPrE
…headers work

JEP 450 compact object headers are default-on in JDK 27, shrinking the object
header from 12 to 8 bytes and moving AccessibleObject.override from offset 12
to 8. Unsafe hard-coded 12/16, so the override write landed inside the header
and setAccessible() silently no-opped -- the FdBig double-formatting bridge
then threw IllegalAccessError on JDK 27 (proven: -XX:-UseCompactObjectHeaders
makes the unchanged jar pass).

AccessibleObject_override_fieldOffset() now measures the first-field boundary
via a one-field probe instead of hard-coding a value; override sits at that
boundary in every layout, so this tracks compact (8), compressed (12),
uncompressed (16) and 32-bit (8) alike. Being in the shared source it also
repairs every other Unsafe.makeAccessible call site under compact headers.

Verified: the JDK 8 MRJAR formats all slow-path doubles on JDK 8/11/17/25/26
and 27-ea in both header modes. The mrjar-smoke 27-ea job is now a green
forward canary. Updates the FdBig note accordingly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S8vZt12JbC8o8rGyHYTPrE
The compact-header layout that broke the FdBig bridge on JDK 27 was only
exercised by the non-blocking 27-ea leg. A regression to a hard-coded
override offset would pass every blocking job yet break users who enable
-XX:+UseCompactObjectHeaders on GA JDK 25/26 (a product flag, no unlock)
and all JDK 27 users.

The mrjar-smoke step now runs DoubleFormatSmoke under three layouts on
every leg: default, +UseCompactObjectHeaders, and -UseCompressedOops
-UseCompressedClassPointers (offset 16). On the blocking 25/26 legs this
turns the compact-header path into a blocking guard. +IgnoreUnrecognizedVMOptions
keeps the flags harmless on any JDK.

Verified locally: with the fix all three layouts pass on 25/26/27-ea; with
the previous hard-coded offset the compact-headers leg fails on 25 and 26.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S8vZt12JbC8o8rGyHYTPrE
The mrjar-smoke job now runs on JDK 25/26/27-ea under several object-header
layouts, so two "on JDK 25" comments were false: merge the duplicated,
contradictory comment block above the job into one, and update the
DoubleFormatSmoke javadoc to describe the actual matrix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S8vZt12JbC8o8rGyHYTPrE
Measured on JDK 25: unreflect returns a DirectMethodHandle, but asType(erased)
wraps the six handles that narrow an Object parameter back to the bignum type
in a cast-inserting BoundMethodHandle; only the two valueOf* handles (return-only
widening) stay direct. No user consequence -- C2 inlines the chain and folds the
casts, allocation is identical to a direct call -- but the sentence was wrong.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S8vZt12JbC8o8rGyHYTPrE
@mtopolnik

Copy link
Copy Markdown
Contributor

[PR Coverage check]

😍 pass : 27 / 50 (54.00%)

file detail

path covered line new line coverage
🔵 io/questdb/client/std/FdBig.java 25 48 52.08%
🔵 io/questdb/client/std/Unsafe.java 2 2 100.00%

@jerrinot

jerrinot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Review — level 3

$BASE = 981bdb0, head 065c7be, 10 files. The repo's only submodule (core/src/main/c/share/zstd) does not move in this diff, so no submodule content is in scope. No binaries in the diff. Classified unit-testable-only (no wire-protocol, transport, HA or kill -9 surface), so no tandem PR is required — and none is linked.

Findings below were verified by execution, not by reading.

Critical

None.

The mechanism is correct. The measured first-field offset tracks every layout (12 default / 8 compact / 16 uncompressed) on JDK 25 and 26; DoubleFormatSmoke passes on both across all three layouts under -ea; the full suite is green on JDK 26 (3456 tests, 0 failures). Base under +UseCompactObjectHeaders fails the identical trigger — AssertionError inside Unsafe.<clinit> with -ea (which kills the whole client, not just double formatting) and IllegalAccessException without it. A wrong offset can also SIGSEGV the JVM, which I reproduced. Strictly better than base on every axis I could measure.

Moderate

M1 — No CI job checks the rewritten java11 bridge against exact output

  • Problem: CI asserts only round-trip, never exact digits, for the rewritten bridge.
  • Net impact: All Java 9+ users of the shipped jar; a wrong-digit regression would ship green.
  • Evidence: Mutant FdBig.cmp → return 1 yields 1.2300000000000001E105 for 1.23E105, round-trips true, passes every mrjar-smoke leg and JarPackagingIT; killed only by NumbersTest.

DoubleFormatSmoke.java:58 asserts doubleToLongBits(parseDouble(formatted)) == doubleToLongBits(d) — value-preserving, not shortest-representation. The only exact-string oracle is NumbersTest, and build-jdk8 is the only CI job that runs tests, where core/pom.xml:550 pins compat.src.dir=src/main/java8. So CI exercises NumbersTest against the sun.misc bridge and never against the src/main/java11 one this PR rewrote. compile-modern-jdk uses -DskipTests; mrjar-smoke runs only the round-trip smoke.

Reachability: Numbers.append(CharSink,double)appendDouble0FdBig, per double value per row on the v1 ILP/UDP sender paths. A regression means silently wrong double text written into tables, with no error anywhere.

Attribution, stated plainly: the blind spot itself is pre-existing. What this PR changes is the risk class it fails to cover — the old bridge was six lines of compiler-checked direct calls; the new one hand-writes eight MethodType descriptors where a mis-binding is not a compile error. That is what moves it onto a live path.

Why Moderate, not Critical: the current code is proven correct (4 JDKs x 3 layouts), the loud mis-binding modes are caught by the smoke, and a real offset exists — compat.src.dir defaults to src/main/java11, so any mvn -pl core test on JDK 11+ does apply the exact-string oracle to this bridge.

Suggested fix (cheap, hardens everything at once): add an expected-string column to DoubleFormatSmoke's existing 7-value table and compare formatted.equals(expected[i]). One table, no new file or CI job; it hardens all three JDK legs x three layouts plus both JarPackagingIT sub-runs. Do not compare against Double.toStringNumbers implements the pre-JDK-19 algorithm and diverges (e.g. 2e231.9999999999999998E23 vs 2.0E23), which would make the check JDK-version-fragile.

M2 — Shipped javadoc asserts a performance equality that measurement contradicts

  • Problem: FdBig javadoc claims the bridge "costs the same as a direct call"; it does not.
  • Net impact: The costs are real but bounded and acceptable — the defect is the overclaim, not the cost.
  • Evidence: perf stat 3-point scaling on JDK 25: +113 instructions/format (slope, intercept 0) against an 8,269 baseline = +1.37%; the first slow-path double costs **+3.3 ms CPU / ~+9 ms wall**, once per JVM.

FdBig.java:162 and the class javadoc. Six of the eight handles become BoundMethodHandle$Species_LL adapters because asType narrows Object → the bignum type, and that cast is not statically foldable now that value is typed Object.

The ns/op and allocation claims do hold — allocation is byte-identical (640 B/op both sides, gc.alloc.rate.norm matching to three decimals), and wall clock shows no consistent direction. It is the strict "same as a direct call" wording and the cold-start cost that are wrong.

DoubleFormatBenchmark cannot observe the cold-start cost by construction: JMH pays <clinit> and all LambdaForm spinning inside warm-up, and Mode.AverageTime over steady state cannot report a one-shot. Its documented A/B recipe (DoubleFormatBenchmark.java:63-65) is also not executable on the JDK the fix targets — base does not compile on JDK 26.

Suggested fix: soften both claims to what is evidenced ("same allocation and, within measurement error, the same ns/op; ~1.4% more instructions per bridged call, plus a one-off handle-resolution cost at the first slow-path double"), and note that the A/B is only possible on JDK <= 25.

Minor

  • Fixes #96 sits at line 9 of the body, after four paragraphs; convention puts it at the top.
  • JarPackagingIT.java:74-76 claims QUESTDB_SMOKE_JDKS gives "the same check from mvn install" as CI. It does not: runSmokeAgainstJar (line 88) passes no JVM flags, and the object-header layout is precisely the dimension that catches an offset regression. Reword, or pass the three layouts.
  • ci.yml:154-156-XX:+IgnoreUnrecognizedVMOptions on all three legs means a future JDK renaming or retiring UseCompactObjectHeaders turns the compact-headers leg into a silent duplicate of default, job still green (reproduced: the flag is ignored on JDK 17). Echoing the effective flag (-XX:+PrintFlagsFinal -version | grep UseCompactObjectHeaders) inside run_smoke makes vacuity visible at no risk. Worth noting the three layouts are the minimal kill set — dropping any one lets some hard-coded constant survive.
  • DoubleFormatBenchmark.java:96-102 — instance fields ordered shape, index, builder, sink, values; the arrangement convention puts final before non-final. (POOL before MASK is correct: MASK = POOL - 1 makes alphabetical order an illegal forward reference.)

Coverage

Test gate passes. One admitted coverage gap (M1, Moderate); no Critical gap.

The offset half of the fix is covered: mutation testing confirms no hard-coded constant (8/12/16) survives all three blocking smoke legs, and the matrix genuinely expands to three jobs with continue-on-error true only for 27-ea, so 25 and 26 block. The rendered check names are byte-identical to the old ones, so existing branch protection survives the job rename. FdBig's 52% line coverage decomposes to 100% happy-path and 0% error-path; the uncovered lines are exactly the eight catch/rethrow pairs, rethrow's body and the ExceptionInInitializerError path — all unreachable on any supported JDK, and rethrow's reachable branch is functionally confirmed by mutation. Legitimate N-A, not a gap.

Summary

Verdict: approve with comments. Both gates pass. M1 is worth doing in this PR — a few lines, and it closes a blind spot this change makes newly load-bearing. M2 is a one-paragraph doc correction. Neither blocks.

Severity distribution: 0 Critical, 2 Moderate, 4 Minor. All findings are in-diff; no out-of-diff breakage. The parent questdb repo has zero references to io.questdb.client.std.Unsafe, and its own io.questdb.std.Unsafe has no override-offset logic, so there is no twin bug downstream.

Tradeoff worth stating: the PR trades compile-time type checking for runtime handle resolution. That is unavoidable — no module export can reach a package-private class — and the implementation is correct. M1 is the compensating control that trade implies.

Unrelated to this PR, but found while reviewing

BackgroundDrainerMidDrainCapabilityGapTest#testDeliveringBetweenTwoGapWindowsGrantsAFreshSettleBudget is flaky on JDK 11 at both revisions. It first looked like a head-only regression (head failed, base passed); it is not. Across repeated runs base fails 4/6 and head fails 2/7, and the outcome inverts between isolated-method and whole-class modes. It is a wall-clock-budget test (10 s). CI never sees it, because only the JDK 8 job runs tests. Probably worth its own issue.

@jerrinot jerrinot changed the title fix(core): restore double formatting on JDK 26 fix(core): compatibility with JDK 26 Sep 11, 2026
@jerrinot jerrinot added the READY label Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working READY

Projects

None yet

Development

Successfully merging this pull request may close these issues.

java-questdb-client fails to build on JDK 26: FDBigInteger no longer accessible in jdk.internal.math

2 participants