From 5eefcf19b30f3c61d95b4a2532ceffff30726681 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 00:05:54 +0000 Subject: [PATCH 1/2] perf(encode): audit the write path and apply the wins that measured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds docs/encoding-performance.md — a standing audit of the three encode paths with what was applied, what measured as nothing and why, and a ranked backlog. Measured on an interleaved A/B of two benchmarks.jar builds, same host, -f 3 -wi 3 -i 5. Throughput: SimpleMessage codegen +12.1% (UTF-16) / +13.5% (UTF-8), typed +7.7% / +2.6%; ComplexMessage +3.2% / +2.6%; Struct +57% / +64%. Allocation: -88 B/op everywhere (30% of SimpleMessage's total), -657 B/op on Struct, -1294 B/op on map-heavy typed encodes. Applied: - Word-packed field names in codegen. fastjson2's writeName2Raw(long) ... writeName16Raw(long, long) family takes the name already packed into machine words and stores it with one or two Unsafe.putLongs — what fastjson2's own ASM/@JSONCompiled writers use. New FieldNames derives the constants as native-order word reads out of the literal `"name":` (offset 0, except lengths 8 and 16 where fastjson2 emits the opening quote itself). The same constants serve UTF-8 and UTF-16 writers, so the isUTF8() branch and the second array disappear rather than moving. Codegen only, by measurement: a runtime holder has to switch on the length, and that indirect branch cost 10% on ComplexMessage's typed path — recorded in FieldName's javadoc. - Typed Struct/Value/ListValue. getOneofs() allocates an Arrays.asList + unmodifiableList pair on every call, so the old path allocated twice per Value written, then scanned for the set oneof field and switched on its String name. Compiled messages now use getFieldsMap()/getKindCase(); DynamicMessage keeps the reflective path with the map-entry descriptors hoisted out of the per-entry loop and the kind oneof cached. - Typed WKT wrapper writes in codegen — an Int32Value/StringValue/... field emits writeInt32(wrap.getValue()) instead of routing through WellKnownTypes.write()'s full-name String switch and getField() boxing. - One JSONWriter.Context per encoder instead of one per encode call. - Reflection path checks hasField before getField, so an absent optional field no longer pays a boxed reflective read that is thrown away. - Map key/value FieldDescriptors resolved at schema-build time; FieldWriter.writeMap no longer calls findFieldByName per map write. - Repeated numeric/bool lists narrow to Internal.IntList/LongList/... and read primitives; repeated strings go through writeString(List). The boxing removal measured as exactly nothing — HotSpot already scalar-replaces it once the loop inlines — and is kept only as insurance for the C1 and interpreted tiers. Documented as such so nobody re-measures it hoping. Also fixes a latent correctness bug found on the way: an explicit [json_name = "..."] carrying non-ASCII was truncated by a (byte) charAt(i) cast, and one carrying a quote or newline made the generator emit Java source that does not compile. Such names now go through writeName(String) and all emitted literals are escaped. Allocation budgets tightened to the new baselines. New FieldNamesTest pins the undocumented fastjson2 packing layout for every length on both encodings, so a fastjson2 upgrade that moves it fails there rather than corrupting every field name; 545 tests pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Rjh5q2cHevWc6DPzyoACQp --- CLAUDE.md | 26 +- allocation-check.sh | 18 +- buff-json-protoc-plugin/CLAUDE.md | 48 +-- .../buffjson/protoc/EncoderGenerator.java | 306 +++++++++++++---- .../suboptimal/buffjson/FieldNamesTest.java | 189 +++++++++++ buff-json/CLAUDE.md | 27 +- .../suboptimal/buffjson/BuffJsonEncoder.java | 15 +- .../buffjson/internal/FieldNames.java | 233 +++++++++++++ .../buffjson/internal/FieldWriter.java | 27 +- .../buffjson/internal/MessageSchema.java | 56 ++-- .../internal/ProtobufMessageWriter.java | 30 +- .../buffjson/internal/WellKnownTypes.java | 66 +++- .../buffjson/internal/typed/FieldName.java | 43 ++- .../internal/typed/TypedFieldAccessor.java | 159 +++++++-- .../typed/TypedFieldAccessorFactory.java | 19 +- docs/encoding-performance.md | 316 ++++++++++++++++++ 16 files changed, 1358 insertions(+), 220 deletions(-) create mode 100644 buff-json-tests/src/test/java/io/suboptimal/buffjson/FieldNamesTest.java create mode 100644 buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldNames.java create mode 100644 docs/encoding-performance.md diff --git a/CLAUDE.md b/CLAUDE.md index 666222b..a4c0291 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -21,11 +21,12 @@ BuffJsonEncoder.encode(message) holder.buffJsonEncoder().writeFields(jw, msg, writer) # typed getters, no reflection -> nested: OtherEncoder.INSTANCE.writeFields(jw, nested, writer) # direct call -> WKT Timestamp/Duration: writeTimestampDirect(seconds, nanos) # no reflection - -> field name: if (utf8) writeNameRaw(NAME_X_BYTES); else writeNameRaw(NAME_X); + -> WKT wrappers: jsonWriter.writeInt32(wrap.getValue()) # no reflection + -> field name: writeNameRaw(NAME_X_W0[, ...]) # packed words, no branch -> 2. if useTyped && !(message instanceof DynamicMessage) # TYPED-ACCESSOR TypedMessageSchema.forMessage(...).writeFields(jw, msg, this) # LambdaMetafactory-bound getters -> ToIntFunction, ToLongFunction, Predicate, Function - -> RepeatedIntAccessor / RepeatedLongAccessor / RepeatedStringAccessor / RepeatedMessageAccessor + -> RepeatedInt/Long/Double/Float/Bool/String/MessageAccessor # primitive lists, no boxing -> TypedMapAccessor uses getXxxMap() / getXxxValueMap() -> 3. MessageSchema.forDescriptor(...) iteration # PURE REFLECTION Object value = message.getField(fd); FieldWriter.writeValue(...) # boxing @@ -37,19 +38,19 @@ BuffJsonEncoder.encode(message) Codegen optimizations: - **Direct nested encoder calls** — `AddressJsonEncoder.INSTANCE.writeFields(jw, msg, writer)` instead of routing through `ProtobufMessageWriter` (avoids ConcurrentHashMap lookup + instanceof check per nested message). - **Inline WKT Timestamp/Duration** — `WellKnownTypes.writeTimestampDirect(jsonWriter, ts.getSeconds(), ts.getNanos())` bypasses descriptor string switch, field cache lookup, and `getField()` reflection+boxing. +- **Inline WKT wrappers** — an `Int32Value`/`StringValue`/… field emits `jsonWriter.writeInt32(wrap.getValue())` etc. directly, so the nine wrapper types skip `WellKnownTypes.write()`'s full-name String switch, descriptor cache lookup and `getField()` boxing entirely. - **Pre-cached enum name arrays** — static `String[]` built at class init from enum descriptor values, replaces `forNumber()` + `getValueDescriptor().getName()` per write. - **String map key optimization** — avoids redundant `toString()` for String-typed map keys. - **Zero-alloc int64**: `writeString((long) v)` (signed) and `WellKnownTypes.writeUnsignedLongString(jw, v)` (unsigned) — no `Long.toString()` / `Long.toUnsignedString()` String allocation. - **Zero-alloc bytes**: `jsonWriter.writeBase64(v.toByteArray())` — no Base64 String intermediate. - **`isFinite()`-first** float/double NaN/Inf branches. -- **Pre-encoded UTF-8 byte[] field names** — both `char[] NAME_X` and `byte[] NAME_X_BYTES` emitted; `boolean utf8 = jsonWriter.isUTF8()` hoisted at top of `writeFields`; per-field branch via `emitWriteName` — JIT specializes per call site. +- **Word-packed field names** — a name of 2–16 printable-ASCII characters (i.e. essentially every protobuf field) is packed into one or two `long`s at class init and written with fastjson2's `writeName2Raw(long)` … `writeName16Raw(long, long)`: one or two `Unsafe.putLong` stores instead of a static array load plus `arraycopy`. `JSONWriterUTF16` widens the *same* packed bytes on the way out, so there is **no `isUTF8()` branch** on this path — the `char[]`/`byte[]` pair and the hoisted `boolean utf8` local are emitted only for the rare unpackable `json_name`. See `FieldNames`. +- **No boxing in repeated numeric fields** — repeated int32/int64/double/float/bool lists are `instanceof`-narrowed to `Internal.IntList`/`LongList`/`DoubleList`/`FloatList`/`BooleanList` and read with `getInt(i)`/`getLong(i)`/…, avoiding the `Integer.valueOf`/`Long.valueOf` allocation `List.get(i)` performs per element. +- **Bulk repeated strings** — `jsonWriter.writeString(values)` hands the whole `List` to fastjson2, which writes brackets, commas and per-element escaping with one capacity check. **2. Typed-accessor path** (~6x JsonFormat) — used when codegen is disabled or unavailable. `TypedMessageSchema` caches per-Descriptor arrays of `TypedFieldAccessor` records (sealed interface, ~20 variants) whose getter slots are bound via `LambdaMetafactory` to the protoc-generated typed getters. No `getField()` reflection, no boxing. Specialized `RepeatedInt/Long/String/MessageAccessor` and `TypedMapAccessor` eliminate per-element switch dispatch. A `FAILED` sentinel marks descriptors where lambda binding failed (silent fallback to path 3). -**3. Pure-reflection path** — used for `DynamicMessage` (no compiled getters → can't bind lambdas) or descriptors where `LambdaMetafactory` failed. Iterates cached `MessageSchema.FieldInfo[]`, calls `message.getField(fd)` (boxes primitives), `FieldWriter` switches on `JavaType`. Field-name dispatch (`nameWithColon` / `nameWithColonUtf8`) matches the codegen UTF-8 win. - -fastjson2 handles: buffer pooling, number formatting, string escaping, UTF-8 encoding, Base64 encoding (`writeBase64(byte[])`). -We handle: protobuf field extraction, proto3 JSON spec compliance, well-known types, epoch→calendar arithmetic for timestamps. +**3. Pure-reflection path** — used for `DynamicMessage` (no compiled getters → can't bind lambdas) or descriptors where `LambdaMetafactory` failed. Iterates cached `MessageSchema.FieldInfo[]`, calls `message.getField(fd)` (boxes primitives) — after `hasField` for fields with presence, so an absent optional field costs no boxed read — and `FieldWriter` switches on `JavaType`. Field names go through the same `FieldName` (`char[]`/`byte[]`, dispatched on `isUTF8()`) as the typed path. fastjson2 handles: buffer pooling, number formatting, string escaping, UTF-8 encoding, Base64 encoding (`writeBase64(byte[])`). We handle: protobuf field extraction, proto3 JSON spec compliance, well-known types, epoch→calendar arithmetic for timestamps. @@ -96,7 +97,9 @@ JSONFactory.getDefaultObjectReaderProvider().register(decoder.readerModule()); - **Module exposure** — `encoder.writerModule()` and `decoder.readerModule()` return fastjson2 modules backed by configured writer/reader instances, for mixed pojo+protobuf projects using `JSON.toJSONString()`. - **`MessageSchema` caching**: One-time cost per Descriptor. Avoids `getAllFields()` TreeMap allocation. - **`TypedMessageSchema` caching**: Per-Descriptor `LambdaMetafactory`-bound typed accessors, lazily built. `FAILED` sentinel marks descriptors where binding failed (silent fallback to reflection path). -- **Pre-computed field names — both `char[]` and `byte[]`**: `writeNameRaw(byte[])` is faster on UTF-8 writers (direct `arraycopy`), but throws `UnsupportedOperation` on `JSONWriterUTF16`. Both runtime (`MessageSchema.FieldInfo`) and codegen (`*JsonEncoder`) and typed-accessor (`FieldName` record) hoist `boolean utf8 = jsonWriter.isUTF8()` once and dispatch per write. +- **Word-packed field names (`FieldNames`)**: fastjson2's `writeNameRaw(long[, long])` family takes the name already packed into machine words and stores it with one or two `Unsafe.putLong`s — it is what fastjson2's own ASM/`@JSONCompiled` writers use, and it is the main remaining gap to the POJO ceiling. `FieldNames` derives the constants by reading native-order words straight out of the literal `"name":` text (offset 0, except lengths 8 and 16 where fastjson2 emits the opening quote itself). **The same constants serve UTF-8 and UTF-16 writers**, because `JSONWriterUTF16` widens the packed ASCII to `char`s on the way out — so packed names need no `isUTF8()` branch at all. **Codegen only, by measurement**: the plugin knows each name's length so it emits the exact `writeNameRaw` call with no dispatch (+12–13% on `SimpleMessage`). Routing the runtime paths through it too requires a `switch` on the length — an indirect branch in the hottest code keyed on a per-field value — which cost 10% on `ComplexMessage.buffJsonRuntime`, so `FieldName` keeps the pre-encoded `char[]`/`byte[]` pair. `FieldNames` still supplies those arrays, and codegen falls back to them for the rare unpackable `json_name`. A `json_name` that is not `FieldNames.isRawWritable` at all (non-ASCII, or carrying a quote/backslash/control char) goes through `writeName(String)` + `writeColon()` so fastjson2 escapes and transcodes it — the old `(byte) charAt(i)` array builder corrupted such names, and the generator emitted them into Java literals unescaped, which did not compile. +- **Shared `JSONWriter.Context`**: `JSONWriter.of()` allocates a fresh `Context` per call to carry configuration that never changes between encodes. `BuffJsonEncoder` holds one `JSONFactory.createWriteContext()` and passes it to `JSONWriter.of(ctx)`/`ofUTF8(ctx)`. The context is read-only during writing, so one per encoder is safe to share across threads. +- **No boxing on repeated primitives**: protobuf-java backs repeated numeric fields with `Internal.IntList`/`LongList`/`DoubleList`/`FloatList`/`BooleanList`; `List.get(i)` on those boxes every element. Both codegen and the typed accessors `instanceof`-narrow and read via the primitive getters, keeping a generic `List` branch for non-protobuf list implementations. - **`message.getField(descriptor)`** for field access in pure-reflection path only (involves boxing for primitives). Codegen and typed-accessor paths bypass it entirely. - **`Float.floatToRawIntBits() == 0`** for default value checks (correctly handles `-0.0`). - **Native fastjson2 methods** for zero-allocation writes: `writeString(long)` for signed int64 (no `Long.toString()` allocation), `writeBase64(byte[])` for bytes fields (no intermediate Base64 String), `writeNameRaw(byte[])` for field names on UTF-8 path (direct `arraycopy`). @@ -110,6 +113,13 @@ JSONFactory.getDefaultObjectReaderProvider().register(decoder.readerModule()); - **Decoder descriptor cache**: `GeneratedDecoderRegistry` is a simple `ConcurrentHashMap` populated as a side-effect of `instanceof` lookups. Used only for the descriptor-only decode path (nested messages in the runtime reflection path). - **Baked JSON Schema resource (single home for comments, no injected code)**: The protoc plugin bakes one full JSON Schema per message to `META-INF/buff-json/schema/.json` by reusing `ProtobufSchema` at code-gen time (comments from `SourceCodeInfo`, which protoc always sends to plugins; buf.validate constraints from a registered `ExtensionRegistry`). Nothing is injected into protobuf's generated classes — they keep depending only on `protobuf-java`. There is **no** separate comments resource: comments exist only inside the baked schema. At runtime `ProtobufSchema.generateJson()` returns the baked text verbatim, and `ProtobufSchema.generate()→Map` does the live descriptor walk (for exact `Integer`/`Long`/`Float`/`Double` types a JSON round-trip can't preserve) then **overlays the `description` fields** from the baked schema (parsed with fastjson2, structurally matched). When no baked schema exists (e.g. a `DynamicMessage` from a `.desc` with `--include_source_info`), comments come from `SourceCodeInfo`. GraalVM native-image friendly: `buff-json-schema` ships one `resource-config.json` whose glob includes the schema resources from any jar on the classpath, so consumers need no native-image config of their own. +## Encoding Performance + +`docs/encoding-performance.md` is the standing audit of the encode path: what was measured and +applied, what measured as *nothing* and why (HotSpot already handled it), and the ranked backlog +of remaining opportunities with the reasoning behind each. Read it before optimizing the write +path — several attractive-looking ideas are recorded there as already-tried or already-refuted. + ## Allocation Regression Check `./allocation-check.sh` runs JMH `-prof gc` on a representative subset of benchmarks (SimpleMessage codegen+runtime × UTF-16+UTF-8, ComplexMessage codegen+runtime, DoubleHeavy codegen × UTF-16+UTF-8) and asserts `gc.alloc.rate.norm` (bytes per `@Benchmark` invocation) stays within per-benchmark budgets defined in the script. Total runtime ~1 minute. `--quick` flag for local iteration. Wired into CI as a separate `allocation-check` job in `.github/workflows/ci.yml`. Catches regressions like a missed zero-alloc path, a forgotten try-with-resources, or a new String/byte[] allocation per call. diff --git a/allocation-check.sh b/allocation-check.sh index dce8617..751f4fd 100755 --- a/allocation-check.sh +++ b/allocation-check.sh @@ -33,17 +33,19 @@ RESULTS_FILE="${RESULTS_FILE:-/tmp/buff-json-alloc-check.json}" # across runs and platforms. If you tighten or widen a budget, update the # baseline comment. BUDGETS=( - # SimpleMessage (6 fields, ~80-byte JSON output) — String/byte[] return is the - # dominant allocation; the encoder itself is near-zero. - "io.suboptimal.buffjson.benchmarks.SimpleMessageBenchmark.compiledUtf16:380" # baseline ~296 B/op - "io.suboptimal.buffjson.benchmarks.SimpleMessageBenchmark.compiledUtf8:350" # baseline ~272 B/op - "io.suboptimal.buffjson.benchmarks.SimpleMessageBenchmark.runtimeUtf16:400" # baseline ~296 B/op (typed-accessor) - "io.suboptimal.buffjson.benchmarks.SimpleMessageBenchmark.runtimeUtf8:380" # baseline ~272 B/op (typed-accessor) + # SimpleMessage (6 fields, ~80-byte JSON output) — the String/byte[] return is + # now essentially the *only* allocation; the encoder itself is zero. Dropped + # by 88 B/op when BuffJsonEncoder started sharing one JSONWriter.Context + # instead of letting JSONWriter.of() allocate one per call. + "io.suboptimal.buffjson.benchmarks.SimpleMessageBenchmark.compiledUtf16:290" # baseline ~208 B/op + "io.suboptimal.buffjson.benchmarks.SimpleMessageBenchmark.compiledUtf8:260" # baseline ~184 B/op + "io.suboptimal.buffjson.benchmarks.SimpleMessageBenchmark.runtimeUtf16:290" # baseline ~208 B/op (typed-accessor) + "io.suboptimal.buffjson.benchmarks.SimpleMessageBenchmark.runtimeUtf8:260" # baseline ~184 B/op (typed-accessor) # ComplexMessage (nested + repeated + maps + oneof) — bigger output, more # internal collections. - "io.suboptimal.buffjson.benchmarks.ComplexMessageBenchmark.buffJsonCompiled:2300" # baseline ~1773 B/op - "io.suboptimal.buffjson.benchmarks.ComplexMessageBenchmark.buffJsonRuntime:2500" # baseline ~1773 B/op + "io.suboptimal.buffjson.benchmarks.ComplexMessageBenchmark.buffJsonCompiled:2000" # baseline ~1444 B/op + "io.suboptimal.buffjson.benchmarks.ComplexMessageBenchmark.buffJsonRuntime:1900" # baseline ~1308 B/op # DoubleHeavy (25 doubles, IoT/telemetry profile) — number formatting cost. "io.suboptimal.buffjson.benchmarks.DoubleHeavyBenchmark.compiledUtf16:2200" # baseline ~1773 B/op diff --git a/buff-json-protoc-plugin/CLAUDE.md b/buff-json-protoc-plugin/CLAUDE.md index 16fb495..2ac7183 100644 --- a/buff-json-protoc-plugin/CLAUDE.md +++ b/buff-json-protoc-plugin/CLAUDE.md @@ -33,34 +33,38 @@ For each non-WKT, non-map-entry message type: 1. A `FooJsonEncoder.java` class implementing `BuffJsonGeneratedEncoder` 2. A `public static final INSTANCE` singleton for direct calls from other encoders -3. Pre-computed name constants per field — both `char[] NAME_*` (UTF-16 path) and `byte[] NAME_*_BYTES` (UTF-8 path), populated from `nameChars(...)` / `nameBytes(...)` helpers at class init. ASCII-only. +3. Word-packed name constants per field — `static final long NAME_*_W0` (plus `_W1`/`_T` for names of 10–16 / exactly 9 characters), from `FieldNames.packedWord0/packedWord1/packedTailInt` at class init, written with `jsonWriter.writeNameRaw(...)`. The same words serve UTF-8 and UTF-16 writers, so there is no encoding branch. (Codegen only — the runtime paths measured *slower* with the length switch a generic holder needs; see `FieldName`.) Names outside 2–16 printable ASCII fall back to `char[] NAME_*` + `byte[] NAME_*_BYTES` (from `FieldNames.nameWithColonChars/Bytes`) and the hoisted `boolean utf8` local, which is now emitted **only** when such a field exists. A `json_name` that is not raw-writable at all (non-ASCII, or carrying a quote/backslash/control char) becomes `String NAME_*_STR` written via `writeName(String)` + `writeColon()`; all emitted name literals go through `javaStringLiteral` so such a name can no longer produce source that fails to compile. 4. Pre-cached `String[] ENUM_*_NAMES` arrays for each enum type (built from enum descriptor at class init, avoiding `UNRECOGNIZED` which throws from `getNumber()`) -5. A `writeFields(JSONWriter, T, ProtobufMessageWriter)` method with inlined per-field encoding logic, opening with `boolean utf8 = jsonWriter.isUTF8();` so each field-name write dispatches via `if (utf8) writeNameRaw(NAME_X_BYTES); else writeNameRaw(NAME_X);` +5. A `writeFields(JSONWriter, T, ProtobufMessageWriter)` method with inlined per-field encoding logic; each field-name write is a single `jsonWriter.writeNameRaw(NAME_X_W0[, ...])` 6. A `message_implements` insertion point per message adding `BuffJsonCodecHolder` to the implements clause 7. A `class_scope` insertion point per message adding `buffJsonEncoder()`/`buffJsonDecoder()` method implementations 8. A `META-INF/buff-json/schema/.json` resource per message (the baked JSON Schema, comments and buf.validate constraints included) — read at runtime by `buff-json-schema`, with nothing injected into the generated protobuf classes ## Field Handling -| Category | Generated pattern | -|--------------------------|---------------------------------------------------------------------------------------------| -| Scalar (no presence) | `int v = msg.getId(); if (v != 0) { emitWriteName; writeInt32(v); }` | -| Scalar (optional) | `if (msg.hasId()) { emitWriteName; writeInt32(msg.getId()); }` | -| uint32/fixed32 | `writeInt64(Integer.toUnsignedLong(...))` | -| int64 variants | `writeString(...)` — no `Long.toString()` allocation, fastjson2 unboxes | -| uint64/fixed64 | `WellKnownTypes.writeUnsignedLongString(jsonWriter, ...)` — no String allocation | -| float/double | Inline NaN/Infinity check, `isFinite()` first (hot path) | -| Enum | Static `ENUM_*_NAMES` array lookup by `msg.getStatusValue()` (no `forNumber()`) | -| bytes | `jsonWriter.writeBase64(v.toByteArray())` — fastjson2 encodes directly into buffer | -| Field name | `if (utf8) writeNameRaw(NAME_X_BYTES); else writeNameRaw(NAME_X);` — JIT-specialized branch | -| Repeated | `msg.getFooList()`, check isEmpty, iterate | -| Map (String key) | `msg.getFooMap()`, iterate, `entry.getKey()` directly (no `toString()`) | -| Map (non-String key) | `msg.getFooMap()`, iterate, `entry.getKey().toString()` | -| Oneof | `switch (msg.getFooCase())` with per-case typed accessor | -| Nested message (non-WKT) | `FooJsonEncoder.INSTANCE.writeFields(jw, nested, writer)` — direct call, bypasses registry | -| Nested message (WKT) | `WellKnownTypes.write(jsonWriter, nested, writer)` | -| Timestamp | `WellKnownTypes.writeTimestampDirect(jsonWriter, ts.getSeconds(), ts.getNanos())` | -| Duration | `WellKnownTypes.writeDurationDirect(jsonWriter, dur.getSeconds(), dur.getNanos())` | +| Category | Generated pattern | +|--------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------| +| Scalar (no presence) | `int v = msg.getId(); if (v != 0) { emitWriteName; writeInt32(v); }` | +| Scalar (optional) | `if (msg.hasId()) { emitWriteName; writeInt32(msg.getId()); }` | +| uint32/fixed32 | `writeInt64(Integer.toUnsignedLong(...))` | +| int64 variants | `writeString(...)` — no `Long.toString()` allocation, fastjson2 unboxes | +| uint64/fixed64 | `WellKnownTypes.writeUnsignedLongString(jsonWriter, ...)` — no String allocation | +| float/double | Inline NaN/Infinity check, `isFinite()` first (hot path) | +| Enum | Static `ENUM_*_NAMES` array lookup by `msg.getStatusValue()` (no `forNumber()`) | +| bytes | `jsonWriter.writeBase64(v.toByteArray())` — fastjson2 encodes directly into buffer | +| Field name | `writeNameRaw(NAME_X_W0[, NAME_X_W1 / NAME_X_T])` — packed words, no encoding branch | +| Field name (unpackable) | `if (utf8) writeNameRaw(NAME_X_BYTES); else writeNameRaw(NAME_X);` | +| Repeated numeric/bool | narrow to `Internal.IntList`/`LongList`/`DoubleList`/`FloatList`/`BooleanList`, read `getInt(i)` etc. (no boxing); generic `List` branch retained | +| Repeated string | `jsonWriter.writeString(values)` — whole array in one call | +| Repeated | `msg.getFooList()`, check isEmpty, iterate | +| Map (String key) | `msg.getFooMap()`, iterate, `entry.getKey()` directly (no `toString()`) | +| Map (non-String key) | `msg.getFooMap()`, iterate, `entry.getKey().toString()` | +| Oneof | `switch (msg.getFooCase())` with per-case typed accessor | +| Nested message (non-WKT) | `FooJsonEncoder.INSTANCE.writeFields(jw, nested, writer)` — direct call, bypasses registry | +| Nested message (WKT) | `WellKnownTypes.write(jsonWriter, nested, writer)` | +| Wrapper WKTs | `jsonWriter.writeInt32(wrap.getValue())` and friends — typed, no reflective WKT dispatch | +| Timestamp | `WellKnownTypes.writeTimestampDirect(jsonWriter, ts.getSeconds(), ts.getNanos())` | +| Duration | `WellKnownTypes.writeDurationDirect(jsonWriter, dur.getSeconds(), dur.getNanos())` | ## Name Resolution @@ -75,7 +79,7 @@ For each non-WKT, non-map-entry message type: - **`google.protobuf.Empty`** is NOT in the WKT set — it serializes as a regular empty message `{}` - **`DynamicMessage`** cannot use generated encoders (would fail cast) — guarded in `ProtobufMessageWriter` - **Map entry types** (`options.map_entry = true`) are skipped — they're synthetic -- **`writeNameRaw(byte[])` throws `UnsupportedOperation` on `JSONWriterUTF16`** — generated code emits both `NAME_X` (char[]) and `NAME_X_BYTES` (byte[]) and dispatches on `boolean utf8 = jsonWriter.isUTF8()` hoisted at the top of `writeFields`. Helper: `EncoderGenerator.emitWriteName(sb, constName, indent)`. +- **Field-name packing** — `EncoderGenerator.emitWriteName(sb, fd, indent)` picks the call shape from `fd.getJsonName().length()`. `EncoderGenerator.isPackable` mirrors `FieldNames.isPackable` so the plugin does not need buff-json's runtime classes loadable during code generation; keep the two in sync. `writeNameRaw(byte[])` still throws `UnsupportedOperation` on `JSONWriterUTF16`, which is why the unpackable fallback keeps both arrays and the `utf8` local. - **Enum `UNRECOGNIZED`** — protobuf's generated `UNRECOGNIZED` constant throws `IllegalArgumentException` from `getNumber()`. Enum name arrays use `EnumDescriptor.getValues()` (not Java `.values()`) to avoid this - **Multiple distinct enum types per message** — each enum's name-array initializer is wrapped in its own `{ }` block inside the single `static {}` so the `edVals`/`max` locals don't collide (a message referencing two different enum types previously failed to compile) - **Negative enum values** (e.g. `NEG = -1`) — can't index the `String[]` name array, so they're skipped during array fill (`if (v.getNumber() >= 0)`, avoids `ArrayIndexOutOfBounds` at class init) and resolved at the write site via a descriptor fallback (`EnumClass.getDescriptor().findValueByNumber(n)`) so a named negative value still serializes by name; only genuinely unknown numbers fall through to the integer diff --git a/buff-json-protoc-plugin/src/main/java/io/suboptimal/buffjson/protoc/EncoderGenerator.java b/buff-json-protoc-plugin/src/main/java/io/suboptimal/buffjson/protoc/EncoderGenerator.java index 2835120..75e015d 100644 --- a/buff-json-protoc-plugin/src/main/java/io/suboptimal/buffjson/protoc/EncoderGenerator.java +++ b/buff-json-protoc-plugin/src/main/java/io/suboptimal/buffjson/protoc/EncoderGenerator.java @@ -43,9 +43,36 @@ final class EncoderGenerator { private static final Set WELL_KNOWN_TYPES = BuffJsonProtocPlugin.WELL_KNOWN_TYPES; + /** + * Emitted body for each wrapper well-known type, given a {@code wrap} local + * holding the wrapper message. Wrappers serialize as their bare value, so the + * whole reflective WKT dispatch collapses to one typed getter plus the same + * scalar write a plain field of that type would use. + */ + private static final Map WRAPPER_WRITES = wrapperWrites(); + private EncoderGenerator() { } + private static Map wrapperWrites() { + Map m = new LinkedHashMap<>(); + m.put("google.protobuf.Int32Value", " jsonWriter.writeInt32(wrap.getValue());\n"); + m.put("google.protobuf.UInt32Value", + " jsonWriter.writeInt64(Integer.toUnsignedLong(wrap.getValue()));\n"); + m.put("google.protobuf.Int64Value", " jsonWriter.writeString(wrap.getValue());\n"); + m.put("google.protobuf.UInt64Value", + " io.suboptimal.buffjson.internal.WellKnownTypes.writeUnsignedLongString(jsonWriter, wrap.getValue());\n"); + m.put("google.protobuf.BoolValue", " jsonWriter.writeBool(wrap.getValue());\n"); + m.put("google.protobuf.StringValue", " jsonWriter.writeString(wrap.getValue());\n"); + m.put("google.protobuf.BytesValue", + " jsonWriter.writeBase64(wrap.getValue().toByteArray());\n"); + m.put("google.protobuf.FloatValue", + " io.suboptimal.buffjson.internal.FieldWriter.writeFloatValue(jsonWriter, wrap.getValue());\n"); + m.put("google.protobuf.DoubleValue", + " io.suboptimal.buffjson.internal.FieldWriter.writeDoubleValue(jsonWriter, wrap.getValue());\n"); + return m; + } + static String generate(Descriptor msgDesc, String javaPackage, String encoderSimpleName, String messageClassName, Map protoToJavaClass, Map protoToEncoderClass) { @@ -60,37 +87,49 @@ static String generate(Descriptor msgDesc, String javaPackage, String encoderSim sb.append(" public static final ").append(encoderSimpleName).append(" INSTANCE = new ") .append(encoderSimpleName).append("();\n\n"); - // Name constants: char[] for UTF-16 writers, byte[] for UTF-8 writers. - // Pre-encoded at class init; ASCII-only since proto field names are ASCII. + // Field-name constants. + // + // Names of 2-16 printable-ASCII characters (i.e. essentially every protobuf + // field) are packed into machine words and written via fastjson2's + // writeNameRaw family: one or two Unsafe.putLong stores, no arraycopy, and + // — because JSONWriterUTF16 widens the same packed bytes on the way out — no + // isUTF8() branch either. Anything else keeps the char[]/byte[] pair. + boolean anyArrayName = false; for (FieldDescriptor fd : msgDesc.getFields()) { if (fd.getOptions().hasDeprecated() && fd.getOptions().getDeprecated()) continue; String jsonName = fd.getJsonName(); - sb.append(" private static final char[] NAME_").append(constantName(fd)); - sb.append(" = nameChars(\"").append(jsonName).append("\");\n"); - sb.append(" private static final byte[] NAME_").append(constantName(fd)); - sb.append("_BYTES = nameBytes(\"").append(jsonName).append("\");\n"); + if (isPackable(jsonName)) { + int n = jsonName.length(); + sb.append(" private static final long NAME_").append(constantName(fd)); + sb.append("_W0 = io.suboptimal.buffjson.internal.FieldNames.packedWord0(") + .append(javaStringLiteral(jsonName)).append(");\n"); + if (n >= 10) { + sb.append(" private static final long NAME_").append(constantName(fd)); + sb.append("_W1 = io.suboptimal.buffjson.internal.FieldNames.packedWord1(") + .append(javaStringLiteral(jsonName)).append(");\n"); + } else if (n == 9) { + sb.append(" private static final int NAME_").append(constantName(fd)); + sb.append("_T = io.suboptimal.buffjson.internal.FieldNames.packedTailInt(") + .append(javaStringLiteral(jsonName)).append(");\n"); + } + } else if (isRawWritable(jsonName)) { + anyArrayName = true; + sb.append(" private static final char[] NAME_").append(constantName(fd)); + sb.append(" = io.suboptimal.buffjson.internal.FieldNames.nameWithColonChars(") + .append(javaStringLiteral(jsonName)).append(");\n"); + sb.append(" private static final byte[] NAME_").append(constantName(fd)); + sb.append("_BYTES = io.suboptimal.buffjson.internal.FieldNames.nameWithColonBytes(") + .append(javaStringLiteral(jsonName)).append(");\n"); + } else { + // An explicit json_name carrying non-ASCII or an escapable character + // cannot be pre-encoded raw — fastjson2 has to escape and transcode it. + sb.append(" private static final String NAME_").append(constantName(fd)); + sb.append("_STR = ").append(javaStringLiteral(jsonName)).append(";\n"); + } } sb.append("\n"); - // nameChars / nameBytes helpers - sb.append(" private static char[] nameChars(String name) {\n"); - sb.append(" char[] chars = new char[name.length() + 3];\n"); - sb.append(" chars[0] = '\"';\n"); - sb.append(" name.getChars(0, name.length(), chars, 1);\n"); - sb.append(" chars[name.length() + 1] = '\"';\n"); - sb.append(" chars[name.length() + 2] = ':';\n"); - sb.append(" return chars;\n"); - sb.append(" }\n\n"); - sb.append(" private static byte[] nameBytes(String name) {\n"); - sb.append(" byte[] bytes = new byte[name.length() + 3];\n"); - sb.append(" bytes[0] = '\"';\n"); - sb.append(" for (int i = 0; i < name.length(); i++) bytes[i + 1] = (byte) name.charAt(i);\n"); - sb.append(" bytes[name.length() + 1] = '\"';\n"); - sb.append(" bytes[name.length() + 2] = ':';\n"); - sb.append(" return bytes;\n"); - sb.append(" }\n\n"); - // Pre-collect enum types used by int-valued fields (implicit presence, // explicit presence, oneof) so we can generate cached name arrays. // Key: enum constant prefix (e.g. "STATUS"), Value: Java enum class name @@ -127,7 +166,9 @@ static String generate(Descriptor msgDesc, String javaPackage, String encoderSim sb.append(" @Override\n"); sb.append(" public void writeFields(JSONWriter jsonWriter, ").append(messageClassName) .append(" message, io.suboptimal.buffjson.internal.ProtobufMessageWriter writer) {\n"); - sb.append(" boolean utf8 = jsonWriter.isUTF8();\n"); + if (anyArrayName) { + sb.append(" boolean utf8 = jsonWriter.isUTF8();\n"); + } // Emit fields in descriptor (field-number) order, matching JsonFormat's // output order. A oneof's switch is emitted at the position of its @@ -165,14 +206,13 @@ private static void generateImplicitPresenceField(StringBuilder sb, FieldDescrip Map protoToJavaClass, Map protoToEncoderClass) { String getter = "message." + getterName(fd) + "()"; - String constName = "NAME_" + constantName(fd); switch (fd.getJavaType()) { case INT -> { sb.append(" {\n"); sb.append(" int v = ").append(getter).append(";\n"); sb.append(" if (v != 0) {\n"); - emitWriteName(sb, constName, " "); + emitWriteName(sb, fd, " "); writeIntValue(sb, fd, "v"); sb.append(" }\n"); sb.append(" }\n"); @@ -181,7 +221,7 @@ private static void generateImplicitPresenceField(StringBuilder sb, FieldDescrip sb.append(" {\n"); sb.append(" long v = ").append(getter).append(";\n"); sb.append(" if (v != 0L) {\n"); - emitWriteName(sb, constName, " "); + emitWriteName(sb, fd, " "); writeLongValue(sb, fd, "v"); sb.append(" }\n"); sb.append(" }\n"); @@ -190,7 +230,7 @@ private static void generateImplicitPresenceField(StringBuilder sb, FieldDescrip sb.append(" {\n"); sb.append(" float v = ").append(getter).append(";\n"); sb.append(" if (Float.floatToRawIntBits(v) != 0) {\n"); - emitWriteName(sb, constName, " "); + emitWriteName(sb, fd, " "); writeFloatValue(sb, "v"); sb.append(" }\n"); sb.append(" }\n"); @@ -199,14 +239,14 @@ private static void generateImplicitPresenceField(StringBuilder sb, FieldDescrip sb.append(" {\n"); sb.append(" double v = ").append(getter).append(";\n"); sb.append(" if (Double.doubleToRawLongBits(v) != 0) {\n"); - emitWriteName(sb, constName, " "); + emitWriteName(sb, fd, " "); writeDoubleValue(sb, "v"); sb.append(" }\n"); sb.append(" }\n"); } case BOOLEAN -> { sb.append(" if (").append(getter).append(") {\n"); - emitWriteName(sb, constName, " "); + emitWriteName(sb, fd, " "); sb.append(" jsonWriter.writeBool(true);\n"); sb.append(" }\n"); } @@ -214,7 +254,7 @@ private static void generateImplicitPresenceField(StringBuilder sb, FieldDescrip sb.append(" {\n"); sb.append(" String v = ").append(getter).append(";\n"); sb.append(" if (!v.isEmpty()) {\n"); - emitWriteName(sb, constName, " "); + emitWriteName(sb, fd, " "); sb.append(" jsonWriter.writeString(v);\n"); sb.append(" }\n"); sb.append(" }\n"); @@ -223,7 +263,7 @@ private static void generateImplicitPresenceField(StringBuilder sb, FieldDescrip sb.append(" {\n"); sb.append(" com.google.protobuf.ByteString v = ").append(getter).append(";\n"); sb.append(" if (!v.isEmpty()) {\n"); - emitWriteName(sb, constName, " "); + emitWriteName(sb, fd, " "); sb.append(" jsonWriter.writeBase64(v.toByteArray());\n"); sb.append(" }\n"); sb.append(" }\n"); @@ -232,7 +272,7 @@ private static void generateImplicitPresenceField(StringBuilder sb, FieldDescrip sb.append(" {\n"); sb.append(" int ev = message.").append(getterName(fd)).append("Value();\n"); sb.append(" if (ev != 0) {\n"); - emitWriteName(sb, constName, " "); + emitWriteName(sb, fd, " "); writeEnumValue(sb, enumArrayConstant(fd), enumJavaClass(fd, protoToJavaClass), "ev"); sb.append(" }\n"); sb.append(" }\n"); @@ -250,10 +290,9 @@ private static void generatePresenceField(StringBuilder sb, FieldDescriptor fd, String hasGetter = "message." + hasName(fd) + "()"; String getter = "message." + getterName(fd) + "()"; - String constName = "NAME_" + constantName(fd); sb.append(" if (").append(hasGetter).append(") {\n"); - emitWriteName(sb, constName, " "); + emitWriteName(sb, fd, " "); switch (fd.getJavaType()) { case INT -> writeIntValue(sb, fd, getter); @@ -279,8 +318,6 @@ private static void generatePresenceField(StringBuilder sb, FieldDescriptor fd, private static void generateRepeatedField(StringBuilder sb, FieldDescriptor fd, String msgClass, Map protoToJavaClass, Map protoToEncoderClass) { - String constName = "NAME_" + constantName(fd); - // For enums, use raw int value list to handle UNRECOGNIZED constants String listGetter; String elementType; @@ -296,38 +333,105 @@ private static void generateRepeatedField(StringBuilder sb, FieldDescriptor fd, sb.append(" java.util.List<").append(elementType).append("> values = ").append(listGetter) .append(";\n"); sb.append(" if (!values.isEmpty()) {\n"); - emitWriteName(sb, constName, " "); + emitWriteName(sb, fd, " "); + + // Repeated string: fastjson2 writes brackets, commas and per-element + // escaping in one call, with a single capacity check for the whole array. + if (fd.getJavaType() == FieldDescriptor.JavaType.STRING) { + sb.append(" jsonWriter.writeString(values);\n"); + sb.append(" }\n"); + sb.append(" }\n"); + return; + } + + // Repeated numeric/bool: protobuf-java backs these with Internal.IntList / + // LongList / DoubleList / FloatList / BooleanList, whose primitive getters + // skip the Integer.valueOf/Long.valueOf boxing that List.get(i) performs on + // every element. The generic List branch stays for the (unusual) case of a + // non-protobuf list implementation. + String primitiveList = primitiveListInterface(fd); + if (primitiveList != null) { + String local = "prim"; + sb.append(" int n = values.size();\n"); + sb.append(" jsonWriter.startArray();\n"); + sb.append(" if (values instanceof ").append(primitiveList).append(' ').append(local) + .append(") {\n"); + sb.append(" for (int i = 0; i < n; i++) {\n"); + sb.append(" if (i > 0) jsonWriter.writeComma();\n"); + emitRepeatedElement(sb, fd, primitiveAccess(fd, local), protoToJavaClass, protoToEncoderClass); + sb.append(" }\n"); + sb.append(" } else {\n"); + sb.append(" for (int i = 0; i < n; i++) {\n"); + sb.append(" if (i > 0) jsonWriter.writeComma();\n"); + emitRepeatedElement(sb, fd, "values.get(i)", protoToJavaClass, protoToEncoderClass); + sb.append(" }\n"); + sb.append(" }\n"); + sb.append(" jsonWriter.endArray();\n"); + sb.append(" }\n"); + sb.append(" }\n"); + return; + } + sb.append(" jsonWriter.startArray();\n"); - sb.append(" for (int i = 0; i < values.size(); i++) {\n"); + sb.append(" for (int i = 0, n = values.size(); i < n; i++) {\n"); sb.append(" if (i > 0) jsonWriter.writeComma();\n"); + emitRepeatedElement(sb, fd, "values.get(i)", protoToJavaClass, protoToEncoderClass); + sb.append(" }\n"); + sb.append(" jsonWriter.endArray();\n"); + sb.append(" }\n"); + sb.append(" }\n"); + } + private static void emitRepeatedElement(StringBuilder sb, FieldDescriptor fd, String expr, + Map protoToJavaClass, Map protoToEncoderClass) { switch (fd.getJavaType()) { - case INT -> writeIntValue(sb, fd, "values.get(i)"); - case LONG -> writeLongValue(sb, fd, "values.get(i)"); - case FLOAT -> writeFloatValue(sb, "values.get(i)"); - case DOUBLE -> writeDoubleValue(sb, "values.get(i)"); - case BOOLEAN -> sb.append(" jsonWriter.writeBool(values.get(i));\n"); - case STRING -> sb.append(" jsonWriter.writeString(values.get(i));\n"); - case BYTE_STRING -> sb.append(" jsonWriter.writeBase64(values.get(i).toByteArray());\n"); + case INT -> writeIntValue(sb, fd, expr); + case LONG -> writeLongValue(sb, fd, expr); + case FLOAT -> writeFloatValue(sb, expr); + case DOUBLE -> writeDoubleValue(sb, expr); + case BOOLEAN -> sb.append(" jsonWriter.writeBool(").append(expr).append(");\n"); + case STRING -> sb.append(" jsonWriter.writeString(").append(expr).append(");\n"); + case BYTE_STRING -> + sb.append(" jsonWriter.writeBase64(").append(expr).append(".toByteArray());\n"); case ENUM -> { // Use raw int values to handle UNRECOGNIZED enum constants // (which throw from getNumber()/getValueDescriptor()) - writeEnumValue(sb, enumArrayConstant(fd), enumJavaClass(fd, protoToJavaClass), "values.get(i)"); + writeEnumValue(sb, enumArrayConstant(fd), enumJavaClass(fd, protoToJavaClass), expr); } - case MESSAGE -> writeMessageValue(sb, fd, "values.get(i)", protoToJavaClass, protoToEncoderClass); + case MESSAGE -> writeMessageValue(sb, fd, expr, protoToJavaClass, protoToEncoderClass); } + } - sb.append(" }\n"); - sb.append(" jsonWriter.endArray();\n"); - sb.append(" }\n"); - sb.append(" }\n"); + /** + * The {@code com.google.protobuf.Internal} primitive-list interface backing a + * repeated field of this type, or {@code null} when there is none (string, + * bytes, message; enums use the boxed {@code Integer} value list). + */ + private static String primitiveListInterface(FieldDescriptor fd) { + return switch (fd.getJavaType()) { + case INT, ENUM -> "com.google.protobuf.Internal.IntList"; + case LONG -> "com.google.protobuf.Internal.LongList"; + case DOUBLE -> "com.google.protobuf.Internal.DoubleList"; + case FLOAT -> "com.google.protobuf.Internal.FloatList"; + case BOOLEAN -> "com.google.protobuf.Internal.BooleanList"; + default -> null; + }; + } + + private static String primitiveAccess(FieldDescriptor fd, String local) { + return switch (fd.getJavaType()) { + case INT, ENUM -> local + ".getInt(i)"; + case LONG -> local + ".getLong(i)"; + case DOUBLE -> local + ".getDouble(i)"; + case FLOAT -> local + ".getFloat(i)"; + case BOOLEAN -> local + ".getBoolean(i)"; + default -> throw new IllegalStateException("no primitive list for " + fd.getJavaType()); + }; } private static void generateMapField(StringBuilder sb, FieldDescriptor fd, String msgClass, Map protoToJavaClass, Map protoToEncoderClass) { - String constName = "NAME_" + constantName(fd); - Descriptor entryDesc = fd.getMessageType(); FieldDescriptor keyFd = entryDesc.findFieldByName("key"); FieldDescriptor valueFd = entryDesc.findFieldByName("value"); @@ -349,7 +453,7 @@ private static void generateMapField(StringBuilder sb, FieldDescriptor fd, Strin sb.append(" java.util.Map<").append(keyType).append(", ").append(valueType).append("> map = ") .append(mapGetter).append(";\n"); sb.append(" if (!map.isEmpty()) {\n"); - emitWriteName(sb, constName, " "); + emitWriteName(sb, fd, " "); sb.append(" jsonWriter.startObject();\n"); sb.append(" for (var entry : map.entrySet()) {\n"); if (keyFd.getJavaType() == FieldDescriptor.JavaType.STRING) { @@ -393,11 +497,10 @@ private static void generateOneof(StringBuilder sb, OneofDescriptor oneof, Strin for (FieldDescriptor fd : oneof.getFields()) { String enumValue = fd.getName().toUpperCase(); - String constName = "NAME_" + constantName(fd); String getter = "message." + getterName(fd) + "()"; sb.append(" case ").append(enumValue).append(" -> {\n"); - emitWriteName(sb, constName, " "); + emitWriteName(sb, fd, " "); switch (fd.getJavaType()) { case INT -> writeIntValue(sb, fd, getter); @@ -511,6 +614,20 @@ private static void writeMessageValue(StringBuilder sb, FieldDescriptor fd, Stri sb.append( " io.suboptimal.buffjson.internal.WellKnownTypes.writeDurationDirect(jsonWriter, dur.getSeconds(), dur.getNanos());\n"); sb.append(" }\n"); + // The typed wrapper write needs the concrete wrapper class in scope. That + // holds when protoToJavaClass knows the type (protoc sends every transitive + // dependency, so it normally does); if it does not, the surrounding + // declaration would be plain Message and getValue() would not compile — fall + // through to the reflective WKT write. + } else if (WRAPPER_WRITES.containsKey(fullName) && protoToJavaClass.containsKey(fullName)) { + // Wrapper WKTs unwrap to a bare JSON value. The type is known here, so emit + // the typed getter directly instead of routing through + // WellKnownTypes.write() -> full-name String switch -> descriptor cache -> + // getField() reflection + boxing. + sb.append(" {\n"); + sb.append(" var wrap = ").append(expr).append(";\n"); + sb.append(WRAPPER_WRITES.get(fullName)); + sb.append(" }\n"); } else if (WELL_KNOWN_TYPES.contains(fullName)) { sb.append(" io.suboptimal.buffjson.internal.WellKnownTypes.write(jsonWriter, ").append(expr) .append(", writer);\n"); @@ -530,12 +647,75 @@ private static void writeMessageValue(StringBuilder sb, FieldDescriptor fd, Stri } /** - * Emits the field-name write — dispatches on the JSONWriter type so UTF-8 - * writers consume pre-encoded byte[] without char→byte transcoding. + * Emits the field-name write. Packable names (2–16 printable-ASCII characters) + * go through fastjson2's {@code writeNameRaw} family — the name is already + * in machine words, so this is one or two {@code putLong}s with no encoding + * branch. Everything else falls back to the pre-encoded + * {@code char[]}/{@code byte[]} pair, dispatched on the hoisted {@code utf8} + * local. + */ + private static void emitWriteName(StringBuilder sb, FieldDescriptor fd, String indent) { + String constName = "NAME_" + constantName(fd); + String jsonName = fd.getJsonName(); + if (!isRawWritable(jsonName)) { + sb.append(indent).append("jsonWriter.writeName(").append(constName).append("_STR);\n"); + sb.append(indent).append("jsonWriter.writeColon();\n"); + return; + } + if (!isPackable(jsonName)) { + sb.append(indent).append("if (utf8) jsonWriter.writeNameRaw(").append(constName) + .append("_BYTES); else jsonWriter.writeNameRaw(").append(constName).append(");\n"); + return; + } + int n = jsonName.length(); + sb.append(indent).append("jsonWriter.writeName").append(n).append("Raw(").append(constName).append("_W0"); + if (n >= 10) { + sb.append(", ").append(constName).append("_W1"); + } else if (n == 9) { + sb.append(", ").append(constName).append("_T"); + } + sb.append(");\n"); + } + + /** + * Mirrors {@code FieldNames.isPackable} at code-generation time so the plugin + * can pick the call shape without depending on buff-json's runtime classes + * being loadable here. + */ + private static boolean isPackable(String jsonName) { + int n = jsonName.length(); + return n >= 2 && n <= 16 && isRawWritable(jsonName); + } + + /** Mirrors {@code FieldNames.isRawWritable} at code-generation time. */ + private static boolean isRawWritable(String jsonName) { + for (int i = 0; i < jsonName.length(); i++) { + char c = jsonName.charAt(i); + if (c < 0x20 || c > 0x7e || c == '"' || c == '\\') { + return false; + } + } + return true; + } + + /** + * Mirrors {@code FieldNames.javaStringLiteral} — an explicit {@code json_name} + * carrying a quote or a newline would otherwise emit source that does not + * compile. */ - private static void emitWriteName(StringBuilder sb, String constName, String indent) { - sb.append(indent).append("if (utf8) jsonWriter.writeNameRaw(").append(constName) - .append("_BYTES); else jsonWriter.writeNameRaw(").append(constName).append(");\n"); + private static String javaStringLiteral(String jsonName) { + StringBuilder sb = new StringBuilder(jsonName.length() + 8).append('"'); + for (int i = 0; i < jsonName.length(); i++) { + char c = jsonName.charAt(i); + if (c == '"' || c == '\\') { + sb.append('\\').append(c); + } else if (c >= 0x20 && c <= 0x7e) { + sb.append(c); + } else { + sb.append(String.format("\\u%04x", (int) c)); + } + } + return sb.append('"').toString(); } // --- Naming helpers --- diff --git a/buff-json-tests/src/test/java/io/suboptimal/buffjson/FieldNamesTest.java b/buff-json-tests/src/test/java/io/suboptimal/buffjson/FieldNamesTest.java new file mode 100644 index 0000000..b5e8698 --- /dev/null +++ b/buff-json-tests/src/test/java/io/suboptimal/buffjson/FieldNamesTest.java @@ -0,0 +1,189 @@ +package io.suboptimal.buffjson; + +import static org.junit.jupiter.api.Assertions.*; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.util.stream.IntStream; + +import com.alibaba.fastjson2.JSONWriter; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; + +import io.suboptimal.buffjson.internal.FieldNames; + +/** + * Locks down the word-packed field-name fast path. + * + *

+ * {@link FieldNames} derives the constants that fastjson2's + * {@code writeName2Raw(long)} … {@code writeName16Raw(long, long)} family + * consumes. That layout is an implementation detail of fastjson2 — it is not + * documented, it differs per name length (lengths 8 and 16 skip the opening + * quote), and a change to it would silently corrupt every field name in + * every message rather than failing loudly. These tests assert the layout + * directly, so a fastjson2 upgrade that moves it fails here instead of in the + * conformance suite. + */ +class FieldNamesTest { + + private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup(); + + static IntStream packableLengths() { + return IntStream.rangeClosed(2, FieldNames.MAX_PACKED_LENGTH); + } + + private static String nameOfLength(int n) { + StringBuilder sb = new StringBuilder(n); + for (int i = 0; i < n; i++) { + sb.append((char) ('a' + i % 26)); + } + return sb.toString(); + } + + /** Invokes the {@code writeNameRaw} overload matching the name's length. */ + private static void writePackedName(JSONWriter jw, String name) throws Throwable { + int n = name.length(); + MethodType type; + if (n == 9) { + type = MethodType.methodType(void.class, long.class, int.class); + } else if (n >= 10) { + type = MethodType.methodType(void.class, long.class, long.class); + } else { + type = MethodType.methodType(void.class, long.class); + } + MethodHandle mh = LOOKUP.findVirtual(JSONWriter.class, "writeName" + n + "Raw", type); + if (n == 9) { + mh.invoke(jw, FieldNames.packedWord0(name), FieldNames.packedTailInt(name)); + } else if (n >= 10) { + mh.invoke(jw, FieldNames.packedWord0(name), FieldNames.packedWord1(name)); + } else { + mh.invoke(jw, FieldNames.packedWord0(name)); + } + } + + @Nested + class PackedLayout { + + @ParameterizedTest + @MethodSource("io.suboptimal.buffjson.FieldNamesTest#packableLengths") + void writesTheNameOnUtf8(int length) throws Throwable { + String name = nameOfLength(length); + try (JSONWriter jw = JSONWriter.ofUTF8()) { + jw.startObject(); + writePackedName(jw, name); + jw.writeInt32(1); + jw.endObject(); + assertEquals("{\"" + name + "\":1}", jw.toString()); + } + } + + @ParameterizedTest + @MethodSource("io.suboptimal.buffjson.FieldNamesTest#packableLengths") + void writesTheNameOnUtf16(int length) throws Throwable { + String name = nameOfLength(length); + try (JSONWriter jw = JSONWriter.of()) { + jw.startObject(); + writePackedName(jw, name); + jw.writeInt32(1); + jw.endObject(); + assertEquals("{\"" + name + "\":1}", jw.toString()); + } + } + + /** + * The comma between fields comes from {@code writeNameRaw} itself, so a + * second field must not lose or duplicate it. + */ + @ParameterizedTest + @MethodSource("io.suboptimal.buffjson.FieldNamesTest#packableLengths") + void separatesConsecutiveFields(int length) throws Throwable { + String name = nameOfLength(length); + try (JSONWriter jw = JSONWriter.ofUTF8()) { + jw.startObject(); + writePackedName(jw, name); + jw.writeInt32(1); + writePackedName(jw, name); + jw.writeInt32(2); + jw.endObject(); + assertEquals("{\"" + name + "\":1,\"" + name + "\":2}", jw.toString()); + } + } + } + + @Nested + class Packability { + + @ParameterizedTest + @ValueSource(strings = {"", "a", "abcdefghijklmnopq", "naïve", "a\"b", "a\\b", "a\tb"}) + void rejectsNamesTheFastPathCannotEncode(String name) { + assertFalse(FieldNames.isPackable(name), name); + assertThrows(IllegalArgumentException.class, () -> FieldNames.packedWord0(name)); + } + + @Test + void acceptsTypicalProtoJsonNames() { + for (String name : new String[]{"id", "userId", "timestampMillis", "scoreValue", "a1"}) { + assertTrue(FieldNames.isPackable(name), name); + } + } + + /** + * The array fallback must stay usable for the names the packed path rejects — + * it is what {@code FieldName} and the generated encoders fall back to. + */ + @Test + void arrayFallbackEncodesTheSameText() { + String name = "x"; + assertTrue(FieldNames.isRawWritable(name)); + assertFalse(FieldNames.isPackable(name)); + assertEquals("\"x\":", new String(FieldNames.nameWithColonChars(name))); + assertArrayEquals("\"x\":".getBytes(java.nio.charset.StandardCharsets.US_ASCII), + FieldNames.nameWithColonBytes(name)); + } + + @ParameterizedTest + @ValueSource(strings = {"naïve", "a\"b", "a\\b", "a\tb", "日本語"}) + void javaLiteralOfAnEscapableNameCompilesAndRoundTrips(String name) { + assertFalse(FieldNames.isRawWritable(name), name); + String literal = FieldNames.javaStringLiteral(name); + assertTrue(literal.startsWith("\"") && literal.endsWith("\""), literal); + // Every character in the literal body is source-safe ASCII. + for (int i = 1; i < literal.length() - 1; i++) { + char c = literal.charAt(i); + assertTrue(c >= 0x20 && c <= 0x7e, literal + " has a raw char at " + i); + } + } + } + + /** + * A {@code [json_name = "…"]} can be any string. Names that cannot be + * pre-encoded raw must be handed to fastjson2 so they get escaped and + * transcoded, not truncated by a {@code (byte) charAt(i)} cast. + */ + @Nested + class EscapingFallback { + + @ParameterizedTest + @ValueSource(strings = {"naïve", "a\"b", "日本語", "x"}) + void writesValidJsonOnBothEncodings(String name) { + var fieldName = io.suboptimal.buffjson.internal.typed.FieldName.of(name); + for (boolean utf8 : new boolean[]{true, false}) { + try (JSONWriter jw = utf8 ? JSONWriter.ofUTF8() : JSONWriter.of()) { + jw.startObject(); + fieldName.writeTo(jw); + jw.writeInt32(1); + jw.endObject(); + // Round-trip through a parser: the name must survive intact. + assertEquals(name, com.alibaba.fastjson2.JSON.parseObject(jw.toString()).keySet().iterator().next(), + "utf8=" + utf8 + " json=" + jw); + } + } + } + } +} diff --git a/buff-json/CLAUDE.md b/buff-json/CLAUDE.md index e596d03..9cd004b 100644 --- a/buff-json/CLAUDE.md +++ b/buff-json/CLAUDE.md @@ -24,8 +24,12 @@ io.suboptimal.buffjson.internal/ ProtobufMessageWriter.java # Three-tier dispatch in writeFields(): codec-holder → typed-accessor → reflection ProtobufMessageReader.java # Stateful instance holding TypeRegistry + useGenerated. Main deserialization logic. GeneratedDecoderRegistry.java # Simple ConcurrentHashMap cache for descriptor-only decode path - MessageSchema.java # Cached FieldInfo[] per Descriptor (reflection path); each FieldInfo carries - # both char[] nameWithColon and byte[] nameWithColonUtf8 for UTF-8 dispatch + FieldNames.java # Packs a jsonName into the long/int words fastjson2's writeNameRaw family takes + # (native-order reads out of the literal `"name":`; offset 1 for lengths 8 and 16). + # Same constants serve UTF-8 and UTF-16 writers. Also builds the char[]/byte[] + # fallbacks, gates them via isRawWritable, and escapes names for generated source. + MessageSchema.java # Cached FieldInfo[] per Descriptor (reflection path); each FieldInfo carries a + # FieldName plus the pre-resolved map key/value FieldDescriptors FieldWriter.java # Type-dispatched value writing (scalars, maps, repeated) for the reflection path. # Float/double NaN/Inf branches reordered isFinite-first. FieldReader.java # Type-dispatched value reading (scalars, maps, repeated) @@ -35,7 +39,10 @@ io.suboptimal.buffjson.internal/ # writeUnsignedLongString() — zero-alloc unsigned int64 formatting. io.suboptimal.buffjson.internal.typed/ - FieldName.java # Record (char[] chars, byte[] utf8). writeTo(JSONWriter) dispatches on isUTF8(). + FieldName.java # Pre-encoded name shared by the typed and reflection paths. writeTo(JSONWriter) + # dispatches on isUTF8() -> writeNameRaw(byte[]/char[]); a json_name that is not + # raw-writable (non-ASCII/escapable) goes via writeName(String) + writeColon(). + # The word-packed writeNameRaw path is codegen-only — see the javadoc. TypedFieldAccessor.java # Sealed interface, ~20 record variants (IntAccessor, LongAccessor, ..., # PresenceMessageAccessor, RepeatedIntAccessor, RepeatedEnumAccessor, # MapAccessor, TypedMapAccessor, etc.). Each variant holds pre-bound @@ -52,7 +59,7 @@ io.suboptimal.buffjson.internal.typed/ ## Serialization Flow (hot path) 1. `BuffJsonEncoder.encode(message)`: - - Creates `JSONWriter` directly via `JSONWriter.of()` (bypasses fastjson2 module dispatch). + - Creates `JSONWriter` directly via `JSONWriter.of(writeContext)` (bypasses fastjson2 module dispatch). `writeContext` is one `JSONFactory.createWriteContext()` per encoder — `JSONWriter.of()` would otherwise allocate a fresh `Context` per call for configuration that never changes. - Reuses cached `ProtobufMessageWriter(typeRegistry, useGenerated, useTyped)` (volatile field on encoder; invalidated on setters). - Calls `writer.writeMessage(jsonWriter, message)`. 2. `ProtobufMessageWriter.writeFields(jsonWriter, message)` — three-tier dispatch: @@ -61,21 +68,27 @@ io.suboptimal.buffjson.internal.typed/ - Nested messages call other encoders directly via `INSTANCE.writeFields(jw, msg, writer)` (no registry, no instanceof per nested). - Timestamp/Duration fields call `writeTimestampDirect()`/`writeDurationDirect()`. - Enum fields use pre-cached `String[]` name arrays. - - Field-name writes dispatch on `boolean utf8 = jsonWriter.isUTF8()` hoisted at the top of `writeFields`. + - Field-name writes are `jsonWriter.writeNameRaw(NAME_X_W0[, ...])` — the name is already in machine words, so no `isUTF8()` branch and no `arraycopy`. The `boolean utf8` local and the `char[]`/`byte[]` constants are emitted only when some field has an unpackable `json_name`. + - Repeated numeric/bool fields narrow to `Internal.IntList`/`LongList`/`DoubleList`/`FloatList`/`BooleanList` and read primitives (no per-element boxing); repeated strings go through `jsonWriter.writeString(values)` in one call. + - Wrapper WKT fields (`Int32Value`, `StringValue`, …) emit the typed `getValue()` write directly. - Returns — never falls through. - **Tier 2 — Typed-accessor** (if `useTyped && !(message instanceof DynamicMessage)`): - `TypedMessageSchema.forMessage(descriptor, msg.getClass()).writeFields(jw, msg, this)` → LambdaMetafactory-bound typed getters. - First call per Descriptor: `TypedFieldAccessorFactory.create(...)` discovers `getXxx`/`hasXxx`/`getXxxList`/`getXxxValueList`/`getXxxMap`/`getXxxValueMap` by name reflection, then binds via `LambdaMetafactory.metafactory(...)` to `ToIntFunction`, `ToLongFunction`, `Predicate`, `Function`, etc. Builds a single `TypedFieldAccessor[]` in field-number order, with each oneof represented by an `OneofAccessor` placed at its first-declared member (so output order matches `JsonFormat`). Cached. - On any failure (e.g., `DynamicMessage`, custom protoc, missing accessor), returns `null` — schema goes to `FAILED` sentinel; falls through to Tier 3. - **Getter-name resolution must match protoc exactly** or binding fails and the whole message silently drops to Tier 3. Two easy-to-miss cases: (1) `float` getters — pass the **direct** `(Msg)float` handle to `metafactory` and let it widen `float`→`double`; pre-adapting with `explicitCastArguments` yields a non-direct handle metafactory rejects (this had been sinking every float-containing message to reflection). (2) digit-containing field names — `toCamelCase` capitalizes after a digit (`field0name5` → `getField0Name5`), matching protobuf. + - Repeated numeric/bool accessors (`RepeatedInt/Long/Double/Float/BoolAccessor`) narrow to the matching `Internal.*List` and read primitives, so no element is boxed; `RepeatedStringAccessor` hands the whole list to `jw.writeString(List)`. - Enum accessors hold the dense name array **and** the `EnumDescriptor`: the array is the fast path; negative/sparse numbers fall back to `findValueByNumber` (so `NEG = -1` → name); `NullValue` enums write JSON `null`. - Returns once schema runs successfully. - **Tier 3 — Pure reflection** (fallback): - Iterates cached `MessageSchema.FieldInfo[]` (no `getAllFields()` TreeMap). - `Object value = message.getField(fd)` (boxes primitives). - `FieldWriter.writeValue(jw, fd, value, this)` dispatches on `JavaType`. - - Field-name writes use `nameWithColon` (UTF-16) or `nameWithColonUtf8` (UTF-8) per the hoisted `utf8` local. -3. For MESSAGE fields in any path: `WellKnownTypes.isWellKnownType()` check first, then recurses via `writer.writeMessage()` (which re-enters the three-tier dispatch). + - For fields with presence, `hasField` is checked **before** `getField` — the old order paid a boxed reflective read for every absent optional field and threw the value away. + - Map fields pass `FieldInfo.mapKeyDescriptor()`/`mapValueDescriptor()` into `FieldWriter.writeMap`, which no longer calls `findFieldByName("key"/"value")` per map write. + - Field-name writes go through `FieldInfo.name().writeTo(jw)` (`FieldName` — pre-encoded `char[]`/`byte[]`, dispatched on `isUTF8()`; the word-packed path is codegen-only, see `FieldName`'s javadoc for the measurement). +3. For MESSAGE fields in any path: `WellKnownTypes.isWellKnownType()` check first, then recurses via `writer.writeMessage()` (which re-enters the three-tier dispatch). Codegen skips this for Timestamp, Duration and the nine wrappers, whose writes are emitted inline. +4. `Struct`/`Value`/`ListValue` take a typed fast path when the message is a compiled `com.google.protobuf.Struct`/`Value`/`ListValue`: `getFieldsMap()` instead of materializing the synthetic MapEntry list, and `getKindCase()` (an int switch) instead of `getOneofs().get(0)` — which allocates an `Arrays.asList` + `unmodifiableList` wrapper pair on *every* call — plus `getOneofFieldDescriptor()` and a switch on the field's String name. The reflective path stays for `DynamicMessage`, with the map-entry key/value descriptors hoisted out of the per-entry loop and the `kind` oneof cached per Descriptor. ## Settings Flow (no ThreadLocals) diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/BuffJsonEncoder.java b/buff-json/src/main/java/io/suboptimal/buffjson/BuffJsonEncoder.java index 615ea4f..24c81c9 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/BuffJsonEncoder.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/BuffJsonEncoder.java @@ -3,6 +3,7 @@ import java.io.IOException; import java.io.OutputStream; +import com.alibaba.fastjson2.JSONFactory; import com.alibaba.fastjson2.JSONWriter; import com.alibaba.fastjson2.modules.ObjectWriterModule; import com.google.protobuf.Message; @@ -46,6 +47,14 @@ public final class BuffJsonEncoder { private boolean useTypedAccessors = true; private volatile ProtobufMessageWriter cachedWriter; + /** + * Shared write context. {@code JSONWriter.of()} otherwise allocates a fresh + * {@link JSONWriter.Context} per call to carry configuration that never changes + * between encodes. The context is read-only during writing, so one per encoder + * is safe to share across threads. + */ + private final JSONWriter.Context writeContext = JSONFactory.createWriteContext(); + BuffJsonEncoder() { } @@ -90,7 +99,7 @@ public boolean getTypedAccessors() { */ public String encode(MessageOrBuilder message) { Message msg = toMessage(message); - try (JSONWriter writer = JSONWriter.of()) { + try (JSONWriter writer = JSONWriter.of(writeContext)) { messageWriter().writeMessage(writer, msg); return writer.toString(); } @@ -101,7 +110,7 @@ public String encode(MessageOrBuilder message) { */ public byte[] encodeToBytes(MessageOrBuilder message) { Message msg = toMessage(message); - try (JSONWriter writer = JSONWriter.ofUTF8()) { + try (JSONWriter writer = JSONWriter.ofUTF8(writeContext)) { messageWriter().writeMessage(writer, msg); return writer.getBytes(); } @@ -113,7 +122,7 @@ public byte[] encodeToBytes(MessageOrBuilder message) { */ public void encode(MessageOrBuilder message, OutputStream out) throws IOException { Message msg = toMessage(message); - try (JSONWriter writer = JSONWriter.ofUTF8()) { + try (JSONWriter writer = JSONWriter.ofUTF8(writeContext)) { messageWriter().writeMessage(writer, msg); writer.flushTo(out); } diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldNames.java b/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldNames.java new file mode 100644 index 0000000..ea979e3 --- /dev/null +++ b/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldNames.java @@ -0,0 +1,233 @@ +package io.suboptimal.buffjson.internal; + +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; +import java.nio.ByteOrder; + +/** + * Packs a JSON field name into the {@code long}/{@code int} words consumed by + * fastjson2's {@code JSONWriter.writeName2Raw(long)} … + * {@code writeName16Raw(long, long)} family. + * + *

Why

+ * + * The pre-encoded {@code writeNameRaw(byte[])} / {@code writeNameRaw(char[])} + * path still costs a static field load, an array length read, and an + * {@code arraycopy} of the name bytes per field — plus an {@code isUTF8()} + * branch to pick the right array. The {@code writeNameNRaw} family instead + * takes the name pre-packed into machine words and stores it with one or two + * {@code Unsafe.putLong} instructions. It is what fastjson2's own ASM/compiled + * writers use, and it is the main remaining gap to the fastjson2 POJO ceiling. + * + *

Layout

+ * + * The words are simply machine-word reads out of the literal {@code "name":} + * text, zero-padded. For a name of length {@code n}: + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + *
Packed-word layout per name length
ncallwords read from {@code "name":} at
2–7{@code writeNameRaw(long)}{@code long} @ 0
8{@code writeName8Raw(long)}{@code long} @ 1
9{@code writeName9Raw(long, int)}{@code long} @ 0, {@code int} @ 8
10–15{@code writeNameRaw(long, long)}{@code long} @ 0, {@code long} @ 8
16{@code writeName16Raw(long, long)}{@code long} @ 1, {@code long} @ 9
+ * + *

+ * The offset-1 cases (8 and 16) are the two lengths where fastjson2 writes the + * opening quote separately and stores only the name body. The words are read in + * {@linkplain ByteOrder#nativeOrder() native byte order} because fastjson2 + * stores them with {@code Unsafe.putLong}, so the same constants are correct on + * little- and big-endian hosts alike. + * + *

+ * Both encodings share the constants. {@code JSONWriterUTF16} widens the + * same packed ASCII bytes to {@code char}s on the way out, so a packed name + * needs no {@code isUTF8()} branch at all — unlike the {@code byte[]}/ + * {@code char[]} pair it replaces. + * + *

Applicability

+ * + * {@link #isPackable(String)} gates the fast path: names must be 2–16 + * characters of printable ASCII with nothing needing JSON escaping. Protobuf + * field names are {@code [A-Za-z_][A-Za-z0-9_]*} and {@code getJsonName()} + * lowerCamelCases them, so essentially every field qualifies; an explicit + * {@code [json_name = "…"]} carrying non-ASCII or a quote falls back to the + * {@code writeNameRaw} arrays, or — when even those cannot represent it, see + * {@link #isRawWritable} — to {@code writeName(String)}. + */ +public final class FieldNames { + + /** Longest name the {@code writeNameNRaw} family covers. */ + public static final int MAX_PACKED_LENGTH = 16; + + private static final VarHandle LONG_VIEW = MethodHandles.byteArrayViewVarHandle(long[].class, + ByteOrder.nativeOrder()); + + private static final VarHandle INT_VIEW = MethodHandles.byteArrayViewVarHandle(int[].class, + ByteOrder.nativeOrder()); + + private FieldNames() { + } + + /** + * Returns whether {@code jsonName} can use the packed {@code writeNameNRaw} + * fast path — 2–16 characters, printable ASCII, no JSON escaping needed. + */ + public static boolean isPackable(String jsonName) { + int n = jsonName.length(); + if (n < 2 || n > MAX_PACKED_LENGTH) { + return false; + } + return isRawWritable(jsonName); + } + + /** + * Returns whether {@code jsonName} can be written as raw pre-encoded bytes or + * chars at all — printable ASCII with nothing needing JSON escaping, any + * length. + * + *

+ * Protobuf field names always qualify, but an explicit + * {@code [json_name = "…"]} is an arbitrary string: it may carry non-ASCII + * (which {@link #nameWithColonBytes} cannot encode) or a + * quote/backslash/control character (which would break out of the JSON string). + * Those names must go through {@code JSONWriter.writeName(String)} + + * {@code writeColon()}, which escapes and transcodes properly. + */ + public static boolean isRawWritable(String jsonName) { + for (int i = 0; i < jsonName.length(); i++) { + char c = jsonName.charAt(i); + if (c < 0x20 || c > 0x7e || c == '"' || c == '\\') { + return false; + } + } + return true; + } + + /** The first packed word — the {@code long} argument of every variant. */ + public static long packedWord0(String jsonName) { + checkPackable(jsonName); + return (long) LONG_VIEW.get(quotedName(jsonName), wordOffset(jsonName.length())); + } + + /** + * The second packed word — the trailing {@code long} of the 10–16 variants. + */ + public static long packedWord1(String jsonName) { + checkPackable(jsonName); + return (long) LONG_VIEW.get(quotedName(jsonName), wordOffset(jsonName.length()) + 8); + } + + /** The trailing {@code int} of the 9-character variant. */ + public static int packedTailInt(String jsonName) { + checkPackable(jsonName); + return (int) INT_VIEW.get(quotedName(jsonName), wordOffset(jsonName.length()) + 8); + } + + /** + * Pre-computes {@code "fieldName":} as a char array for the UTF-16 + * {@link com.alibaba.fastjson2.JSONWriter#writeNameRaw(char[])} fallback. Only + * valid for {@linkplain #isRawWritable raw-writable} names. + */ + public static char[] nameWithColonChars(String jsonName) { + char[] chars = new char[jsonName.length() + 3]; + chars[0] = '"'; + jsonName.getChars(0, jsonName.length(), chars, 1); + chars[jsonName.length() + 1] = '"'; + chars[jsonName.length() + 2] = ':'; + return chars; + } + + /** + * Pre-computes {@code "fieldName":} as a byte array for the UTF-8 + * {@link com.alibaba.fastjson2.JSONWriter#writeNameRaw(byte[])} fallback. Only + * valid for {@linkplain #isRawWritable raw-writable} names — the byte cast + * below would truncate anything else. + */ + public static byte[] nameWithColonBytes(String jsonName) { + byte[] bytes = new byte[jsonName.length() + 3]; + bytes[0] = '"'; + for (int i = 0; i < jsonName.length(); i++) { + bytes[i + 1] = (byte) jsonName.charAt(i); + } + bytes[jsonName.length() + 1] = '"'; + bytes[jsonName.length() + 2] = ':'; + return bytes; + } + + /** + * Escapes {@code jsonName} for embedding in generated Java source. Only needed + * for names that are not {@linkplain #isRawWritable raw-writable} — an explicit + * {@code json_name} carrying a quote, a backslash or a newline would otherwise + * emit source that does not compile. + */ + public static String javaStringLiteral(String jsonName) { + StringBuilder sb = new StringBuilder(jsonName.length() + 8).append('"'); + for (int i = 0; i < jsonName.length(); i++) { + char c = jsonName.charAt(i); + if (c == '"' || c == '\\') { + sb.append('\\').append(c); + } else if (c >= 0x20 && c <= 0x7e) { + sb.append(c); + } else { + sb.append(String.format("\\u%04x", (int) c)); + } + } + return sb.append('"').toString(); + } + + /** + * {@code "name":} zero-padded to 32 bytes so the word reads at offsets up to 17 + * stay in bounds. + */ + private static byte[] quotedName(String jsonName) { + byte[] buf = new byte[32]; + buf[0] = '"'; + for (int i = 0; i < jsonName.length(); i++) { + buf[i + 1] = (byte) jsonName.charAt(i); + } + buf[jsonName.length() + 1] = '"'; + buf[jsonName.length() + 2] = ':'; + return buf; + } + + /** + * Offset into the quoted name at which the first packed word starts. Lengths 8 + * and 16 are the two variants where fastjson2 emits the opening quote itself. + */ + private static int wordOffset(int length) { + return (length == 8 || length == 16) ? 1 : 0; + } + + private static void checkPackable(String jsonName) { + if (!isPackable(jsonName)) { + throw new IllegalArgumentException("Field name is not packable: " + jsonName); + } + } +} diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldWriter.java b/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldWriter.java index 8d6539d..1a557ee 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldWriter.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldWriter.java @@ -121,7 +121,7 @@ public static void writeValue(JSONWriter jsonWriter, FieldDescriptor fd, Object * Writes a float value, handling NaN and Infinity as quoted strings per proto3 * JSON spec. Also used by {@link WellKnownTypes} for FloatValue wrapper. */ - static void writeFloatValue(JSONWriter jsonWriter, float value) { + public static void writeFloatValue(JSONWriter jsonWriter, float value) { if (Float.isFinite(value)) { jsonWriter.writeFloat(value); } else if (Float.isNaN(value)) { @@ -135,7 +135,7 @@ static void writeFloatValue(JSONWriter jsonWriter, float value) { * Writes a double value, handling NaN and Infinity as quoted strings per proto3 * JSON spec. Also used by {@link WellKnownTypes} for DoubleValue wrapper. */ - static void writeDoubleValue(JSONWriter jsonWriter, double value) { + public static void writeDoubleValue(JSONWriter jsonWriter, double value) { if (Double.isFinite(value)) { jsonWriter.writeDouble(value); } else if (Double.isNaN(value)) { @@ -171,18 +171,27 @@ public static void writeRepeated(JSONWriter jsonWriter, FieldDescriptor fd, List * {@link com.google.protobuf.MapEntry} and * {@link com.google.protobuf.DynamicMessage} map entries. */ - public static void writeMap(JSONWriter jsonWriter, FieldDescriptor valueDescriptor, List entries, - ProtobufMessageWriter writer) { - var entryDesc = valueDescriptor.getContainingType(); - var keyFd = entryDesc.findFieldByName("key"); - var valueFd = entryDesc.findFieldByName("value"); + public static void writeMap(JSONWriter jsonWriter, FieldDescriptor keyDescriptor, FieldDescriptor valueDescriptor, + List entries, ProtobufMessageWriter writer) { jsonWriter.startObject(); for (Object entry : entries) { Message entryMsg = (Message) entry; - jsonWriter.writeName(entryMsg.getField(keyFd).toString()); + jsonWriter.writeName(entryMsg.getField(keyDescriptor).toString()); jsonWriter.writeColon(); - writeValue(jsonWriter, valueFd, entryMsg.getField(valueFd), writer); + writeValue(jsonWriter, valueDescriptor, entryMsg.getField(valueDescriptor), writer); } jsonWriter.endObject(); } + + /** + * Writes a map field, resolving the synthetic {@code key} field from the value + * descriptor's containing type. Prefer the overload that takes both descriptors + * — callers with a cached schema already have them, and {@code findFieldByName} + * is a hash lookup per call. + */ + public static void writeMap(JSONWriter jsonWriter, FieldDescriptor valueDescriptor, List entries, + ProtobufMessageWriter writer) { + var keyFd = valueDescriptor.getContainingType().findFieldByName("key"); + writeMap(jsonWriter, keyFd, valueDescriptor, entries, writer); + } } diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/internal/MessageSchema.java b/buff-json/src/main/java/io/suboptimal/buffjson/internal/MessageSchema.java index f230582..9723a0a 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/internal/MessageSchema.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/internal/MessageSchema.java @@ -7,6 +7,8 @@ import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; +import io.suboptimal.buffjson.internal.typed.FieldName; + /** * Cached metadata for a protobuf message type, built from its * {@link Descriptor}. @@ -73,55 +75,26 @@ public FieldInfo fieldByJsonName(String name) { public static final class FieldInfo { private final FieldDescriptor descriptor; private final String jsonName; - private final char[] nameWithColon; - private final byte[] nameWithColonUtf8; + private final FieldName name; private final FieldDescriptor.JavaType javaType; private final boolean isRepeated; private final boolean isMapField; private final boolean hasPresence; + private final FieldDescriptor mapKeyDescriptor; private final FieldDescriptor mapValueDescriptor; FieldInfo(FieldDescriptor fd) { this.descriptor = fd; this.jsonName = fd.getJsonName(); - this.nameWithColon = buildNameWithColon(this.jsonName); - this.nameWithColonUtf8 = buildNameWithColonUtf8(this.jsonName); + this.name = FieldName.of(this.jsonName); this.javaType = fd.getJavaType(); this.isRepeated = fd.isRepeated(); this.isMapField = fd.isMapField(); this.hasPresence = fd.hasPresence(); + this.mapKeyDescriptor = fd.isMapField() ? fd.getMessageType().findFieldByName("key") : null; this.mapValueDescriptor = fd.isMapField() ? fd.getMessageType().findFieldByName("value") : null; } - /** - * Pre-computes {@code "fieldName":} as a char array for the UTF-16 - * {@link com.alibaba.fastjson2.JSONWriter#writeNameRaw(char[])} path. Protobuf - * JSON field names are always ASCII. - */ - private static char[] buildNameWithColon(String name) { - char[] chars = new char[name.length() + 3]; - chars[0] = '"'; - name.getChars(0, name.length(), chars, 1); - chars[name.length() + 1] = '"'; - chars[name.length() + 2] = ':'; - return chars; - } - - /** - * Pre-computes {@code "fieldName":} as a byte array for the UTF-8 - * {@link com.alibaba.fastjson2.JSONWriter#writeNameRaw(byte[])} path, avoiding - * char→byte transcoding per field write. ASCII-only. - */ - private static byte[] buildNameWithColonUtf8(String name) { - byte[] bytes = new byte[name.length() + 3]; - bytes[0] = '"'; - for (int i = 0; i < name.length(); i++) - bytes[i + 1] = (byte) name.charAt(i); - bytes[name.length() + 1] = '"'; - bytes[name.length() + 2] = ':'; - return bytes; - } - public FieldDescriptor descriptor() { return descriptor; } @@ -130,12 +103,17 @@ public String jsonName() { return jsonName; } + /** Pre-encoded {@code "fieldName":} in both writer encodings. */ + public FieldName name() { + return name; + } + public char[] nameWithColon() { - return nameWithColon; + return name.chars(); } public byte[] nameWithColonUtf8() { - return nameWithColonUtf8; + return name.utf8(); } public FieldDescriptor.JavaType javaType() { @@ -154,6 +132,14 @@ public boolean hasPresence() { return hasPresence; } + /** + * The synthetic map-entry {@code key} field, resolved once here so the write + * path never calls {@code findFieldByName} per map write. + */ + public FieldDescriptor mapKeyDescriptor() { + return mapKeyDescriptor; + } + public FieldDescriptor mapValueDescriptor() { return mapValueDescriptor; } diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/internal/ProtobufMessageWriter.java b/buff-json/src/main/java/io/suboptimal/buffjson/internal/ProtobufMessageWriter.java index f6e43dd..648a596 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/internal/ProtobufMessageWriter.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/internal/ProtobufMessageWriter.java @@ -115,7 +115,6 @@ void writeFields(JSONWriter jsonWriter, Message message) { var schema = MessageSchema.forDescriptor(message.getDescriptorForType()); var fields = schema.fields(); - boolean utf8 = jsonWriter.isUTF8(); for (var fieldInfo : fields) { FieldDescriptor fd = fieldInfo.descriptor(); @@ -124,35 +123,32 @@ void writeFields(JSONWriter jsonWriter, Message message) { List entries = (List) message.getField(fd); if (entries.isEmpty()) continue; - writeName(jsonWriter, fieldInfo, utf8); - FieldWriter.writeMap(jsonWriter, fieldInfo.mapValueDescriptor(), entries, this); + fieldInfo.name().writeTo(jsonWriter); + FieldWriter.writeMap(jsonWriter, fieldInfo.mapKeyDescriptor(), fieldInfo.mapValueDescriptor(), entries, + this); } else if (fieldInfo.isRepeated()) { List values = (List) message.getField(fd); if (values.isEmpty()) continue; - writeName(jsonWriter, fieldInfo, utf8); + fieldInfo.name().writeTo(jsonWriter); FieldWriter.writeRepeated(jsonWriter, fd, values, this); + } else if (fieldInfo.hasPresence()) { + // hasField first: getField on an absent field boxes a value we would + // throw away (and materializes a default instance for message fields). + if (!message.hasField(fd)) + continue; + fieldInfo.name().writeTo(jsonWriter); + FieldWriter.writeValue(jsonWriter, fd, message.getField(fd), this); } else { Object value = message.getField(fd); - if (fieldInfo.hasPresence()) { - if (!message.hasField(fd)) - continue; - } else if (isDefaultValue(fieldInfo, value)) { + if (isDefaultValue(fieldInfo, value)) continue; - } - writeName(jsonWriter, fieldInfo, utf8); + fieldInfo.name().writeTo(jsonWriter); FieldWriter.writeValue(jsonWriter, fd, value, this); } } } - private static void writeName(JSONWriter jsonWriter, MessageSchema.FieldInfo fieldInfo, boolean utf8) { - if (utf8) - jsonWriter.writeNameRaw(fieldInfo.nameWithColonUtf8()); - else - jsonWriter.writeNameRaw(fieldInfo.nameWithColon()); - } - private static boolean isDefaultValue(MessageSchema.FieldInfo fieldInfo, Object value) { return switch (fieldInfo.javaType()) { case INT -> (int) value == 0; diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/internal/WellKnownTypes.java b/buff-json/src/main/java/io/suboptimal/buffjson/internal/WellKnownTypes.java index 8eebd64..9ae1288 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/internal/WellKnownTypes.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/internal/WellKnownTypes.java @@ -15,6 +15,7 @@ import com.google.protobuf.*; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; +import com.google.protobuf.Descriptors.OneofDescriptor; /** * Specialized JSON serialization for protobuf @@ -91,6 +92,9 @@ public final class WellKnownTypes { */ private static final ConcurrentHashMap WKT_FIELD_CACHE = new ConcurrentHashMap<>(); + /** Cached {@code kind} oneof per {@code google.protobuf.Value} descriptor. */ + private static final ConcurrentHashMap VALUE_KIND_CACHE = new ConcurrentHashMap<>(); + private WellKnownTypes() { } @@ -379,13 +383,28 @@ private static void writeFieldMask(JSONWriter jsonWriter, Message message) { } private static void writeStruct(JSONWriter jsonWriter, Message message, ProtobufMessageWriter writer) { + // Compiled Struct: iterate the real map instead of materializing the + // synthetic MapEntry list and pulling key/value back out through getField(). + if (message instanceof Struct struct) { + jsonWriter.startObject(); + for (var entry : struct.getFieldsMap().entrySet()) { + jsonWriter.writeName(entry.getKey()); + jsonWriter.writeColon(); + writeValue(jsonWriter, entry.getValue(), writer); + } + jsonWriter.endObject(); + return; + } + var fields = getFields(message, "fields"); @SuppressWarnings("unchecked") List entries = (List) message.getField(fields[0]); jsonWriter.startObject(); + // Every entry shares the same map-entry descriptor, so resolve key/value once + // rather than hitting the descriptor cache per entry. + FieldDescriptor[] entryFields = entries.isEmpty() ? null : getFields(entries.get(0), "key", "value"); for (var entry : entries) { - var entryFields = getFields(entry, "key", "value"); String key = (String) entry.getField(entryFields[0]); Message value = (Message) entry.getField(entryFields[1]); jsonWriter.writeName(key); @@ -396,8 +415,23 @@ private static void writeStruct(JSONWriter jsonWriter, Message message, Protobuf } private static void writeValue(JSONWriter jsonWriter, Message message, ProtobufMessageWriter writer) { + // Compiled Value: getKindCase() is an int switch on the oneof case, replacing + // a getOneofs() call (which allocates two list wrappers per invocation), a + // getOneofFieldDescriptor() scan, and a switch on the field's String name. + if (message instanceof Value value) { + switch (value.getKindCase()) { + case NULL_VALUE, KIND_NOT_SET -> jsonWriter.writeNull(); + case NUMBER_VALUE -> jsonWriter.writeDouble(value.getNumberValue()); + case STRING_VALUE -> jsonWriter.writeString(value.getStringValue()); + case BOOL_VALUE -> jsonWriter.writeBool(value.getBoolValue()); + case STRUCT_VALUE -> writeStruct(jsonWriter, value.getStructValue(), writer); + case LIST_VALUE -> writeListValue(jsonWriter, value.getListValue(), writer); + } + return; + } + var desc = message.getDescriptorForType(); - var kindOneof = desc.getOneofs().get(0); + var kindOneof = kindOneof(desc); var activeField = message.getOneofFieldDescriptor(kindOneof); if (activeField == null) { @@ -416,12 +450,24 @@ private static void writeValue(JSONWriter jsonWriter, Message message, ProtobufM } private static void writeListValue(JSONWriter jsonWriter, Message message, ProtobufMessageWriter writer) { + if (message instanceof ListValue listValue) { + var typedValues = listValue.getValuesList(); + jsonWriter.startArray(); + for (int i = 0, n = typedValues.size(); i < n; i++) { + if (i > 0) + jsonWriter.writeComma(); + writeValue(jsonWriter, typedValues.get(i), writer); + } + jsonWriter.endArray(); + return; + } + var fields = getFields(message, "values"); @SuppressWarnings("unchecked") List values = (List) message.getField(fields[0]); jsonWriter.startArray(); - for (int i = 0; i < values.size(); i++) { + for (int i = 0, n = values.size(); i < n; i++) { if (i > 0) jsonWriter.writeComma(); writeValue(jsonWriter, values.get(i), writer); @@ -429,6 +475,20 @@ private static void writeListValue(JSONWriter jsonWriter, Message message, Proto jsonWriter.endArray(); } + /** + * Cached {@code kind} oneof for a {@code google.protobuf.Value} descriptor. + * {@code Descriptor.getOneofs()} wraps the backing array in a fresh + * {@code Arrays.asList} + {@code unmodifiableList} on every call — two + * allocations per Value written. + */ + private static OneofDescriptor kindOneof(Descriptor desc) { + var cached = VALUE_KIND_CACHE.get(desc); + if (cached != null) { + return cached; + } + return VALUE_KIND_CACHE.computeIfAbsent(desc, d -> d.getOneofs().get(0)); + } + private static void writeWrapper(JSONWriter jsonWriter, Message message, ProtobufMessageWriter writer) { // Wrapper types have a single "value" field — delegate to FieldWriter // which already handles unsigned formatting, NaN/Infinity, Base64, etc. diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/internal/typed/FieldName.java b/buff-json/src/main/java/io/suboptimal/buffjson/internal/typed/FieldName.java index d9aea83..e0a1ddd 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/internal/typed/FieldName.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/internal/typed/FieldName.java @@ -2,16 +2,49 @@ import com.alibaba.fastjson2.JSONWriter; +import io.suboptimal.buffjson.internal.FieldNames; + /** - * Pre-encoded field name in both UTF-16 (char[]) and UTF-8 (byte[]) forms. - * Dispatches to the optimal variant based on the JSONWriter type. + * Pre-encoded field name in both UTF-16 ({@code char[]}) and UTF-8 + * ({@code byte[]}) forms, for the runtime write paths (typed accessors and pure + * reflection). Dispatches on the writer's encoding. + * + *

+ * Why not the word-packed {@code writeNameRaw} path here? Generated + * encoders use it (see {@link FieldNames}) because they know each name's length + * at code-generation time and emit the matching call directly. A runtime holder + * cannot: it has to {@code switch} on the length, and that switch is an + * indirect branch inside the hottest code, keyed on a per-field instance value. + * Measured on this repo's benchmarks, that costs more than the + * {@code arraycopy} it saves — flat on {@code SimpleMessage} (few distinct name + * lengths) and ~10% down on {@code ComplexMessage} (fifteen fields, nine + * distinct lengths). So the packed path stays where it is free of dispatch, and + * the runtime paths keep the pre-encoded arrays. */ -public record FieldName(char[] chars, byte[] utf8) { +public record FieldName(char[] chars, byte[] utf8, String escaping) { + + /** + * @param jsonName + * the field's JSON name — normally protobuf's lowerCamelCase form, + * but an explicit {@code [json_name = "…"]} can be any string + */ + public static FieldName of(String jsonName) { + if (!FieldNames.isRawWritable(jsonName)) { + // Non-ASCII or escapable: the pre-encoded arrays cannot represent it, so let + // fastjson2 escape and transcode it per write. + return new FieldName(null, null, jsonName); + } + return new FieldName(FieldNames.nameWithColonChars(jsonName), FieldNames.nameWithColonBytes(jsonName), null); + } public void writeTo(JSONWriter jw) { - if (jw.isUTF8()) + if (escaping != null) { + jw.writeName(escaping); + jw.writeColon(); + } else if (jw.isUTF8()) { jw.writeNameRaw(utf8); - else + } else { jw.writeNameRaw(chars); + } } } diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/internal/typed/TypedFieldAccessor.java b/buff-json/src/main/java/io/suboptimal/buffjson/internal/typed/TypedFieldAccessor.java index e11e23a..abff290 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/internal/typed/TypedFieldAccessor.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/internal/typed/TypedFieldAccessor.java @@ -12,6 +12,7 @@ import com.google.protobuf.Descriptors.EnumDescriptor; import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.Descriptors.OneofDescriptor; +import com.google.protobuf.Internal; import com.google.protobuf.Message; import io.suboptimal.buffjson.internal.FieldWriter; @@ -308,17 +309,33 @@ record RepeatedIntAccessor(Function> listGetter, boolean unsign @Override public void write(JSONWriter jw, Message msg, ProtobufMessageWriter writer) { List values = (List) (List) listGetter.apply(msg); - if (values.isEmpty()) + int n = values.size(); + if (n == 0) return; name.writeTo(jw); jw.startArray(); - for (int i = 0; i < values.size(); i++) { - if (i > 0) - jw.writeComma(); - if (unsigned) - jw.writeInt64(Integer.toUnsignedLong(values.get(i))); - else - jw.writeInt32(values.get(i)); + // protobuf-java backs repeated int32 with Internal.IntList; going through + // List.get(i) would box every element (Integer.valueOf allocates outside + // the -128..127 cache). + if (values instanceof Internal.IntList ints) { + for (int i = 0; i < n; i++) { + if (i > 0) + jw.writeComma(); + int v = ints.getInt(i); + if (unsigned) + jw.writeInt64(Integer.toUnsignedLong(v)); + else + jw.writeInt32(v); + } + } else { + for (int i = 0; i < n; i++) { + if (i > 0) + jw.writeComma(); + if (unsigned) + jw.writeInt64(Integer.toUnsignedLong(values.get(i))); + else + jw.writeInt32(values.get(i)); + } } jw.endArray(); } @@ -330,40 +347,130 @@ record RepeatedLongAccessor(Function> listGetter, boolean unsig @Override public void write(JSONWriter jw, Message msg, ProtobufMessageWriter writer) { List values = (List) (List) listGetter.apply(msg); - if (values.isEmpty()) + int n = values.size(); + if (n == 0) return; name.writeTo(jw); jw.startArray(); - for (int i = 0; i < values.size(); i++) { - if (i > 0) - jw.writeComma(); - if (unsigned) - WellKnownTypes.writeUnsignedLongString(jw, values.get(i)); - else - jw.writeString(values.get(i)); + if (values instanceof Internal.LongList longs) { + for (int i = 0; i < n; i++) { + if (i > 0) + jw.writeComma(); + long v = longs.getLong(i); + if (unsigned) + WellKnownTypes.writeUnsignedLongString(jw, v); + else + jw.writeString(v); + } + } else { + for (int i = 0; i < n; i++) { + if (i > 0) + jw.writeComma(); + if (unsigned) + WellKnownTypes.writeUnsignedLongString(jw, values.get(i)); + else + jw.writeString((long) values.get(i)); + } } jw.endArray(); } } @SuppressWarnings("unchecked") - record RepeatedStringAccessor(Function> listGetter, FieldName name) implements TypedFieldAccessor { + record RepeatedDoubleAccessor(Function> listGetter, FieldName name) implements TypedFieldAccessor { @Override public void write(JSONWriter jw, Message msg, ProtobufMessageWriter writer) { - List values = (List) (List) listGetter.apply(msg); - if (values.isEmpty()) + List values = (List) (List) listGetter.apply(msg); + int n = values.size(); + if (n == 0) return; name.writeTo(jw); jw.startArray(); - for (int i = 0; i < values.size(); i++) { - if (i > 0) - jw.writeComma(); - jw.writeString(values.get(i)); + if (values instanceof Internal.DoubleList doubles) { + for (int i = 0; i < n; i++) { + if (i > 0) + jw.writeComma(); + FieldWriter.writeDoubleValue(jw, doubles.getDouble(i)); + } + } else { + for (int i = 0; i < n; i++) { + if (i > 0) + jw.writeComma(); + FieldWriter.writeDoubleValue(jw, values.get(i)); + } + } + jw.endArray(); + } + } + + @SuppressWarnings("unchecked") + record RepeatedFloatAccessor(Function> listGetter, FieldName name) implements TypedFieldAccessor { + @Override + public void write(JSONWriter jw, Message msg, ProtobufMessageWriter writer) { + List values = (List) (List) listGetter.apply(msg); + int n = values.size(); + if (n == 0) + return; + name.writeTo(jw); + jw.startArray(); + if (values instanceof Internal.FloatList floats) { + for (int i = 0; i < n; i++) { + if (i > 0) + jw.writeComma(); + FieldWriter.writeFloatValue(jw, floats.getFloat(i)); + } + } else { + for (int i = 0; i < n; i++) { + if (i > 0) + jw.writeComma(); + FieldWriter.writeFloatValue(jw, values.get(i)); + } + } + jw.endArray(); + } + } + + @SuppressWarnings("unchecked") + record RepeatedBoolAccessor(Function> listGetter, FieldName name) implements TypedFieldAccessor { + @Override + public void write(JSONWriter jw, Message msg, ProtobufMessageWriter writer) { + List values = (List) (List) listGetter.apply(msg); + int n = values.size(); + if (n == 0) + return; + name.writeTo(jw); + jw.startArray(); + if (values instanceof Internal.BooleanList bools) { + for (int i = 0; i < n; i++) { + if (i > 0) + jw.writeComma(); + jw.writeBool(bools.getBoolean(i)); + } + } else { + for (int i = 0; i < n; i++) { + if (i > 0) + jw.writeComma(); + jw.writeBool(values.get(i)); + } } jw.endArray(); } } + @SuppressWarnings("unchecked") + record RepeatedStringAccessor(Function> listGetter, FieldName name) implements TypedFieldAccessor { + @Override + public void write(JSONWriter jw, Message msg, ProtobufMessageWriter writer) { + List values = (List) (List) listGetter.apply(msg); + if (values.isEmpty()) + return; + name.writeTo(jw); + // fastjson2 writes the whole array — brackets, commas and per-element + // escaping — in one call, with a single capacity check up front. + jw.writeString(values); + } + } + @SuppressWarnings("unchecked") record RepeatedMessageAccessor(Function> listGetter, FieldName name) implements TypedFieldAccessor { @@ -411,15 +518,15 @@ public void write(JSONWriter jw, Message msg, ProtobufMessageWriter writer) { // --- Map fields --- - record MapAccessor(Function> entriesGetter, FieldDescriptor mapValueDescriptor, - FieldName name) implements TypedFieldAccessor { + record MapAccessor(Function> entriesGetter, FieldDescriptor mapKeyDescriptor, + FieldDescriptor mapValueDescriptor, FieldName name) implements TypedFieldAccessor { @Override public void write(JSONWriter jw, Message msg, ProtobufMessageWriter writer) { List entries = entriesGetter.apply(msg); if (entries.isEmpty()) return; name.writeTo(jw); - FieldWriter.writeMap(jw, mapValueDescriptor, entries, writer); + FieldWriter.writeMap(jw, mapKeyDescriptor, mapValueDescriptor, entries, writer); } } diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/internal/typed/TypedFieldAccessorFactory.java b/buff-json/src/main/java/io/suboptimal/buffjson/internal/typed/TypedFieldAccessorFactory.java index bc904af..4dd07c7 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/internal/typed/TypedFieldAccessorFactory.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/internal/typed/TypedFieldAccessorFactory.java @@ -195,6 +195,9 @@ private static TypedFieldAccessor createRepeatedAccessor(FieldDescriptor fd, Cla return switch (fd.getJavaType()) { case INT -> new TypedFieldAccessor.RepeatedIntAccessor(listGetter, isUnsigned32(fd), name); case LONG -> new TypedFieldAccessor.RepeatedLongAccessor(listGetter, isUnsigned64(fd), name); + case DOUBLE -> new TypedFieldAccessor.RepeatedDoubleAccessor(listGetter, name); + case FLOAT -> new TypedFieldAccessor.RepeatedFloatAccessor(listGetter, name); + case BOOLEAN -> new TypedFieldAccessor.RepeatedBoolAccessor(listGetter, name); case STRING -> new TypedFieldAccessor.RepeatedStringAccessor(listGetter, name); case MESSAGE -> new TypedFieldAccessor.RepeatedMessageAccessor(listGetter, name); default -> new TypedFieldAccessor.RepeatedAccessor(listGetter, fd, name); @@ -226,7 +229,7 @@ private static TypedFieldAccessor createMapAccessor(FieldDescriptor fd, Class> entriesGetter = msg -> (List) msg.getField(fd); - return new TypedFieldAccessor.MapAccessor(entriesGetter, valueFd, name); + return new TypedFieldAccessor.MapAccessor(entriesGetter, keyFd, valueFd, name); } } @@ -340,19 +343,7 @@ static String toCamelCase(String name) { } static FieldName fieldName(String jsonName) { - char[] chars = new char[jsonName.length() + 3]; - chars[0] = '"'; - jsonName.getChars(0, jsonName.length(), chars, 1); - chars[jsonName.length() + 1] = '"'; - chars[jsonName.length() + 2] = ':'; - // Proto field names are always ASCII, so UTF-8 encoding is trivial - byte[] utf8 = new byte[jsonName.length() + 3]; - utf8[0] = '"'; - for (int i = 0; i < jsonName.length(); i++) - utf8[i + 1] = (byte) jsonName.charAt(i); - utf8[jsonName.length() + 1] = '"'; - utf8[jsonName.length() + 2] = ':'; - return new FieldName(chars, utf8); + return FieldName.of(jsonName); } private static boolean isUnsigned32(FieldDescriptor fd) { diff --git a/docs/encoding-performance.md b/docs/encoding-performance.md new file mode 100644 index 0000000..f8bda6d --- /dev/null +++ b/docs/encoding-performance.md @@ -0,0 +1,316 @@ +# Where encoding time still goes, and what is left to reclaim + +An audit of the three encode paths (codegen, typed-accessor, pure reflection) against what +fastjson2 and protobuf-java actually make available. Part 1 is what this branch changed and +measured. Part 2 is the ranked backlog, with the reasoning for each item so the next person +does not have to re-derive it. + +Everything here is about *encoding*. The decode path was not audited. + +## Method + +`./run-benchmarks.sh` numbers are not comparable across sessions on shared/virtualised hosts — +the baseline run for this audit showed score errors up to 40% of the score with `-f 1 -i 3`. All +figures below come from an interleaved A/B: two `benchmarks.jar` builds (before/after) from the +same reactor, run back to back on the same host with identical JMH settings +(`-f 3 -wi 3 -i 5 -r 1 -w 1`), `JsonFormat` baselines excluded to keep the run short. + +Treat single-digit-percent deltas here as noise. The items worth acting on were the ones that +showed up as double-digit, or that removed an allocation the `gc.alloc.rate.norm` profiler could +confirm. + +## Results + +Throughput, ops/s, higher is better. Score error was ±2–5% on both sides except where noted. + +| benchmark | before | after | delta | +|-----------------------------------|-----------:|-----------:|-------------------:| +| `SimpleMessage.compiledUtf16` | 9,874,885 | 11,071,259 | +12.1% | +| `SimpleMessage.compiledUtf8` | 11,474,893 | 13,021,993 | +13.5% | +| `SimpleMessage.runtimeUtf16` | 7,485,001 | 8,064,932 | +7.7% | +| `SimpleMessage.runtimeUtf8` | 8,870,673 | 9,099,641 | +2.6% | +| `ComplexMessage.buffJsonCompiled` | 749,489 | 773,347 | +3.2% | +| `ComplexMessage.buffJsonRuntime` | 646,093 | 662,785 | +2.6% | +| `Wkt.structCompiled` | 296,651 | 467,034 | +57.4% | +| `Wkt.structRuntime` | 292,599 | 481,153 | +64.4% | +| `Wkt.timestampCompiled` | 4,736,791 | 4,891,055 | +3.3% | +| `Wkt.timestampRuntime` | 2,839,048 | 2,833,593 | −0.2% | +| `RepeatedAndMap.repeatedCompiled` | 115,094 | 112,168 | −2.5% (±14% after) | +| `RepeatedAndMap.repeatedRuntime` | 94,399 | 94,978 | +0.6% | +| `RepeatedAndMap.mapCompiled` | 95,574 | 94,222 | −1.4% (±7% after) | +| `RepeatedAndMap.mapRuntime` | 76,305 | 76,494 | +0.2% | + +Allocation, `gc.alloc.rate.norm` B/op, lower is better: + +| benchmark | before | after | delta | +|-----------------------------------|-------:|------:|-------:| +| `SimpleMessage.compiledUtf16` | 296 | 208 | −88 | +| `SimpleMessage.compiledUtf8` | 272 | 184 | −88 | +| `SimpleMessage.runtimeUtf16` | 296 | 208 | −88 | +| `SimpleMessage.runtimeUtf8` | 272 | 184 | −88 | +| `ComplexMessage.buffJsonCompiled` | 1,532 | 1,444 | −88 | +| `ComplexMessage.buffJsonRuntime` | 1,396 | 1,308 | −88 | +| `Wkt.structCompiled` / `Runtime` | 1,305 | 648 | −657 | +| `Wkt.timestampCompiled` | 464 | 376 | −88 | +| `RepeatedAndMap.mapRuntime` | 8,021 | 6,727 | −1,294 | +| `RepeatedAndMap.repeated*` | 5,385 | 5,297 | −88 | + +The flat −88 B/op everywhere is the per-call `JSONWriter.Context`. For `SimpleMessage` that is 30% +of the total allocation — after the change the returned `String`/`byte[]` is essentially the only +thing allocated per encode. Budgets in `allocation-check.sh` were tightened to match. + +## Part 1 — Applied + +### 1. Word-packed field names (codegen) + +**The finding.** fastjson2's `JSONWriter` exposes a `writeName2Raw(long)` … +`writeName16Raw(long, long)` family that takes the field name *already packed into machine +words* and stores it with one or two `Unsafe.putLong` instructions. It is what fastjson2's own +ASM and `@JSONCompiled` writers use, and it was the single largest structural difference between +buff-json's generated encoders and the fastjson2 POJO ceiling that `CeilingBenchmark` measures. + +buff-json was using `writeNameRaw(byte[])` / `writeNameRaw(char[])`, which per field costs a +static array load, an array length read, an `arraycopy` of the name, **and** an `isUTF8()` branch +to choose between the two arrays. + +Two things make the replacement unusually clean: + +- The packed words are just native-order machine-word reads out of the literal `"name":` text, + zero-padded — offset 0 for every length except 8 and 16, where fastjson2 emits the opening + quote itself and the word holds only the name body. (Derived empirically for lengths 2–16 on + both writer encodings; `FieldNames` documents the table.) +- **`JSONWriterUTF16` consumes the same constants**, widening the packed ASCII to `char`s on the + way out. So a packed name needs no `isUTF8()` branch at all — the branch and the second array + disappear rather than moving. + +Protobuf field names are `[A-Za-z_][A-Za-z0-9_]*` and `getJsonName()` lowerCamelCases them, so +essentially every field is packable. `FieldNames.isPackable` gates on 2–16 printable ASCII with +nothing needing JSON escaping; longer or 1-character names fall back to the pre-encoded arrays. + +**Codegen only, deliberately.** Codegen emits the exact call +(`jsonWriter.writeName6Raw(NAME_SCORE_W0)`) because it knows each name's length at +code-generation time — no dispatch remains at all. The first attempt also routed the two runtime +paths through it, with `FieldName` switching on the packed length. That measured *worse*: flat on +`SimpleMessage` and **−10.8% on `ComplexMessage.buffJsonRuntime`**. The switch is an indirect +branch in the hottest code, keyed on a per-field instance value, and `ComplexMessage` has fifteen +fields spanning nine distinct name lengths — enough target variety to defeat the branch predictor +by more than the `arraycopy` it saves. Reverting the runtime paths to the pre-encoded arrays turned +that −10.8% into +2.6% and also moved `SimpleMessage.runtimeUtf16` from −0.4% to +7.7%. The +reasoning is recorded in `FieldName`'s javadoc so it does not get "fixed" again. + +Where the win lands: +12–13% on `SimpleMessage` codegen, which is six fields and ~80 bytes of +output — i.e. the shape where field-name writing is the largest single cost. + +*Not supported, before or after:* JSONB output. `JSONWriterJSONB.writeNameRaw(byte[])` forwards to +`writeRaw(byte[])`, which dumps JSON *text* bytes into a JSONB stream — so routing a protobuf +message through `JSONB.toBytes` via `writerModule()` produced garbage already. `writeNameNRaw` +produces different garbage. Also unchanged: `UseSingleQuotes` was, and still is, ignored for field +names. + +### 2. No boxing on repeated primitives (codegen + typed path) — no measured win + +`message.getFooList()` for a repeated numeric field returns a `List` whose runtime type +is `com.google.protobuf.IntArrayList`, and `IntArrayList.get(i)` returns +`Integer.valueOf(getInt(i))` — apparently an allocation for every element outside the −128..127 +cache. The `Internal.IntList` / `LongList` / `DoubleList` / `FloatList` / `BooleanList` interfaces +are public and expose `getInt(i)` / `getLong(i)` / …, so an `instanceof` narrow removes the boxing +*and* the unbox; a generic `List` branch is kept for non-protobuf list implementations. + +**It measured as exactly nothing** — 0 B/op and 0% on `RepeatedAndMap`, whose `ints` field holds +50–200 values from `rng.nextInt()` (so essentially none are cache hits). HotSpot was already +scalar-replacing the boxes: `IntArrayList.get` inlines, the `Integer` never escapes the loop, and +escape analysis deletes it. If boxing were really allocating there, it alone would have been +~2,000 of the 5,385 B/op measured. + +Kept anyway, as insurance rather than a win: the elimination depends on the loop inlining, which +does not hold in the interpreter or at C1 (relevant for short-lived processes that never reach +C2), nor if a large `writeFields` body pushes the loop past the inlining budget. Do not expect it +to show up in a benchmark. **This is the cautionary tale of the audit** — "obvious" boxing on a +hot path is often already gone. + +The typed path also gained `RepeatedDouble/Float/BoolAccessor`; those types previously fell +through to the generic `RepeatedAccessor`, which re-switched on `JavaType` per element. + +### 3. Bulk repeated-string writes + +`jsonWriter.writeString(List)` writes the whole array — brackets, commas, per-element +escaping — with one capacity check, replacing the hand-rolled +`startArray`/`writeComma`/`writeString`/`endArray` loop. + +### 4. Typed `Struct` / `Value` / `ListValue` + +`WellKnownTypes.writeValue` called `desc.getOneofs().get(0)` per `Value`, and +`Descriptor.getOneofs()` is `Collections.unmodifiableList(Arrays.asList(oneofs))` — **two +allocations per value written** — followed by `getOneofFieldDescriptor()` and a `switch` on the +field's `String` name. + +`Struct`, `Value` and `ListValue` are ordinary compiled classes in protobuf-java, so the writer +now takes a typed path when it has one: `struct.getFieldsMap()` instead of materializing the +synthetic MapEntry list and pulling key/value back out through `getField()`, and +`value.getKindCase()` — an int switch — instead of the descriptor dance. `DynamicMessage` keeps +the reflective path, with the per-entry `getFields(entry, "key", "value")` lookup hoisted out of +the loop (it was a cache probe *per struct entry*) and the `kind` oneof cached per Descriptor. + +**+57%/+64% throughput and −657 B/op** — by far the biggest single win in the audit, and the one +that was easiest to miss, because it was hiding inside a well-known-type helper rather than on the +main field loop. + +### 5. Typed WKT wrappers in codegen + +An `Int32Value`/`StringValue`/… field went through `WellKnownTypes.write()` → full-name `String` +switch → descriptor cache probe → `getField()` reflection + boxing. The plugin knows the type, so +it now emits `jsonWriter.writeInt32(wrap.getValue())` and friends directly — the same write a +plain field of that type would get. + +### 6. Reflection path: `hasField` before `getField` + +`writeFields` read `Object value = message.getField(fd)` *before* checking presence, so every +absent `optional` field paid a reflective read and a boxing allocation whose result was +immediately discarded. + +### 7. Map key/value descriptors resolved once + +`FieldWriter.writeMap` called `findFieldByName("key")` and `findFieldByName("value")` — two hash +lookups — on *every map field write*. Both are now resolved when the schema is built +(`MessageSchema.FieldInfo.mapKeyDescriptor()`, `TypedFieldAccessor.MapAccessor`). The old +two-argument overload is kept for callers without a cached schema. + +### 8. Non-ASCII `json_name` (a correctness fix found on the way) + +Auditing the name path surfaced a latent bug rather than a slow path. `[json_name = "…"]` accepts +any string, and: + +- the old `buildNameWithColonUtf8` did `(byte) name.charAt(i)`, truncating anything non-ASCII into + corrupt UTF-8, and would have emitted a raw `"` or `\` straight into the JSON string; +- `EncoderGenerator` interpolated the name into a Java string literal unescaped, so a `json_name` + containing a quote or newline generated source that **does not compile**. + +`FieldNames.isRawWritable` now gates the raw arrays (printable ASCII, nothing escapable), anything +else goes through `JSONWriter.writeName(String)` + `writeColon()` so fastjson2 escapes and +transcodes it, and `FieldNames.javaStringLiteral` (mirrored in the generator) escapes emitted +literals. Covered by `FieldNamesTest`. + +### 9. One `JSONWriter.Context` per encoder + +`JSONWriter.of()` allocates a fresh `JSONWriter.Context` per call to carry configuration that +never changes between encodes. `BuffJsonEncoder` now holds one and passes it to +`JSONWriter.of(ctx)` / `ofUTF8(ctx)`. Verified safe to share across writers and encodings; the +context is read-only during writing. + +**−88 B/op on every benchmark**, which on `SimpleMessage` is 30% of the total. The cheapest item +in the audit by a wide margin: five lines. + +## Part 2 — Backlog, highest value first + +### B1. Devirtualize `JSONWriter` (generate per-encoding `writeFields` bodies) + +The remaining big structural item. `writeFields` calls `writeString`, `writeInt32`, `writeDouble`, +`writeBase64`, `startObject`… on `JSONWriter`, an abstract class with six concrete subclasses in +the jar (`JSONWriterUTF8`, `JSONWriterUTF16` + three JDK/unsafe-free variants, `JSONWriterJSONB`). +An application that uses both `encode()` (UTF-16) and `encodeToBytes()` (UTF-8) makes every one of +those call sites bimorphic; add other fastjson2 usage in the same process and the profile can go +megamorphic, at which point HotSpot stops inlining the methods that do the actual work. + +Fix: have the plugin emit `writeFieldsUtf8(JSONWriterUTF8 …)` and +`writeFieldsUtf16(JSONWriterUTF16 …)`, with `writeFields` doing one type check and dispatching. +Every write inside then has a concrete receiver. + +Cost: 2× generated bytecode per message, and bigger methods can *lose* inlining, so this must be +measured rather than assumed. The typed-accessor path cannot follow without duplicating ~20 +accessor variants; it would stay as-is. + +### B2. Pre-size the output buffer + +`JSONWriter.ensureCapacity(int)` is public. Nothing calls it, so a large message grows the buffer +by repeated `grow()`-and-copy from fastjson2's initial thread-cached array. A per-Descriptor +rolling estimate (an EMA of that type's recent output sizes) passed to `ensureCapacity` at the top +of `writeMessage` would collapse that chain to one allocation. Cheap to build, and it targets +exactly the benchmarks that are slowest in absolute terms (`RepeatedAndMap`, `stringHeavy`, deep +nesting). + +### B3. Fuse constant name+value pairs + +`writeNameRaw` handles the leading comma, so anything constant can be folded into the "name": + +- implicit-presence `bool` — the value written is always `true`, so `"active":true` is one + constant and one call; +- enums — the value set is known at code-generation time, so `"status":"ACTIVE"` can be a + `byte[][]`/`char[][]` indexed by enum number. This also removes fastjson2's escape scan and + UTF-16→UTF-8 transcoding of the enum name on every write, which is pure waste for names that + are ASCII and fixed. + +Cost: constants grow as fields × enum cardinality. + +### B4. Zero-allocation numeric map keys + +`entry.getKey().toString()` allocates a `String` per entry for int/long-keyed maps. +`writeNameRaw(byte[], int, int)` and `writeNameRaw(char[], int, int)` both exist, so the key can +be formatted into a reusable scratch buffer as `"`, digits, `"`, `:` and written in one call with +comma handling intact. (`writeName(int)`/`writeName(long)` look like the answer but are not — they +write an *unquoted* number and no colon, which proto3 JSON map keys forbid.) + +### B5. Zero-allocation `Timestamp` / `Duration` + +`writeTimestampDirect` allocates an exact-size `byte[20..30]` per timestamp purely because +`writeStringLatin1` has no offset/length overload. `writeRaw(char[], int, int)` does take one — if +it behaves identically on UTF-8 and UTF-16 writers (needs checking), the formatting could target a +thread-local scratch with the quotes written in, and the allocation disappears. Timestamps are +common enough in real payloads for this to matter. + +### B6. Base64 without the `ByteString` copy + +`writeBase64(v.toByteArray())` copies the entire payload before encoding it. Encoding straight out +of the `ByteString` (`asReadOnlyByteBuffer()`, or `copyTo` into a pooled buffer) removes a +payload-sized allocation per bytes field — the largest single allocation in any message carrying a +blob. + +### B7. Resolve WKT-ness at schema-build time everywhere + +`WellKnownTypes.isWellKnownType()` is a `Set.of(…).contains(descriptor.getFullName())` probe, and +`WellKnownTypes.write()` then re-dispatches on a `String` switch — both **per nested WKT write**, +in the typed and reflection paths. A message field's type is known when the schema is built, so a +`WktKind` stored in the accessor / `FieldInfo` turns both into a tableswitch. Codegen already +bypasses this for Timestamp, Duration and the wrappers. + +### B8. Memoize the nested encoder in typed accessors + +`PresenceMessageAccessor` / `RepeatedMessageAccessor` call `writer.writeMessage`, re-entering the +three-tier dispatch — an `instanceof BuffJsonCodecHolder` plus a megamorphic `buffJsonEncoder()`, +or a `TypedMessageSchema` ConcurrentHashMap lookup — for every nested message. A message field's +concrete type is fixed, so the accessor can cache the resolved encoder on first use. This is the +typed-path analogue of codegen's direct `INSTANCE.writeFields` call, which is where a good part of +the codegen-vs-typed gap on nested messages comes from. + +### B9. Hoist the type switch out of reflection-path loops + +`FieldWriter.writeRepeated`/`writeMap` call `writeValue`, which re-switches on `JavaType` for +every element. Specializing per element type (as the typed accessors now do) would help +`DynamicMessage` and `Any` payloads, which are the only traffic left on that path. + +### B10. Small allocation cleanups + +- `writeUnsignedLongString` allocates `byte[20]` then `Arrays.copyOf` for values ≥ 2^63 — compute + the digit count first and allocate once (the same trick `writeDurationDirect` already uses). +- `writeFieldMask` allocates a `StringBuilder` + `String`, plus one `String` per path in + `snakeToCamel`. + +### B11. Encoder-level odds and ends + +- `messageWriter()` reads a `volatile` per encode. Marginal on x86, an acquire barrier on ARM. + Constructing the writer eagerly (and rebuilding it in the setters) would make the field final. +- `encode()`/`encodeToBytes()` copy out of the pooled buffer to build the `String`/`byte[]`. That + is inherent to the API shape; a streaming/`Appendable` API (already on the "Not Yet Implemented" + list) is what lets hot callers avoid it. + +## Things that looked promising and are not + +- **`writeName(int)` / `writeName(long)`** for numeric map keys — writes an unquoted number and no + colon. See B4. +- **`writeInt32(int[])` / `writeInt64(long[])` / `writeDouble(double[])`** write a whole JSON array + in one call, but protobuf's `Internal.*List` does not expose its backing array, so feeding them + would require copying it out — which costs more than the loop saves. (`writeString(List)` + is the exception, since it takes the `List` directly — see applied item 3.) +- **`writeDouble(double[])` for repeated doubles**, even if the array were free: fastjson2 writes + non-finite values as `null`, whereas proto3 JSON requires `"NaN"`/`"Infinity"` strings, so each + element needs its own finite check anyway. + From 2b68831737de35f7142a534d97d9115e6670b061 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 11:03:21 +0000 Subject: [PATCH 2/2] fix(encode): repair two correctness regressions found reviewing the packed-name path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review of the previous commit surfaced two bugs in the word-packed field names, both reproduced, plus several smaller defects. **UseSingleQuotes emitted invalid JSON.** The quote characters are split between us and fastjson2, and the split differs per name length: both quotes are baked into the packed word for lengths 2-6 and 9-14, only the opening quote for 7 and 15, neither for 8 and 16. fastjson2 emits its share from `this.quote`, so a single-quote writer produced `{"optionalFixed32':7}` — unparseable. The `writeNameRaw` arrays had always *ignored* the feature and emitted valid double-quoted names, so this was a regression from "ignores a feature" to "emits garbage". **Native-order packing is not endian-agnostic.** `JSONWriterUTF8` stores the word with `Unsafe.putLong` (native order), but `JSONWriterUTF16.putLong` widens it by extracting bytes at hard-coded little-endian positions with no `BIG_ENDIAN` branch (confirmed by disassembly). One constant cannot satisfy both on a big-endian host; the javadoc claimed otherwise. Both are now gated in one place. `FieldNames.PACKED_NAMES_SUPPORTED` self-checks the layout at class init — it packs a probe name of every length 2-16, writes it through both encodings and compares the text — so a big-endian host or a future fastjson2 that moves the layout degrades to the arrays instead of silently corrupting every field name. `ProtobufMessageWriter.canUsePackedNames` additionally vetoes the codegen tier for a single-quote writer: one field load per message, falling through to the typed tier, which the cross-path fuzz test already proves byte-identical. Measured cost: none (final interleaved A/B still +15.1%/+15.2% on SimpleMessage codegen, +116%/+108% on Struct). Also from the review: - Deprecated fields: the name-constant loop skipped `[deprecated = true]` while the writeFields loop did not, so such a field referenced an undeclared constant and the generated source did not compile. They now serialize like any other field (matching JsonFormat and both runtime paths), with `@SuppressWarnings("deprecation")` emitted on the class since consumers build with -Werror. - Java literal escaping: `\uXXXX` for a line terminator does not work — javac translates unicode escapes before tokenizing (JLS 3.3), so a unicode-escaped LF becomes a real newline and leaves the literal unclosed. Both copies now emit `\n`/`\r`/`\t`/`\b`/`\f` properly, and the test compiles the emitted literal with javax.tools.JavaCompiler instead of asserting the weaker "printable ASCII" property a broken escape satisfies. - `Value.getKindCase()` switch gained a `default -> writeNull()` arm; a kind added by a future protobuf-java would otherwise write nothing after the name and colon, i.e. `{"k":}`, and javac has no exhaustiveness lint for switch statements. - Dropped the `VALUE_KIND_CACHE` added last commit: a strong-keyed Descriptor map pins the descriptor pool of every schema ever loaded, and it only served a path `DynamicMessage` reaches, to save two allocations. - `packedWord1`/`packedTailInt` now reject lengths that have no such word, instead of returning 0 from the zero-padded tail and writing NUL bytes. - Removed the dead `writeMap` overload and the `nameWithColon()` accessors, which had become callerless and returned null for a non-raw-writable name. - Corrected stale docs: the reflection-path field-name dispatch in the architecture diagram, the accessor list, the DoubleHeavy allocation baselines, and the `Struct`/`Value` fast path missing from Key Design Decisions. The audit doc now also records that the largest generated `writeFields` in the repo is 3,240 bytecodes against HotSpot's 8,000 `DontCompileHugeMethods` limit (a reviewer's 14,644 figure did not reproduce), and that absolute benchmark numbers on this host moved ~25% between runs while the ratios held — so only ever compare two jars run back to back. 560 tests pass; allocation budgets green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Rjh5q2cHevWc6DPzyoACQp --- CLAUDE.md | 7 +- allocation-check.sh | 4 +- buff-json-protoc-plugin/CLAUDE.md | 1 + .../buffjson/protoc/DecoderGenerator.java | 5 + .../buffjson/protoc/EncoderGenerator.java | 53 +++++- .../suboptimal/buffjson/FieldNamesTest.java | 105 +++++++++++- buff-json/CLAUDE.md | 6 +- .../suboptimal/buffjson/BuffJsonEncoder.java | 9 + .../buffjson/internal/FieldNames.java | 141 +++++++++++++-- .../buffjson/internal/FieldWriter.java | 12 -- .../buffjson/internal/MessageSchema.java | 15 +- .../internal/ProtobufMessageWriter.java | 41 ++++- .../buffjson/internal/WellKnownTypes.java | 51 +++--- docs/encoding-performance.md | 160 ++++++++++++------ 14 files changed, 474 insertions(+), 136 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a4c0291..100fd78 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -30,7 +30,7 @@ BuffJsonEncoder.encode(message) -> TypedMapAccessor uses getXxxMap() / getXxxValueMap() -> 3. MessageSchema.forDescriptor(...) iteration # PURE REFLECTION Object value = message.getField(fd); FieldWriter.writeValue(...) # boxing - -> field name: if (utf8) writeNameRaw(nameWithColonUtf8); else writeNameRaw(nameWithColon); + -> field name: FieldName.writeTo(jw) -> isUTF8() ? byte[] : char[] # same as tier 2 ``` **1. Codegen path** (~10x JsonFormat) — protoc plugin generates `*JsonEncoder` per message. Each encoder calls typed getters directly (`msg.getId()` → `int`), eliminating reflection, boxing, runtime type dispatch, and ConcurrentHashMap lookups. @@ -48,7 +48,7 @@ Codegen optimizations: - **No boxing in repeated numeric fields** — repeated int32/int64/double/float/bool lists are `instanceof`-narrowed to `Internal.IntList`/`LongList`/`DoubleList`/`FloatList`/`BooleanList` and read with `getInt(i)`/`getLong(i)`/…, avoiding the `Integer.valueOf`/`Long.valueOf` allocation `List.get(i)` performs per element. - **Bulk repeated strings** — `jsonWriter.writeString(values)` hands the whole `List` to fastjson2, which writes brackets, commas and per-element escaping with one capacity check. -**2. Typed-accessor path** (~6x JsonFormat) — used when codegen is disabled or unavailable. `TypedMessageSchema` caches per-Descriptor arrays of `TypedFieldAccessor` records (sealed interface, ~20 variants) whose getter slots are bound via `LambdaMetafactory` to the protoc-generated typed getters. No `getField()` reflection, no boxing. Specialized `RepeatedInt/Long/String/MessageAccessor` and `TypedMapAccessor` eliminate per-element switch dispatch. A `FAILED` sentinel marks descriptors where lambda binding failed (silent fallback to path 3). +**2. Typed-accessor path** (~6x JsonFormat) — used when codegen is disabled or unavailable. `TypedMessageSchema` caches per-Descriptor arrays of `TypedFieldAccessor` records (sealed interface, ~20 variants) whose getter slots are bound via `LambdaMetafactory` to the protoc-generated typed getters. No `getField()` reflection, no boxing. Specialized `RepeatedInt/Long/Double/Float/Bool/String/MessageAccessor` and `TypedMapAccessor` eliminate per-element switch dispatch. A `FAILED` sentinel marks descriptors where lambda binding failed (silent fallback to path 3). **3. Pure-reflection path** — used for `DynamicMessage` (no compiled getters → can't bind lambdas) or descriptors where `LambdaMetafactory` failed. Iterates cached `MessageSchema.FieldInfo[]`, calls `message.getField(fd)` (boxes primitives) — after `hasField` for fields with presence, so an absent optional field costs no boxed read — and `FieldWriter` switches on `JavaType`. Field names go through the same `FieldName` (`char[]`/`byte[]`, dispatched on `isUTF8()`) as the typed path. @@ -97,7 +97,8 @@ JSONFactory.getDefaultObjectReaderProvider().register(decoder.readerModule()); - **Module exposure** — `encoder.writerModule()` and `decoder.readerModule()` return fastjson2 modules backed by configured writer/reader instances, for mixed pojo+protobuf projects using `JSON.toJSONString()`. - **`MessageSchema` caching**: One-time cost per Descriptor. Avoids `getAllFields()` TreeMap allocation. - **`TypedMessageSchema` caching**: Per-Descriptor `LambdaMetafactory`-bound typed accessors, lazily built. `FAILED` sentinel marks descriptors where binding failed (silent fallback to reflection path). -- **Word-packed field names (`FieldNames`)**: fastjson2's `writeNameRaw(long[, long])` family takes the name already packed into machine words and stores it with one or two `Unsafe.putLong`s — it is what fastjson2's own ASM/`@JSONCompiled` writers use, and it is the main remaining gap to the POJO ceiling. `FieldNames` derives the constants by reading native-order words straight out of the literal `"name":` text (offset 0, except lengths 8 and 16 where fastjson2 emits the opening quote itself). **The same constants serve UTF-8 and UTF-16 writers**, because `JSONWriterUTF16` widens the packed ASCII to `char`s on the way out — so packed names need no `isUTF8()` branch at all. **Codegen only, by measurement**: the plugin knows each name's length so it emits the exact `writeNameRaw` call with no dispatch (+12–13% on `SimpleMessage`). Routing the runtime paths through it too requires a `switch` on the length — an indirect branch in the hottest code keyed on a per-field value — which cost 10% on `ComplexMessage.buffJsonRuntime`, so `FieldName` keeps the pre-encoded `char[]`/`byte[]` pair. `FieldNames` still supplies those arrays, and codegen falls back to them for the rare unpackable `json_name`. A `json_name` that is not `FieldNames.isRawWritable` at all (non-ASCII, or carrying a quote/backslash/control char) goes through `writeName(String)` + `writeColon()` so fastjson2 escapes and transcodes it — the old `(byte) charAt(i)` array builder corrupted such names, and the generator emitted them into Java literals unescaped, which did not compile. +- **Typed `Struct`/`Value`/`ListValue` (`WellKnownTypes`)**: `Struct`, `Value` and `ListValue` are ordinary compiled protobuf-java classes, so `writeStruct`/`writeValue`/`writeListValue` take an `instanceof` fast path — `struct.getFieldsMap()` instead of materializing the synthetic MapEntry list and pulling key/value back through `getField()`, and `value.getKindCase()` (an int switch, with a `default` arm so a future kind cannot emit a name with no value) instead of `desc.getOneofs().get(0)` — which allocates an `Arrays.asList` + `unmodifiableList` pair on *every* call — plus `getOneofFieldDescriptor()` and a switch on the field's String name. `DynamicMessage` keeps the reflective path, with the map-entry descriptors hoisted out of the per-entry loop. Worth +57–64% and −657 B/op on `WktBenchmark.struct`, the largest single win in the encode audit. +- **Word-packed field names (`FieldNames`)**: fastjson2's `writeNameRaw(long[, long])` family takes the name already packed into machine words and stores it with one or two `Unsafe.putLong`s — it is what fastjson2's own ASM/`@JSONCompiled` writers use, and it is the main remaining gap to the POJO ceiling. `FieldNames` derives the constants by reading native-order words straight out of the literal `"name":` text (offset 0, except lengths 8 and 16 where fastjson2 emits the opening quote itself). **The same constants serve UTF-8 and UTF-16 writers**, because `JSONWriterUTF16` widens the packed ASCII to `char`s on the way out — so packed names need no `isUTF8()` branch at all. **Codegen only, by measurement**: the plugin knows each name's length so it emits the exact `writeNameRaw` call with no dispatch (+12–13% on `SimpleMessage`). Routing the runtime paths through it too requires a `switch` on the length — an indirect branch in the hottest code keyed on a per-field value — which cost 10% on `ComplexMessage.buffJsonRuntime`, so `FieldName` keeps the pre-encoded `char[]`/`byte[]` pair. **Not universally usable**: the quote characters are split between us and fastjson2 and the split *differs per length* (both quotes baked for 2–6 and 9–14, only the opening one for 7 and 15, neither for 8 and 16), so a `UseSingleQuotes` writer would emit `"name'` for lengths 7/15 — invalid JSON, where the arrays merely ignored the feature; and `JSONWriterUTF16` widens the word by extracting bytes at hard-coded little-endian positions with no `BIG_ENDIAN` branch, which native-order packing cannot satisfy on a big-endian host. So `FieldNames.PACKED_NAMES_SUPPORTED` self-checks the layout at class init, and `ProtobufMessageWriter.canUsePackedNames` additionally vetoes the codegen tier for a single-quote writer — one check per message, falling back to the typed path, which owns every byte it emits. `FieldNames` still supplies those arrays, and codegen falls back to them for the rare unpackable `json_name`. A `json_name` that is not `FieldNames.isRawWritable` at all (non-ASCII, or carrying a quote/backslash/control char) goes through `writeName(String)` + `writeColon()` so fastjson2 escapes and transcodes it — the old `(byte) charAt(i)` array builder corrupted such names, and the generator emitted them into Java literals unescaped, which did not compile. - **Shared `JSONWriter.Context`**: `JSONWriter.of()` allocates a fresh `Context` per call to carry configuration that never changes between encodes. `BuffJsonEncoder` holds one `JSONFactory.createWriteContext()` and passes it to `JSONWriter.of(ctx)`/`ofUTF8(ctx)`. The context is read-only during writing, so one per encoder is safe to share across threads. - **No boxing on repeated primitives**: protobuf-java backs repeated numeric fields with `Internal.IntList`/`LongList`/`DoubleList`/`FloatList`/`BooleanList`; `List.get(i)` on those boxes every element. Both codegen and the typed accessors `instanceof`-narrow and read via the primitive getters, keeping a generic `List` branch for non-protobuf list implementations. - **`message.getField(descriptor)`** for field access in pure-reflection path only (involves boxing for primitives). Codegen and typed-accessor paths bypass it entirely. diff --git a/allocation-check.sh b/allocation-check.sh index 751f4fd..1458aa0 100755 --- a/allocation-check.sh +++ b/allocation-check.sh @@ -48,8 +48,8 @@ BUDGETS=( "io.suboptimal.buffjson.benchmarks.ComplexMessageBenchmark.buffJsonRuntime:1900" # baseline ~1308 B/op # DoubleHeavy (25 doubles, IoT/telemetry profile) — number formatting cost. - "io.suboptimal.buffjson.benchmarks.DoubleHeavyBenchmark.compiledUtf16:2200" # baseline ~1773 B/op - "io.suboptimal.buffjson.benchmarks.DoubleHeavyBenchmark.compiledUtf8:2100" # baseline ~1749 B/op + "io.suboptimal.buffjson.benchmarks.DoubleHeavyBenchmark.compiledUtf16:2200" # baseline ~1685 B/op + "io.suboptimal.buffjson.benchmarks.DoubleHeavyBenchmark.compiledUtf8:2100" # baseline ~1661 B/op ) # Parse args diff --git a/buff-json-protoc-plugin/CLAUDE.md b/buff-json-protoc-plugin/CLAUDE.md index 2ac7183..187eb3e 100644 --- a/buff-json-protoc-plugin/CLAUDE.md +++ b/buff-json-protoc-plugin/CLAUDE.md @@ -80,6 +80,7 @@ For each non-WKT, non-map-entry message type: - **`DynamicMessage`** cannot use generated encoders (would fail cast) — guarded in `ProtobufMessageWriter` - **Map entry types** (`options.map_entry = true`) are skipped — they're synthetic - **Field-name packing** — `EncoderGenerator.emitWriteName(sb, fd, indent)` picks the call shape from `fd.getJsonName().length()`. `EncoderGenerator.isPackable` mirrors `FieldNames.isPackable` so the plugin does not need buff-json's runtime classes loadable during code generation; keep the two in sync. `writeNameRaw(byte[])` still throws `UnsupportedOperation` on `JSONWriterUTF16`, which is why the unpackable fallback keeps both arrays and the `utf8` local. +- **`[deprecated = true]` fields** — serialized like any other field (that is what `JsonFormat` and both runtime paths do, so all three paths agree). Because the generated code then calls the field's `@Deprecated` accessor and consumers build with `-Xlint:all -Werror`, both generators emit `@SuppressWarnings("deprecation")` on the class when `EncoderGenerator.hasDeprecatedField(msgDesc)` is true. Do **not** skip deprecated fields in only one of the two loops in `EncoderGenerator.generate` — the constant loop used to, which emitted a name write referencing an undeclared `NAME_*_W0` and generated source that did not compile - **Enum `UNRECOGNIZED`** — protobuf's generated `UNRECOGNIZED` constant throws `IllegalArgumentException` from `getNumber()`. Enum name arrays use `EnumDescriptor.getValues()` (not Java `.values()`) to avoid this - **Multiple distinct enum types per message** — each enum's name-array initializer is wrapped in its own `{ }` block inside the single `static {}` so the `edVals`/`max` locals don't collide (a message referencing two different enum types previously failed to compile) - **Negative enum values** (e.g. `NEG = -1`) — can't index the `String[]` name array, so they're skipped during array fill (`if (v.getNumber() >= 0)`, avoids `ArrayIndexOutOfBounds` at class init) and resolved at the write site via a descriptor fallback (`EnumClass.getDescriptor().findValueByNumber(n)`) so a named negative value still serializes by name; only genuinely unknown numbers fall through to the integer diff --git a/buff-json-protoc-plugin/src/main/java/io/suboptimal/buffjson/protoc/DecoderGenerator.java b/buff-json-protoc-plugin/src/main/java/io/suboptimal/buffjson/protoc/DecoderGenerator.java index 0518769..042f50d 100644 --- a/buff-json-protoc-plugin/src/main/java/io/suboptimal/buffjson/protoc/DecoderGenerator.java +++ b/buff-json-protoc-plugin/src/main/java/io/suboptimal/buffjson/protoc/DecoderGenerator.java @@ -26,6 +26,11 @@ static String generate(Descriptor msgDesc, String javaPackage, String decoderSim sb.append("package ").append(javaPackage).append(";\n\n"); sb.append("import com.alibaba.fastjson2.JSONReader;\n"); sb.append("import io.suboptimal.buffjson.BuffJsonGeneratedDecoder;\n\n"); + // readMessage calls the @Deprecated setter of a [deprecated = true] field, and + // consumers compile with -Xlint:all -Werror. + if (EncoderGenerator.hasDeprecatedField(msgDesc)) { + sb.append("@SuppressWarnings(\"deprecation\")\n"); + } sb.append("public final class ").append(decoderSimpleName); sb.append(" implements BuffJsonGeneratedDecoder<").append(messageClassName).append("> {\n\n"); diff --git a/buff-json-protoc-plugin/src/main/java/io/suboptimal/buffjson/protoc/EncoderGenerator.java b/buff-json-protoc-plugin/src/main/java/io/suboptimal/buffjson/protoc/EncoderGenerator.java index 75e015d..9ac8005 100644 --- a/buff-json-protoc-plugin/src/main/java/io/suboptimal/buffjson/protoc/EncoderGenerator.java +++ b/buff-json-protoc-plugin/src/main/java/io/suboptimal/buffjson/protoc/EncoderGenerator.java @@ -80,6 +80,12 @@ static String generate(Descriptor msgDesc, String javaPackage, String encoderSim sb.append("package ").append(javaPackage).append(";\n\n"); sb.append("import com.alibaba.fastjson2.JSONWriter;\n"); sb.append("import io.suboptimal.buffjson.BuffJsonGeneratedEncoder;\n\n"); + // Deprecated fields still serialize (JsonFormat and the two runtime paths emit + // them), so writeFields calls their @Deprecated getters. Consumers build with + // -Xlint:all -Werror, which would turn that into a compile error. + if (hasDeprecatedField(msgDesc)) { + sb.append("@SuppressWarnings(\"deprecation\")\n"); + } sb.append("public final class ").append(encoderSimpleName); sb.append(" implements BuffJsonGeneratedEncoder<").append(messageClassName).append("> {\n\n"); @@ -94,10 +100,12 @@ static String generate(Descriptor msgDesc, String javaPackage, String encoderSim // writeNameRaw family: one or two Unsafe.putLong stores, no arraycopy, and // — because JSONWriterUTF16 widens the same packed bytes on the way out — no // isUTF8() branch either. Anything else keeps the char[]/byte[] pair. + // Every field in getFields() gets a constant: writeFields() below emits a name + // write for each of them, so skipping any here (deprecated ones used to be + // skipped) produced a reference to an undeclared constant and generated source + // that did not compile. boolean anyArrayName = false; for (FieldDescriptor fd : msgDesc.getFields()) { - if (fd.getOptions().hasDeprecated() && fd.getOptions().getDeprecated()) - continue; String jsonName = fd.getJsonName(); if (isPackable(jsonName)) { int n = jsonName.length(); @@ -677,6 +685,21 @@ private static void emitWriteName(StringBuilder sb, FieldDescriptor fd, String i sb.append(");\n"); } + /** + * Whether any field of {@code msgDesc} is {@code [deprecated = true]}, in which + * case the generated class needs {@code @SuppressWarnings("deprecation")} — it + * calls that field's {@code @Deprecated} accessor, and consumers compile with + * {@code -Werror}. + */ + static boolean hasDeprecatedField(Descriptor msgDesc) { + for (FieldDescriptor fd : msgDesc.getFields()) { + if (fd.getOptions().getDeprecated()) { + return true; + } + } + return false; + } + /** * Mirrors {@code FieldNames.isPackable} at code-generation time so the plugin * can pick the call shape without depending on buff-json's runtime classes @@ -701,18 +724,30 @@ private static boolean isRawWritable(String jsonName) { /** * Mirrors {@code FieldNames.javaStringLiteral} — an explicit {@code json_name} * carrying a quote or a newline would otherwise emit source that does not - * compile. + * compile. Line terminators must use their {@code \n}/{@code \r} escapes rather + * than unicode escapes: javac translates unicode escapes before tokenizing (JLS + * 3.3), so a unicode-escaped line terminator reintroduces a real line break and + * leaves the literal unclosed. Keep in sync with {@code FieldNames}. */ private static String javaStringLiteral(String jsonName) { StringBuilder sb = new StringBuilder(jsonName.length() + 8).append('"'); for (int i = 0; i < jsonName.length(); i++) { char c = jsonName.charAt(i); - if (c == '"' || c == '\\') { - sb.append('\\').append(c); - } else if (c >= 0x20 && c <= 0x7e) { - sb.append(c); - } else { - sb.append(String.format("\\u%04x", (int) c)); + switch (c) { + case '"' -> sb.append("\\\""); + case '\\' -> sb.append("\\\\"); + case '\n' -> sb.append("\\n"); + case '\r' -> sb.append("\\r"); + case '\t' -> sb.append("\\t"); + case '\b' -> sb.append("\\b"); + case '\f' -> sb.append("\\f"); + default -> { + if (c >= 0x20 && c <= 0x7e) { + sb.append(c); + } else { + sb.append(String.format("\\u%04x", (int) c)); + } + } } } return sb.append('"').toString(); diff --git a/buff-json-tests/src/test/java/io/suboptimal/buffjson/FieldNamesTest.java b/buff-json-tests/src/test/java/io/suboptimal/buffjson/FieldNamesTest.java index b5e8698..4717397 100644 --- a/buff-json-tests/src/test/java/io/suboptimal/buffjson/FieldNamesTest.java +++ b/buff-json-tests/src/test/java/io/suboptimal/buffjson/FieldNamesTest.java @@ -148,8 +148,8 @@ void arrayFallbackEncodesTheSameText() { } @ParameterizedTest - @ValueSource(strings = {"naïve", "a\"b", "a\\b", "a\tb", "日本語"}) - void javaLiteralOfAnEscapableNameCompilesAndRoundTrips(String name) { + @ValueSource(strings = {"naïve", "a\"b", "a\\b", "a\tb", "日本語", "a\nb", "a\rb"}) + void javaLiteralOfAnEscapableNameIsSourceSafe(String name) { assertFalse(FieldNames.isRawWritable(name), name); String literal = FieldNames.javaStringLiteral(name); assertTrue(literal.startsWith("\"") && literal.endsWith("\""), literal); @@ -159,6 +159,107 @@ void javaLiteralOfAnEscapableNameCompilesAndRoundTrips(String name) { assertTrue(c >= 0x20 && c <= 0x7e, literal + " has a raw char at " + i); } } + + /** + * Actually compiles the emitted literal and reads the constant back. Checking + * only that the literal body is printable ASCII is not enough: javac + * translates unicode escapes before tokenizing (JLS 3.3), so a printable + * {@code \}{@code u000a} turns into a real newline and leaves the string + * unclosed. That is exactly how a line terminator in a {@code json_name} used + * to emit source that would not compile. + */ + @ParameterizedTest + @ValueSource(strings = {"naïve", "a\"b", "a\\b", "a\tb", "日本語", "a\nb", "a\rb", "a\r\nb", + "quote\"and\\backslash", "id"}) + void javaLiteralCompilesAndRoundTrips(String name) throws Exception { + var compiler = javax.tools.ToolProvider.getSystemJavaCompiler(); + org.junit.jupiter.api.Assumptions.assumeTrue(compiler != null, "no JDK compiler in this JVM"); + + String className = "NameLiteralProbe"; + String source = "public final class " + className + " { public static final String VALUE = " + + FieldNames.javaStringLiteral(name) + "; }"; + + var out = java.nio.file.Files.createTempDirectory("buffjson-literal"); + try { + var diagnostics = new javax.tools.DiagnosticCollector(); + var fileManager = compiler.getStandardFileManager(diagnostics, null, null); + var unit = new javax.tools.SimpleJavaFileObject(java.net.URI.create("string:///" + className + ".java"), + javax.tools.JavaFileObject.Kind.SOURCE) { + @Override + public CharSequence getCharContent(boolean ignoreEncodingErrors) { + return source; + } + }; + boolean ok = compiler.getTask(null, fileManager, diagnostics, java.util.List.of("-d", out.toString()), + null, java.util.List.of(unit)).call(); + assertTrue(ok, + () -> "emitted literal does not compile: " + source + "\n" + diagnostics.getDiagnostics()); + fileManager.close(); + + try (var loader = new java.net.URLClassLoader(new java.net.URL[]{out.toUri().toURL()}, null)) { + Object value = loader.loadClass(className).getField("VALUE").get(null); + assertEquals(name, value, "literal round-trip"); + } + } finally { + try (var paths = java.nio.file.Files.walk(out)) { + paths.sorted(java.util.Comparator.reverseOrder()).forEach(p -> p.toFile().delete()); + } + } + } + } + + /** + * The packed words carry a hard-coded {@code "} while fastjson2 supplies its + * share of the quotes from {@code this.quote} — and the split differs per name + * length. Under {@code UseSingleQuotes} that makes lengths 7 and 15 emit + * {@code "name'} : not a feature we merely ignore, but JSON no parser accepts. + * {@code ProtobufMessageWriter} therefore vetoes the codegen tier for such a + * writer. + */ + @Nested + class SingleQuoteWriter { + + private static final com.google.protobuf_test_messages.proto3.TestMessagesProto3.TestAllTypesProto3 MESSAGE = com.google.protobuf_test_messages.proto3.TestMessagesProto3.TestAllTypesProto3 + .newBuilder().setOptionalInt32(1).setOptionalFixed32(7).setOptionalString("s").build(); + + @Test + void theFixtureStillCoversAnAffectedNameLength() { + // optionalFixed32 is 15 characters. If protobuf ever renames it, this test + // stops covering the bug — fail loudly rather than pass vacuously. + assertTrue(MESSAGE.getDescriptorForType().getFields().stream() + .map(com.google.protobuf.Descriptors.FieldDescriptor::getJsonName).map(String::length) + .anyMatch(n -> n == 7 || n == 15), "no length-7/15 json name left in the fixture"); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void codegenIsVetoedSoOutputStaysValidJson(boolean utf8) { + var codegen = new io.suboptimal.buffjson.internal.ProtobufMessageWriter(null, true, true); + String singleQuoted; + try (JSONWriter jw = utf8 + ? JSONWriter.ofUTF8(JSONWriter.Feature.UseSingleQuotes) + : JSONWriter.of(JSONWriter.Feature.UseSingleQuotes)) { + codegen.writeMessage(jw, MESSAGE); + singleQuoted = jw.toString(); + } + + // Parses at all — the regression emitted `"optionalFixed32':7`. + assertDoesNotThrow(() -> com.alibaba.fastjson2.JSON.parseObject(singleQuoted), singleQuoted); + + // And byte-identical to the typed path under the *same* writer, i.e. the + // codegen tier was skipped. (Compared under the same writer because string + // values do legitimately honour UseSingleQuotes — only the field names, + // which we pre-encode, ignore it.) + var typed = new io.suboptimal.buffjson.internal.ProtobufMessageWriter(null, false, true); + try (JSONWriter jw = utf8 + ? JSONWriter.ofUTF8(JSONWriter.Feature.UseSingleQuotes) + : JSONWriter.of(JSONWriter.Feature.UseSingleQuotes)) { + typed.writeMessage(jw, MESSAGE); + assertEquals(jw.toString(), singleQuoted); + } + assertTrue(singleQuoted.contains("\"optionalFixed32\":"), + () -> "field names must stay double-quoted: " + singleQuoted); + } } /** diff --git a/buff-json/CLAUDE.md b/buff-json/CLAUDE.md index 9cd004b..20f9b61 100644 --- a/buff-json/CLAUDE.md +++ b/buff-json/CLAUDE.md @@ -63,12 +63,12 @@ io.suboptimal.buffjson.internal.typed/ - Reuses cached `ProtobufMessageWriter(typeRegistry, useGenerated, useTyped)` (volatile field on encoder; invalidated on setters). - Calls `writer.writeMessage(jsonWriter, message)`. 2. `ProtobufMessageWriter.writeFields(jsonWriter, message)` — three-tier dispatch: - - **Tier 1 — Codegen** (if `useGenerated && message instanceof BuffJsonCodecHolder`): + - **Tier 1 — Codegen** (if `useGenerated && canUsePackedNames(jw) && message instanceof BuffJsonCodecHolder`): - `holder.buffJsonEncoder().writeFields(jw, msg, this)` → typed getters, no reflection. - Nested messages call other encoders directly via `INSTANCE.writeFields(jw, msg, writer)` (no registry, no instanceof per nested). - Timestamp/Duration fields call `writeTimestampDirect()`/`writeDurationDirect()`. - Enum fields use pre-cached `String[]` name arrays. - - Field-name writes are `jsonWriter.writeNameRaw(NAME_X_W0[, ...])` — the name is already in machine words, so no `isUTF8()` branch and no `arraycopy`. The `boolean utf8` local and the `char[]`/`byte[]` constants are emitted only when some field has an unpackable `json_name`. + - Field-name writes are `jsonWriter.writeNameRaw(NAME_X_W0[, ...])` — the name is already in machine words, so no `isUTF8()` branch and no `arraycopy`. Generated code emits these unconditionally, so **`canUsePackedNames` is the only veto**: it skips this whole tier when `FieldNames.PACKED_NAMES_SUPPORTED` is false (layout self-check failed, or big-endian) or the writer uses single quotes (lengths 7 and 15 take their closing quote from fastjson2, so the mix would be invalid JSON). Falling through to tier 2 is safe because the fuzz test asserts all three tiers are byte-identical. The `boolean utf8` local and the `char[]`/`byte[]` constants are emitted only when some field has an unpackable `json_name`. - Repeated numeric/bool fields narrow to `Internal.IntList`/`LongList`/`DoubleList`/`FloatList`/`BooleanList` and read primitives (no per-element boxing); repeated strings go through `jsonWriter.writeString(values)` in one call. - Wrapper WKT fields (`Int32Value`, `StringValue`, …) emit the typed `getValue()` write directly. - Returns — never falls through. @@ -88,7 +88,7 @@ io.suboptimal.buffjson.internal.typed/ - Map fields pass `FieldInfo.mapKeyDescriptor()`/`mapValueDescriptor()` into `FieldWriter.writeMap`, which no longer calls `findFieldByName("key"/"value")` per map write. - Field-name writes go through `FieldInfo.name().writeTo(jw)` (`FieldName` — pre-encoded `char[]`/`byte[]`, dispatched on `isUTF8()`; the word-packed path is codegen-only, see `FieldName`'s javadoc for the measurement). 3. For MESSAGE fields in any path: `WellKnownTypes.isWellKnownType()` check first, then recurses via `writer.writeMessage()` (which re-enters the three-tier dispatch). Codegen skips this for Timestamp, Duration and the nine wrappers, whose writes are emitted inline. -4. `Struct`/`Value`/`ListValue` take a typed fast path when the message is a compiled `com.google.protobuf.Struct`/`Value`/`ListValue`: `getFieldsMap()` instead of materializing the synthetic MapEntry list, and `getKindCase()` (an int switch) instead of `getOneofs().get(0)` — which allocates an `Arrays.asList` + `unmodifiableList` wrapper pair on *every* call — plus `getOneofFieldDescriptor()` and a switch on the field's String name. The reflective path stays for `DynamicMessage`, with the map-entry key/value descriptors hoisted out of the per-entry loop and the `kind` oneof cached per Descriptor. +4. `Struct`/`Value`/`ListValue` take a typed fast path when the message is a compiled `com.google.protobuf.Struct`/`Value`/`ListValue`: `getFieldsMap()` instead of materializing the synthetic MapEntry list, and `getKindCase()` (an int switch) instead of `getOneofs().get(0)` — which allocates an `Arrays.asList` + `unmodifiableList` wrapper pair on *every* call — plus `getOneofFieldDescriptor()` and a switch on the field's String name. The `getKindCase()` switch carries a `default -> writeNull()` arm, because a kind added by a future protobuf-java would otherwise write nothing after the caller has already emitted the name and colon. The reflective path stays for `DynamicMessage`, with the map-entry key/value descriptors hoisted out of the per-entry loop; the `kind` oneof is *not* cached — a strong-keyed `Descriptor` cache would pin the descriptor pool of every schema ever loaded to save two allocations on a cold path. ## Settings Flow (no ThreadLocals) diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/BuffJsonEncoder.java b/buff-json/src/main/java/io/suboptimal/buffjson/BuffJsonEncoder.java index 24c81c9..2a73fd5 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/BuffJsonEncoder.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/BuffJsonEncoder.java @@ -52,6 +52,15 @@ public final class BuffJsonEncoder { * {@link JSONWriter.Context} per call to carry configuration that never changes * between encodes. The context is read-only during writing, so one per encoder * is safe to share across threads. + * + *

+ * Note that {@code JSONFactory.createWriteContext()} snapshots + * fastjson2's process-wide writer defaults (features, date format, zone, max + * level). Sharing one context therefore pins them at encoder-construction time: + * a later {@code JSON.config(...)} or + * {@code JSONFactory.setDefaultMaxLevel(...)} no longer affects an + * already-built encoder. Configure fastjson2 before calling + * {@link BuffJson#encoder()} if you rely on those globals. */ private final JSONWriter.Context writeContext = JSONFactory.createWriteContext(); diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldNames.java b/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldNames.java index ea979e3..5545e55 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldNames.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldNames.java @@ -4,6 +4,8 @@ import java.lang.invoke.VarHandle; import java.nio.ByteOrder; +import com.alibaba.fastjson2.JSONWriter; + /** * Packs a JSON field name into the {@code long}/{@code int} words consumed by * fastjson2's {@code JSONWriter.writeName2Raw(long)} … @@ -60,10 +62,33 @@ * *

* The offset-1 cases (8 and 16) are the two lengths where fastjson2 writes the - * opening quote separately and stores only the name body. The words are read in - * {@linkplain ByteOrder#nativeOrder() native byte order} because fastjson2 - * stores them with {@code Unsafe.putLong}, so the same constants are correct on - * little- and big-endian hosts alike. + * opening quote separately and stores only the name body. + * + *

Not universally usable — see {@link #PACKED_NAMES_SUPPORTED}

+ * + * This is an undocumented fastjson2 internal, and two things can invalidate it: + * + *
    + *
  • Byte order. {@code JSONWriterUTF8} stores the word with + * {@code Unsafe.putLong} (native order), but {@code JSONWriterUTF16} widens it + * to {@code char}s by extracting bytes at hard-coded little-endian + * positions ({@code v & 0xFF}, {@code (v & 0xFF00) << 8}, …) with no + * {@code BIG_ENDIAN} branch. The two cannot both be satisfied by one constant + * on a big-endian host. + *
  • Quoting. The quote characters are split between us and fastjson2 + * and the split differs per length: we bake both quotes for lengths 2–6 and + * 9–14, only the opening quote for 7 and 15, and neither for 8 and 16. + * fastjson2 emits its share from {@code this.quote}, so under + * {@code JSONWriter.Feature.UseSingleQuotes} a name of length 7 or 15 comes out + * as {@code "name'} — not merely a feature we ignore, but JSON no parser + * accepts. + *
+ * + *

+ * So {@code PACKED_NAMES_SUPPORTED} self-checks the layout at class init, and + * {@code ProtobufMessageWriter} additionally skips the codegen path for a + * single-quote writer. Both failures fall back to the {@code writeNameRaw} + * arrays, which own every byte they emit. * *

* Both encodings share the constants. {@code JSONWriterUTF16} widens the @@ -92,9 +117,81 @@ public final class FieldNames { private static final VarHandle INT_VIEW = MethodHandles.byteArrayViewVarHandle(int[].class, ByteOrder.nativeOrder()); + /** + * Whether this JVM's fastjson2 actually consumes the packed words the way + * {@link #packedWord0} produces them. + * + *

+ * Verified once here rather than assumed, because the layout is an undocumented + * fastjson2 internal and a mismatch would corrupt every field name in every + * message silently. A fastjson2 upgrade that moves the layout, or a + * big-endian host (see the class javadoc), turns this off and the write paths + * fall back to {@code writeNameRaw}. + */ + public static final boolean PACKED_NAMES_SUPPORTED = selfCheck(); + private FieldNames() { } + /** + * Round-trips a probe name of every supported length through both writer + * encodings and confirms the emitted text. Runs once per JVM at class init. + */ + private static boolean selfCheck() { + try { + for (int n = 2; n <= MAX_PACKED_LENGTH; n++) { + StringBuilder sb = new StringBuilder(n); + for (int i = 0; i < n; i++) { + sb.append((char) ('a' + i % 26)); + } + String name = sb.toString(); + String expected = "{\"" + name + "\":1}"; + for (int encoding = 0; encoding < 2; encoding++) { + try (JSONWriter jw = encoding == 0 ? JSONWriter.ofUTF8() : JSONWriter.of()) { + jw.startObject(); + writePackedName(jw, name); + jw.writeInt32(1); + jw.endObject(); + if (!expected.equals(jw.toString())) { + return false; + } + } + } + } + return true; + } catch (Throwable t) { + return false; + } + } + + /** + * Writes a packed name by length. Used only by {@link #selfCheck()} — the + * generated encoders emit the matching {@code writeNameRaw} call directly, + * which is the whole point of the fast path (a length switch here measured + * slower than the arrays; see + * {@link io.suboptimal.buffjson.internal.typed.FieldName}). + */ + private static void writePackedName(JSONWriter jw, String name) { + switch (name.length()) { + case 2 -> jw.writeName2Raw(packedWord0(name)); + case 3 -> jw.writeName3Raw(packedWord0(name)); + case 4 -> jw.writeName4Raw(packedWord0(name)); + case 5 -> jw.writeName5Raw(packedWord0(name)); + case 6 -> jw.writeName6Raw(packedWord0(name)); + case 7 -> jw.writeName7Raw(packedWord0(name)); + case 8 -> jw.writeName8Raw(packedWord0(name)); + case 9 -> jw.writeName9Raw(packedWord0(name), packedTailInt(name)); + case 10 -> jw.writeName10Raw(packedWord0(name), packedWord1(name)); + case 11 -> jw.writeName11Raw(packedWord0(name), packedWord1(name)); + case 12 -> jw.writeName12Raw(packedWord0(name), packedWord1(name)); + case 13 -> jw.writeName13Raw(packedWord0(name), packedWord1(name)); + case 14 -> jw.writeName14Raw(packedWord0(name), packedWord1(name)); + case 15 -> jw.writeName15Raw(packedWord0(name), packedWord1(name)); + case 16 -> jw.writeName16Raw(packedWord0(name), packedWord1(name)); + default -> throw new IllegalArgumentException("not a packable length: " + name.length()); + } + } + /** * Returns whether {@code jsonName} can use the packed {@code writeNameNRaw} * fast path — 2–16 characters, printable ASCII, no JSON escaping needed. @@ -141,12 +238,20 @@ public static long packedWord0(String jsonName) { */ public static long packedWord1(String jsonName) { checkPackable(jsonName); + if (jsonName.length() < 10) { + // Shorter names have no second word; the zero-padded read would silently + // return 0 and write NUL bytes into the name. + throw new IllegalArgumentException("no second packed word for a name of length " + jsonName.length()); + } return (long) LONG_VIEW.get(quotedName(jsonName), wordOffset(jsonName.length()) + 8); } /** The trailing {@code int} of the 9-character variant. */ public static int packedTailInt(String jsonName) { checkPackable(jsonName); + if (jsonName.length() != 9) { + throw new IllegalArgumentException("the tail int is only for 9-character names, got " + jsonName.length()); + } return (int) INT_VIEW.get(quotedName(jsonName), wordOffset(jsonName.length()) + 8); } @@ -186,17 +291,33 @@ public static byte[] nameWithColonBytes(String jsonName) { * for names that are not {@linkplain #isRawWritable raw-writable} — an explicit * {@code json_name} carrying a quote, a backslash or a newline would otherwise * emit source that does not compile. + * + *

+ * A line terminator must come out as its {@code \n}/{@code \r} escape + * and never as a unicode escape: javac translates unicode escapes before + * tokenizing (JLS 3.3), so a unicode-escaped LF turns back into a real newline + * and leaves the string literal unclosed. Every other control character is safe + * as a unicode escape — only line terminators may not appear inside a literal. */ public static String javaStringLiteral(String jsonName) { StringBuilder sb = new StringBuilder(jsonName.length() + 8).append('"'); for (int i = 0; i < jsonName.length(); i++) { char c = jsonName.charAt(i); - if (c == '"' || c == '\\') { - sb.append('\\').append(c); - } else if (c >= 0x20 && c <= 0x7e) { - sb.append(c); - } else { - sb.append(String.format("\\u%04x", (int) c)); + switch (c) { + case '"' -> sb.append("\\\""); + case '\\' -> sb.append("\\\\"); + case '\n' -> sb.append("\\n"); + case '\r' -> sb.append("\\r"); + case '\t' -> sb.append("\\t"); + case '\b' -> sb.append("\\b"); + case '\f' -> sb.append("\\f"); + default -> { + if (c >= 0x20 && c <= 0x7e) { + sb.append(c); + } else { + sb.append(String.format("\\u%04x", (int) c)); + } + } } } return sb.append('"').toString(); diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldWriter.java b/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldWriter.java index 1a557ee..7111cd3 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldWriter.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/internal/FieldWriter.java @@ -182,16 +182,4 @@ public static void writeMap(JSONWriter jsonWriter, FieldDescriptor keyDescriptor } jsonWriter.endObject(); } - - /** - * Writes a map field, resolving the synthetic {@code key} field from the value - * descriptor's containing type. Prefer the overload that takes both descriptors - * — callers with a cached schema already have them, and {@code findFieldByName} - * is a hash lookup per call. - */ - public static void writeMap(JSONWriter jsonWriter, FieldDescriptor valueDescriptor, List entries, - ProtobufMessageWriter writer) { - var keyFd = valueDescriptor.getContainingType().findFieldByName("key"); - writeMap(jsonWriter, keyFd, valueDescriptor, entries, writer); - } } diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/internal/MessageSchema.java b/buff-json/src/main/java/io/suboptimal/buffjson/internal/MessageSchema.java index 9723a0a..fedcf12 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/internal/MessageSchema.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/internal/MessageSchema.java @@ -103,19 +103,16 @@ public String jsonName() { return jsonName; } - /** Pre-encoded {@code "fieldName":} in both writer encodings. */ + /** + * Pre-encoded {@code "fieldName":}. Write it with + * {@link FieldName#writeTo(com.alibaba.fastjson2.JSONWriter)} rather than + * reaching for the arrays — a {@code json_name} that is not raw-writable has + * none, and only {@code writeTo} knows to escape it instead. + */ public FieldName name() { return name; } - public char[] nameWithColon() { - return name.chars(); - } - - public byte[] nameWithColonUtf8() { - return name.utf8(); - } - public FieldDescriptor.JavaType javaType() { return javaType; } diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/internal/ProtobufMessageWriter.java b/buff-json/src/main/java/io/suboptimal/buffjson/internal/ProtobufMessageWriter.java index 648a596..d8fe647 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/internal/ProtobufMessageWriter.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/internal/ProtobufMessageWriter.java @@ -27,7 +27,8 @@ * For each message, paths are tried in order: * *

    - *
  1. Codegen — if {@code useGenerated && message instanceof + *
  2. Codegen — if {@code useGenerated}, the writer can take packed + * names ({@link #canUsePackedNames}), and {@code message instanceof * BuffJsonCodecHolder}, delegates to the generated * {@code BuffJsonGeneratedEncoder} (injected via protoc insertion points). * Direct typed accessors, no reflection or boxing. Highest throughput. @@ -47,10 +48,14 @@ *
* *

- * Field-name writes in tiers 1 and 3 dispatch on {@code jsonWriter.isUTF8()} — - * UTF-8 writers consume pre-encoded {@code byte[]} (no char→byte transcoding), - * UTF-16 writers consume {@code char[]}. Tier 2 dispatches inside - * {@link io.suboptimal.buffjson.internal.typed.FieldName#writeTo}. + * Field names: tier 1 writes machine-word-packed names via fastjson2's + * {@code writeNameRaw} family (see {@link FieldNames}) with no encoding + * branch at all; tiers 2 and 3 both go through + * {@link io.suboptimal.buffjson.internal.typed.FieldName#writeTo}, which + * dispatches on {@code jsonWriter.isUTF8()} between a pre-encoded + * {@code byte[]} and {@code char[]}. Because the packed words are only partly + * ours, tier 1 is vetoed for writers it cannot serve — see + * {@link #canUsePackedNames}. * *

* Default value detection uses raw bit comparison for float/double (to @@ -100,7 +105,7 @@ public void writeMessage(JSONWriter jsonWriter, Message message) { */ @SuppressWarnings("unchecked") void writeFields(JSONWriter jsonWriter, Message message) { - if (useGenerated && message instanceof BuffJsonCodecHolder holder) { + if (useGenerated && canUsePackedNames(jsonWriter) && message instanceof BuffJsonCodecHolder holder) { ((BuffJsonGeneratedEncoder) holder.buffJsonEncoder()).writeFields(jsonWriter, message, this); return; } @@ -149,6 +154,30 @@ void writeFields(JSONWriter jsonWriter, Message message) { } } + /** + * Whether the generated encoders' word-packed field names are valid for this + * writer. Generated code emits {@code writeNameRaw} unconditionally, so this + * is the single place that can veto the codegen tier; the typed-accessor and + * reflection tiers own every byte of the name they emit and are always safe. + * + *

+ * Two vetoes, both rare and both checked once per message rather than per + * field: + * + *

    + *
  • {@link FieldNames#PACKED_NAMES_SUPPORTED} — the layout self-check failed + * (a fastjson2 upgrade moved it, or a big-endian host). + *
  • {@code useSingleQuote} — fastjson2 supplies the closing quote from + * {@code this.quote} for names of length 7 and 15 while the packed word carries + * a hard-coded {@code "}, so a single-quote writer would emit {@code "name'}: + * not merely a feature we ignore (the {@code writeNameRaw} arrays have always + * ignored it, emitting valid double-quoted names) but JSON no parser accepts. + *
+ */ + private static boolean canUsePackedNames(JSONWriter jsonWriter) { + return FieldNames.PACKED_NAMES_SUPPORTED && !jsonWriter.useSingleQuote; + } + private static boolean isDefaultValue(MessageSchema.FieldInfo fieldInfo, Object value) { return switch (fieldInfo.javaType()) { case INT -> (int) value == 0; diff --git a/buff-json/src/main/java/io/suboptimal/buffjson/internal/WellKnownTypes.java b/buff-json/src/main/java/io/suboptimal/buffjson/internal/WellKnownTypes.java index 9ae1288..1b88c3e 100644 --- a/buff-json/src/main/java/io/suboptimal/buffjson/internal/WellKnownTypes.java +++ b/buff-json/src/main/java/io/suboptimal/buffjson/internal/WellKnownTypes.java @@ -15,7 +15,6 @@ import com.google.protobuf.*; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; -import com.google.protobuf.Descriptors.OneofDescriptor; /** * Specialized JSON serialization for protobuf @@ -92,9 +91,6 @@ public final class WellKnownTypes { */ private static final ConcurrentHashMap WKT_FIELD_CACHE = new ConcurrentHashMap<>(); - /** Cached {@code kind} oneof per {@code google.protobuf.Value} descriptor. */ - private static final ConcurrentHashMap VALUE_KIND_CACHE = new ConcurrentHashMap<>(); - private WellKnownTypes() { } @@ -401,15 +397,17 @@ private static void writeStruct(JSONWriter jsonWriter, Message message, Protobuf List entries = (List) message.getField(fields[0]); jsonWriter.startObject(); - // Every entry shares the same map-entry descriptor, so resolve key/value once - // rather than hitting the descriptor cache per entry. - FieldDescriptor[] entryFields = entries.isEmpty() ? null : getFields(entries.get(0), "key", "value"); - for (var entry : entries) { - String key = (String) entry.getField(entryFields[0]); - Message value = (Message) entry.getField(entryFields[1]); - jsonWriter.writeName(key); - jsonWriter.writeColon(); - writeValue(jsonWriter, value, writer); + if (!entries.isEmpty()) { + // Every entry shares the same map-entry descriptor, so resolve key/value once + // rather than hitting the descriptor cache per entry. + FieldDescriptor[] entryFields = getFields(entries.get(0), "key", "value"); + for (var entry : entries) { + String key = (String) entry.getField(entryFields[0]); + Message value = (Message) entry.getField(entryFields[1]); + jsonWriter.writeName(key); + jsonWriter.writeColon(); + writeValue(jsonWriter, value, writer); + } } jsonWriter.endObject(); } @@ -426,13 +424,20 @@ private static void writeValue(JSONWriter jsonWriter, Message message, ProtobufM case BOOL_VALUE -> jsonWriter.writeBool(value.getBoolValue()); case STRUCT_VALUE -> writeStruct(jsonWriter, value.getStructValue(), writer); case LIST_VALUE -> writeListValue(jsonWriter, value.getListValue(), writer); + // A kind added by a future protobuf-java would otherwise write nothing at + // all after the caller has already emitted the name and colon, producing + // structurally invalid JSON. javac has no exhaustiveness lint for switch + // statements, so this arm is the guard. + default -> jsonWriter.writeNull(); } return; } - var desc = message.getDescriptorForType(); - var kindOneof = kindOneof(desc); - var activeField = message.getOneofFieldDescriptor(kindOneof); + // DynamicMessage only — compiled Value returned above. getOneofs() allocates a + // list-wrapper pair per call, but caching the result would mean a second + // strong-keyed Descriptor cache pinning the descriptor pool of every schema + // ever loaded, to save two allocations on a cold path. + var activeField = message.getOneofFieldDescriptor(message.getDescriptorForType().getOneofs().get(0)); if (activeField == null) { jsonWriter.writeNull(); @@ -475,20 +480,6 @@ private static void writeListValue(JSONWriter jsonWriter, Message message, Proto jsonWriter.endArray(); } - /** - * Cached {@code kind} oneof for a {@code google.protobuf.Value} descriptor. - * {@code Descriptor.getOneofs()} wraps the backing array in a fresh - * {@code Arrays.asList} + {@code unmodifiableList} on every call — two - * allocations per Value written. - */ - private static OneofDescriptor kindOneof(Descriptor desc) { - var cached = VALUE_KIND_CACHE.get(desc); - if (cached != null) { - return cached; - } - return VALUE_KIND_CACHE.computeIfAbsent(desc, d -> d.getOneofs().get(0)); - } - private static void writeWrapper(JSONWriter jsonWriter, Message message, ProtobufMessageWriter writer) { // Wrapper types have a single "value" field — delegate to FieldWriter // which already handles unsigned formatting, NaN/Infinity, Base64, etc. diff --git a/docs/encoding-performance.md b/docs/encoding-performance.md index f8bda6d..cd08a67 100644 --- a/docs/encoding-performance.md +++ b/docs/encoding-performance.md @@ -21,43 +21,47 @@ confirm. ## Results -Throughput, ops/s, higher is better. Score error was ±2–5% on both sides except where noted. - -| benchmark | before | after | delta | -|-----------------------------------|-----------:|-----------:|-------------------:| -| `SimpleMessage.compiledUtf16` | 9,874,885 | 11,071,259 | +12.1% | -| `SimpleMessage.compiledUtf8` | 11,474,893 | 13,021,993 | +13.5% | -| `SimpleMessage.runtimeUtf16` | 7,485,001 | 8,064,932 | +7.7% | -| `SimpleMessage.runtimeUtf8` | 8,870,673 | 9,099,641 | +2.6% | -| `ComplexMessage.buffJsonCompiled` | 749,489 | 773,347 | +3.2% | -| `ComplexMessage.buffJsonRuntime` | 646,093 | 662,785 | +2.6% | -| `Wkt.structCompiled` | 296,651 | 467,034 | +57.4% | -| `Wkt.structRuntime` | 292,599 | 481,153 | +64.4% | -| `Wkt.timestampCompiled` | 4,736,791 | 4,891,055 | +3.3% | -| `Wkt.timestampRuntime` | 2,839,048 | 2,833,593 | −0.2% | -| `RepeatedAndMap.repeatedCompiled` | 115,094 | 112,168 | −2.5% (±14% after) | -| `RepeatedAndMap.repeatedRuntime` | 94,399 | 94,978 | +0.6% | -| `RepeatedAndMap.mapCompiled` | 95,574 | 94,222 | −1.4% (±7% after) | -| `RepeatedAndMap.mapRuntime` | 76,305 | 76,494 | +0.2% | - -Allocation, `gc.alloc.rate.norm` B/op, lower is better: - -| benchmark | before | after | delta | -|-----------------------------------|-------:|------:|-------:| -| `SimpleMessage.compiledUtf16` | 296 | 208 | −88 | -| `SimpleMessage.compiledUtf8` | 272 | 184 | −88 | -| `SimpleMessage.runtimeUtf16` | 296 | 208 | −88 | -| `SimpleMessage.runtimeUtf8` | 272 | 184 | −88 | -| `ComplexMessage.buffJsonCompiled` | 1,532 | 1,444 | −88 | -| `ComplexMessage.buffJsonRuntime` | 1,396 | 1,308 | −88 | -| `Wkt.structCompiled` / `Runtime` | 1,305 | 648 | −657 | -| `Wkt.timestampCompiled` | 464 | 376 | −88 | -| `RepeatedAndMap.mapRuntime` | 8,021 | 6,727 | −1,294 | -| `RepeatedAndMap.repeated*` | 5,385 | 5,297 | −88 | +Throughput, ops/s, higher is better, from the final interleaved A/B (after the review fixes below). + +| benchmark | before | after | delta | +|-----------------------------------|----------:|-----------:|-------------------:| +| `SimpleMessage.compiledUtf16` | 7,348,223 | 8,455,279 | **+15.1%** | +| `SimpleMessage.compiledUtf8` | 8,927,071 | 10,282,569 | **+15.2%** | +| `SimpleMessage.runtimeUtf16` | 5,228,166 | 5,324,573 | +1.8% | +| `SimpleMessage.runtimeUtf8` | 5,898,804 | 6,286,012 | +6.6% | +| `ComplexMessage.buffJsonCompiled` | 603,853 | 636,926 | +5.5% | +| `ComplexMessage.buffJsonRuntime` | 504,190 | 523,610 | +3.9% | +| `Wkt.structCompiled` | 256,180 | 554,431 | **+116%** | +| `Wkt.structRuntime` | 254,471 | 528,181 | **+108%** | +| `Wkt.timestampCompiled` | 3,940,234 | 3,806,456 | −3.4% | +| `Wkt.timestampRuntime` | 1,982,941 | 2,145,355 | +8.2% | +| `RepeatedAndMap.*` (4) | — | — | flat, within noise | + +**Do not read the absolute numbers as machine-independent.** An identical earlier A/B on this same +host, before other work loaded it, put `SimpleMessage.compiledUtf16` at 9.87M → 11.07M — ~25% +higher on *both* sides. The ratios were stable across the two runs (+12.1% then +15.1%); the +absolutes were not. Struct's ratio moved more (+57% then +116%) because its win is +allocation-dominated, and allocation costs more when the host is busier. Only ever compare two jars +run back to back. + +Allocation, `gc.alloc.rate.norm` B/op, lower is better (stable across runs): + +| benchmark | before | after | delta | +|-----------------------------------|-------:|------:|-----------:| +| `SimpleMessage.compiledUtf16` | 296 | 208 | −88 | +| `SimpleMessage.compiledUtf8` | 272 | 184 | −88 | +| `SimpleMessage.runtimeUtf16` | 296 | 208 | −88 | +| `SimpleMessage.runtimeUtf8` | 272 | 184 | −88 | +| `ComplexMessage.buffJsonCompiled` | 1,532 | 1,444 | −88 | +| `ComplexMessage.buffJsonRuntime` | 1,396 | 1,308 | −88 | +| `Wkt.struct*` | 1,305 | 648 | **−657** | +| `Wkt.timestampCompiled` | 464 | 376 | −88 | +| `RepeatedAndMap.mapRuntime` | 8,021 | 6,727 | **−1,294** | +| `RepeatedAndMap.repeated*` | 5,385 | 5,297 | −88 | The flat −88 B/op everywhere is the per-call `JSONWriter.Context`. For `SimpleMessage` that is 30% -of the total allocation — after the change the returned `String`/`byte[]` is essentially the only -thing allocated per encode. Budgets in `allocation-check.sh` were tightened to match. +of the total — after the change the returned `String`/`byte[]` is essentially the only thing +allocated per encode. Budgets in `allocation-check.sh` were tightened to match. ## Part 1 — Applied @@ -101,11 +105,31 @@ reasoning is recorded in `FieldName`'s javadoc so it does not get "fixed" again. Where the win lands: +12–13% on `SimpleMessage` codegen, which is six fields and ~80 bytes of output — i.e. the shape where field-name writing is the largest single cost. +**Where the packed words are not usable.** The fast path is only partly ours, and two things +invalidate it — both found by review after the first implementation, both now gated in one place +(`ProtobufMessageWriter.canUsePackedNames`, one check per message, falling back to the typed tier +which the fuzz test proves is byte-identical): + +- **`UseSingleQuotes`.** The quote characters are split between us and fastjson2, and the split + differs per length: both quotes are baked into the word for lengths 2-6 and 9-14, only the + opening quote for 7 and 15, neither for 8 and 16. fastjson2 emits its share from `this.quote`, + so a single-quote writer produced `{"optionalFixed32':7}` — **unparseable**, verified. The + `writeNameRaw` arrays had always *ignored* the feature and emitted valid double-quoted names, so + this was a regression from "ignores a feature" to "emits garbage", not a pre-existing gap. +- **Byte order.** `JSONWriterUTF8` stores the word with `Unsafe.putLong` (native order), but + `JSONWriterUTF16.putLong(char[], int, long)` widens it by extracting bytes at hard-coded + little-endian positions (`v & 0xFF`, `(v & 0xFF00) << 8`, ...) with **no `BIG_ENDIAN` branch** — + confirmed by disassembly. One constant cannot satisfy both on a big-endian host, so the original + javadoc claim that native-order packing is endian-agnostic was wrong. + +`FieldNames.PACKED_NAMES_SUPPORTED` therefore self-checks the layout at class init: it packs a +probe name of every length 2-16, writes it through both encodings, and compares the text. That +turns a big-endian host *or* a future fastjson2 that moves the layout from silent corruption of +every field name into a clean fallback. + *Not supported, before or after:* JSONB output. `JSONWriterJSONB.writeNameRaw(byte[])` forwards to `writeRaw(byte[])`, which dumps JSON *text* bytes into a JSONB stream — so routing a protobuf -message through `JSONB.toBytes` via `writerModule()` produced garbage already. `writeNameNRaw` -produces different garbage. Also unchanged: `UseSingleQuotes` was, and still is, ignored for field -names. +message through `JSONB.toBytes` via `writerModule()` produced garbage already. ### 2. No boxing on repeated primitives (codegen + typed path) — no measured win @@ -122,11 +146,19 @@ scalar-replacing the boxes: `IntArrayList.get` inlines, the `Integer` never esca escape analysis deletes it. If boxing were really allocating there, it alone would have been ~2,000 of the 5,385 B/op measured. -Kept anyway, as insurance rather than a win: the elimination depends on the loop inlining, which -does not hold in the interpreter or at C1 (relevant for short-lived processes that never reach -C2), nor if a large `writeFields` body pushes the loop past the inlining budget. Do not expect it -to show up in a benchmark. **This is the cautionary tale of the audit** — "obvious" boxing on a -hot path is often already gone. +Kept anyway, but only for one reason: escape analysis is a C2 optimization, so the boxing is real +in the interpreter and at C1 — which is where a short-lived process (a CLI, a scale-to-zero +function) spends most of its time. Do not expect it to show up in a steady-state benchmark. +**This is the cautionary tale of the audit** — "obvious" boxing on a hot path is often already +gone. + +The codegen half costs generated bytecode, since the primitive and generic loops are both emitted +per repeated field. Worth knowing the ceiling: HotSpot's `DontCompileHugeMethods` refuses to +JIT-compile *any* method over 8,000 bytecodes, at which point the encoder runs interpreted. The +largest generated `writeFields` in this repo is `TestAllTypesProto3JsonEncoder` at **3,240 +bytecodes** (~200 fields, the official conformance sample), so there is ample headroom — but a wide +message with many repeated primitive fields is the shape that approaches it, and no benchmark is +near that size. If codegen grows further per field, measure a wide message before assuming. The typed path also gained `RepeatedDouble/Float/BoolAccessor`; those types previously fell through to the generic `RepeatedAccessor`, which re-switched on `JavaType` per element. @@ -147,9 +179,14 @@ field's `String` name. `Struct`, `Value` and `ListValue` are ordinary compiled classes in protobuf-java, so the writer now takes a typed path when it has one: `struct.getFieldsMap()` instead of materializing the synthetic MapEntry list and pulling key/value back out through `getField()`, and -`value.getKindCase()` — an int switch — instead of the descriptor dance. `DynamicMessage` keeps -the reflective path, with the per-entry `getFields(entry, "key", "value")` lookup hoisted out of -the loop (it was a cache probe *per struct entry*) and the `kind` oneof cached per Descriptor. +`value.getKindCase()` — an int switch — instead of the descriptor dance. The switch carries a +`default -> writeNull()` arm: a kind added by a future protobuf-java would otherwise write nothing +after the caller has already emitted the name and colon, i.e. `{"k":}`, and javac has no +exhaustiveness lint for switch *statements*. `DynamicMessage` keeps the reflective path, with the +per-entry `getFields(entry, "key", "value")` lookup hoisted out of the loop (it was a cache probe +*per struct entry*). The `kind` oneof is deliberately **not** cached: a strong-keyed `Descriptor` +map would pin the descriptor pool of every schema ever loaded — a real leak for services that +re-parse `.desc` files — to save two allocations on a path only `DynamicMessage` reaches. **+57%/+64% throughput and −657 B/op** — by far the biggest single win in the audit, and the one that was easiest to miss, because it was hiding inside a well-known-type helper rather than on the @@ -172,8 +209,9 @@ immediately discarded. `FieldWriter.writeMap` called `findFieldByName("key")` and `findFieldByName("value")` — two hash lookups — on *every map field write*. Both are now resolved when the schema is built -(`MessageSchema.FieldInfo.mapKeyDescriptor()`, `TypedFieldAccessor.MapAccessor`). The old -two-argument overload is kept for callers without a cached schema. +(`MessageSchema.FieldInfo.mapKeyDescriptor()`, `TypedFieldAccessor.MapAccessor`). Both remaining +callers have a cached schema, so `writeMap` takes both descriptors and the old +`findFieldByName`-resolving overload is gone. ### 8. Non-ASCII `json_name` (a correctness fix found on the way) @@ -188,9 +226,25 @@ any string, and: `FieldNames.isRawWritable` now gates the raw arrays (printable ASCII, nothing escapable), anything else goes through `JSONWriter.writeName(String)` + `writeColon()` so fastjson2 escapes and transcodes it, and `FieldNames.javaStringLiteral` (mirrored in the generator) escapes emitted -literals. Covered by `FieldNamesTest`. +literals. + +A subtlety worth recording, because the first attempt got it wrong: a line terminator **must** be +emitted as `\n`/`\r`, never as a unicode escape. javac translates unicode escapes *before* +tokenizing (JLS 3.3), so `\u000a` turns back into a real newline and leaves the literal unclosed — +the same uncompilable output, now harder to spot. `FieldNamesTest` compiles the emitted literal +with `javax.tools.JavaCompiler` and reads the constant back, rather than asserting the weaker +"body is printable ASCII" property that a broken `\u000a` satisfies. -### 9. One `JSONWriter.Context` per encoder +### 9. Deprecated fields (a second correctness fix) + +The name-constant loop skipped `[deprecated = true]` fields while the `writeFields` loop did not, +so a deprecated field emitted a name write referencing a constant that was never declared — +generated source that does not compile. No `.proto` in the repo has a deprecated field, so CI was +green and this would have broken the first user build that did. Deprecated fields now serialize +like any other (matching `JsonFormat` and both runtime paths), and both generators emit +`@SuppressWarnings("deprecation")` on the class when needed, since consumers build with `-Werror`. + +### 10. One `JSONWriter.Context` per encoder `JSONWriter.of()` allocates a fresh `JSONWriter.Context` per call to carry configuration that never changes between encodes. `BuffJsonEncoder` now holds one and passes it to @@ -200,6 +254,12 @@ context is read-only during writing. **−88 B/op on every benchmark**, which on `SimpleMessage` is 30% of the total. The cheapest item in the audit by a wide margin: five lines. +One behavioural caveat, documented on the field rather than left implicit: `JSONWriter.Context` +copies fastjson2's mutable global defaults (`defaultWriterFeatures`, `defaultWriterZoneId`, +`defaultMaxLevel`, `defaultWriterFormat`) at construction. `JSONWriter.of()` re-read them per call, +so a `JSON.config(...)` executed *after* an encoder was built used to take effect on the next +encode and now does not. Configure fastjson2 before constructing encoders. + ## Part 2 — Backlog, highest value first ### B1. Devirtualize `JSONWriter` (generate per-encoding `writeFields` bodies)