lhotari opened a new pull request, #4876:
URL: https://github.com/apache/bookkeeper/pull/4876
### Motivation
`Java9IntHash` reaches `java.util.zip.CRC32C`'s private `updateBytes` and
`updateDirectByteBuffer`
through `java.lang.reflect.Method.invoke`. `Method.invoke` takes its
arguments as an `Object[]`, so
every checksum boxes the running CRC, the buffer address and both offsets,
and allocates the array to
hold them. That happens once per checksummed buffer, on the ledger write
path.
**This is the fallback used on every platform without the SSE 4.2 native
library, which is every
platform except x86-64 Linux.** `Crc32cIntChecksum` picks `JniIntHash` only
when
`Sse42Crc32C.isSupported()`, and the published `circe-checksum` jar contains
exactly one native
library:
```
$ unzip -l circe-checksum-4.18.0.jar | grep lib/
0 lib/
16936 lib/libcirce-checksum.so
$ file lib/libcirce-checksum.so
lib/libcirce-checksum.so: ELF 64-bit LSB shared object, x86-64, version 1
(SYSV), ...
```
It is built from `crc32c_sse42.cpp`, so it is x86-only by construction. On
**ARM** — Graviton,
Ampere, Apple Silicon — the load fails on the wrong architecture; on
**macOS** and **Windows** there
is no `.dylib`/`.dll` in the jar at all. All of them fall through to
`Java9IntHash` and pay the boxing
on every entry. Deployments on x86-64 Linux take the native path and never
see this.
I found it profiling a Pulsar broker on aarch64 under a write-heavy
workload: allocation sampling put
**9% of all broker allocation** in `Java9IntHash.updateDirectByteBuffer` →
`Long.valueOf` /
`Integer.valueOf`, and the broker was GC-bound.
### Changes
Bind the two methods to `MethodHandle`s and call them with `invokeExact`.
The arguments are passed as
primitives, nothing is allocated, and the JIT can inline through a `static
final` handle.
`Lookup.unreflect` skips its own access check for a `Method` whose
accessible flag is already set,
which is what the existing code establishes with `setAccessible(true)`. So
this needs no Java 9+
lookup API (no `privateLookupIn`) and **still compiles at source level 8**,
which matters for
branch-4.17 and earlier if this is worth backporting. Behaviour when
`java.util.zip.CRC32C` cannot be
reached is unchanged — `HAS_JAVA9_CRC32C` stays false and
`Crc32cIntChecksum` falls back to
`Java8IntHash`.
One behavioural difference worth noting: `invokeExact` does not wrap an
exception raised by the
target the way `Method.invoke` does with `InvocationTargetException`, so an
unchecked exception now
propagates unchanged instead of nested inside a `RuntimeException`.
#### Results
`DigestTypeBenchmark`, `digest=CRC32_C`,
`bufferType=BYTE_BUF_DEFAULT_ALLOC`, JDK 21.0.11 on
aarch64 (so the JNI path is unavailable and `Java9IntHash` is in use), `-f 2
-wi 5 -i 5`:
| entry size | throughput before | throughput after | allocation before |
allocation after |
|---:|---:|---:|---:|---:|
| 64 | 350,622 ± 6,010 ops/ms | **510,521 ± 3,182 ops/ms** (+46%) | 24 B/op,
8,024 MB/s | ~0 B/op, 0.004 MB/s |
| 1024 | 20,765 ± 311 ops/ms | 21,050 ± 75 ops/ms (+1%) | 40 B/op, 792 MB/s
| ~0 B/op, 0.004 MB/s |
Larger entries are dominated by the checksum computation itself, so there
the win is the allocation
rather than the throughput — 8 GB/s of garbage at 64-byte entries becomes
none at all, and
`gc.count` for that run goes from 240 to 0.
#### Tests
`Java9IntHashTest` gains a case that checks `Java9IntHash` against
`Java8IntHash` — an independent
implementation of the same algorithm — over a direct buffer, a heap buffer
and a buffer with neither
an array nor a memory address, plus the incremental `resume` form.
The direct-buffer case is the one that needed adding: it is the only caller
of
`updateDirectByteBuffer`, whose `(int, long, int, int)int` shape has to be
matched exactly by
`invokeExact`, and a mismatch fails at runtime with
`WrongMethodTypeException` rather than at compile
time. No existing test reached it — I confirmed that by swapping the two
offset arguments, which the
new test catches and the pre-existing tests do not.
`mvn -pl circe-checksum test` passes (79 tests), as do the
`bookkeeper-server` digest tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
