diff --git a/codecs/json-codec/src/main/java/software/amazon/smithy/java/json/smithy/JsonReadUtils.java b/codecs/json-codec/src/main/java/software/amazon/smithy/java/json/smithy/JsonReadUtils.java index 31e69ae047..87524f07b4 100644 --- a/codecs/json-codec/src/main/java/software/amazon/smithy/java/json/smithy/JsonReadUtils.java +++ b/codecs/json-codec/src/main/java/software/amazon/smithy/java/json/smithy/JsonReadUtils.java @@ -206,8 +206,9 @@ static void parseString(byte[] buf, int pos, int end, SmithyJsonDeserializer des while (pos < end) { byte b = buf[pos]; if (b == '"') { - // No escapes found -- fast path - deser.parsedString = new String(buf, start, pos - start, StandardCharsets.UTF_8); + // No escapes found -- fast path. Dedup short strings through the + // deserializer's per-document cache (repeated keys/values are common). + deser.parsedString = deser.decodeUtf8Cached(buf, start, pos - start); deser.parsedEndPos = pos + 1; return; } diff --git a/codecs/json-codec/src/main/java/software/amazon/smithy/java/json/smithy/SmithyJsonDeserializer.java b/codecs/json-codec/src/main/java/software/amazon/smithy/java/json/smithy/SmithyJsonDeserializer.java index 9c8fb76c11..a4109c2576 100644 --- a/codecs/json-codec/src/main/java/software/amazon/smithy/java/json/smithy/SmithyJsonDeserializer.java +++ b/codecs/json-codec/src/main/java/software/amazon/smithy/java/json/smithy/SmithyJsonDeserializer.java @@ -16,6 +16,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicReferenceArray; import software.amazon.smithy.java.codecs.commons.NumberCodec; import software.amazon.smithy.java.core.schema.Schema; import software.amazon.smithy.java.core.serde.SerializationException; @@ -51,6 +52,42 @@ final class SmithyJsonDeserializer implements ShapeDeserializer { double parsedDouble; String parsedString; + // Short-string dedup cache. Repeated JSON keys/values (common in collections) are decoded + // once and shared. The packed bytes of a string <= 8 bytes form an exact identity key (every + // content byte on the no-escape fast path is >= 0x20, so leading bytes are non-zero and length + // is encoded implicitly), so a hit returns the shared String with no byte comparison. + // + // The arrays are reused across documents via a striped pool (allocating ~4 KB per parse + // measurably regresses small payloads). Reuse without clearing is safe: the packed key is the + // exact bytes, so a stale entry from a prior document is still byte-identical to any new match. + private static final int STR_CACHE_SIZE = 256; // power of two + private static final int STR_CACHE_MASK = STR_CACHE_SIZE - 1; + + /** Reusable dedup arrays, pooled per the pattern in {@link SmithyJsonSerializer}. */ + static final class StringCache { + final long[] keys = new long[STR_CACHE_SIZE]; + final String[] vals = new String[STR_CACHE_SIZE]; + } + + // Striped cache pool (mirrors SmithyJsonSerializer's serializer pool). Shared by platform and + // virtual threads alike — see acquireCache for why this is safe and memory-bounded under high + // virtual-thread concurrency. + private static final int CACHE_POOL_SLOTS; + private static final int CACHE_POOL_MASK; + private static final AtomicReferenceArray CACHE_POOL; + private static final int CACHE_MAX_PROBE = 3; + + static { + int raw = Runtime.getRuntime().availableProcessors() * 4; + CACHE_POOL_SLOTS = Integer.highestOneBit(raw - 1) << 1; + CACHE_POOL_MASK = CACHE_POOL_SLOTS - 1; + CACHE_POOL = new AtomicReferenceArray<>(CACHE_POOL_SLOTS); + } + + private long[] strCacheKeys; // null until the first short string is decoded + private String[] strCacheVals; + private StringCache pooledCache; // non-null when the arrays came from the pool + SmithyJsonDeserializer(byte[] buf, int pos, int end, JsonSettings settings) { this.buf = buf; this.pos = pos; @@ -64,6 +101,7 @@ final class SmithyJsonDeserializer implements ShapeDeserializer { @Override public void close() { + releaseCache(); // Verify no trailing non-whitespace content int p = JsonReadUtils.skipWhitespace(buf, pos, end); if (p < end) { @@ -72,6 +110,94 @@ public void close() { } } + private static int cachePoolProbe() { + long id = Thread.currentThread().threadId(); + return (int) (id ^ (id >>> 16)) & CACHE_POOL_MASK; + } + + /** Returns the pooled cache for reuse by a later parse on this thread group. */ + private void releaseCache() { + StringCache cache = pooledCache; + if (cache == null) { + return; + } + pooledCache = null; + strCacheKeys = null; + strCacheVals = null; + int base = cachePoolProbe(); + for (int i = 0; i < CACHE_MAX_PROBE; i++) { + int idx = (base + i) & CACHE_POOL_MASK; + if (CACHE_POOL.getPlain(idx) == null + && CACHE_POOL.compareAndExchangeRelease(idx, null, cache) == null) { + return; + } + } + // Pool full — let GC collect. + } + + /** + * Decodes an unescaped string, deduplicating short (<= 8 byte) strings through a + * per-document cache. The packed bytes form an exact identity (see field docs), so a + * cache hit returns a shared, immutable String with no byte comparison. On a collision + * the existing entry is overwritten (bounded memory), and on a miss the freshly decoded + * String is cached. Strings longer than 8 bytes bypass the cache. + */ + String decodeUtf8Cached(byte[] buf, int start, int len) { + if (len == 0) { + return ""; + } + if (len > 8) { + return new String(buf, start, len, StandardCharsets.UTF_8); + } + // Pack bytes into a long. Every content byte is >= 0x20 here (the no-escape fast + // path rejects control bytes), so leading bytes are non-zero and length is encoded + // implicitly — distinct (bytes,length) pairs never collide on the packed key. + long key = 0; + for (int i = 0; i < len; i++) { + key = (key << 8) | (buf[start + i] & 0xFFL); + } + long[] keys = strCacheKeys; + String[] vals = strCacheVals; + if (keys == null) { + StringCache cache = acquireCache(); + keys = strCacheKeys = cache.keys; + vals = strCacheVals = cache.vals; + } + // Fibonacci hash: mix so short keys that differ only in low bits spread out. + int slot = (int) ((key * 0x9E3779B97F4A7C15L) >>> 48) & STR_CACHE_MASK; + if (keys[slot] == key) { + return vals[slot]; + } + String s = new String(buf, start, len, StandardCharsets.UTF_8); + keys[slot] = key; + vals[slot] = s; + return s; + } + + /** + * Acquires a dedup cache from the striped pool (or allocates a fresh one). Mirrors + * {@link SmithyJsonSerializer#acquire}, including its virtual-thread handling: the + * shared pool serves platform and virtual threads alike. The CAS to null gives the + * caller exclusive ownership until {@link #releaseCache} (safe across carrier remounts + * — deserialization never blocks), and the in-flight count tracks concurrent parses + * (≈ carrier count), not the virtual-thread count, so memory stays bounded. Entries + * are reused as-is — see field docs for why reusing a populated cache is correct. + */ + private StringCache acquireCache() { + int base = cachePoolProbe(); + for (int i = 0; i < CACHE_MAX_PROBE; i++) { + int idx = (base + i) & CACHE_POOL_MASK; + StringCache c = CACHE_POOL.getPlain(idx); + if (c != null && CACHE_POOL.compareAndExchangeAcquire(idx, c, null) == c) { + pooledCache = c; + return c; + } + } + StringCache c = new StringCache(); + pooledCache = c; + return c; + } + @Override public boolean readBoolean(Schema schema) { skipWhitespace(); @@ -268,7 +394,11 @@ public Instant readTimestamp(Schema schema) { fracPos++; } int fracLen = fracPos - fracStart; - if (fracLen > 0) { + // Skip the precision fast path if an exponent follows — the precision + // fast path doesn't apply scientific notation and would leave pos before + // the 'e'/'E', corrupting subsequent parsing. + boolean hasExponent = fracPos < end && (buf[fracPos] == 'e' || buf[fracPos] == 'E'); + if (fracLen > 0 && !hasExponent) { int nano = 0; for (int i = 0; i < 9; i++) { nano *= 10; @@ -291,7 +421,7 @@ public Instant readTimestamp(Schema schema) { throw new SerializationException("Epoch seconds out of range: " + parsedLong, e); } } - // No digits after dot -- fall through to double parsing + // No digits after dot, or exponent present -- fall through to double parsing } else if (endPos >= end || (buf[endPos] != 'e' && buf[endPos] != 'E')) { // Pure integer -- no fractional part pos = endPos; diff --git a/codecs/json-codec/src/main/java/software/amazon/smithy/java/json/smithy/SmithyMemberLookup.java b/codecs/json-codec/src/main/java/software/amazon/smithy/java/json/smithy/SmithyMemberLookup.java index 34f5f8f0f2..4c194e64b5 100644 --- a/codecs/json-codec/src/main/java/software/amazon/smithy/java/json/smithy/SmithyMemberLookup.java +++ b/codecs/json-codec/src/main/java/software/amazon/smithy/java/json/smithy/SmithyMemberLookup.java @@ -28,11 +28,23 @@ final class SmithyMemberLookup implements MemberLookup { final Schema[] orderedSchemas; final byte[][] orderedNameBytes; + // Field names of length 1..7 packed into a long as ((len << 56) | big-endian bytes). + // The length lives in the top byte and the <=7 name bytes in the low 56 bits, so the + // packed value is a collision-free identity (encoding length defends against inputs + // with leading 0x00 bytes, which the struct field-name scanner does not reject). This + // lets the common short-name lookup replace the FNV byte-loop + Arrays.equals with a + // handful of `long ==` comparisons. Entry is 0 for names of length 0 or >= 8 (sentinel); + // the packed-path is only entered for input lengths 1..7, whose key has a non-zero top + // byte and so never matches the 0 sentinel. + static final int PACK_MAX_LEN = 7; + final long[] orderedPackedNames; + SmithyMemberLookup(List members, boolean useJsonName) { int size = members.size(); this.orderedHashes = new long[size]; this.orderedSchemas = new Schema[size]; this.orderedNameBytes = new byte[size][]; + this.orderedPackedNames = new long[size]; for (int i = 0; i < size; i++) { Schema m = members.get(i); @@ -46,10 +58,24 @@ final class SmithyMemberLookup implements MemberLookup { byte[] nameBytes = fieldName.getBytes(StandardCharsets.UTF_8); orderedNameBytes[i] = nameBytes; orderedHashes[i] = fnvHash(nameBytes, 0, nameBytes.length); + int len = nameBytes.length; + orderedPackedNames[i] = (len >= 1 && len <= PACK_MAX_LEN) ? packName(nameBytes, 0, len) : 0L; orderedSchemas[i] = m; } } + /** + * Packs a name of length 1..7 into {@code (len << 56) | big-endian bytes}. + * Caller ensures {@code 1 <= len <= 7}. + */ + private static long packName(byte[] buf, int start, int len) { + long key = (long) len << 56; + for (int i = 0; i < len; i++) { + key |= (buf[start + i] & 0xFFL) << ((len - 1 - i) << 3); + } + return key; + } + /** * Looks up a member by matching the field name bytes directly from the input buffer. * No String allocation on the common path. @@ -74,14 +100,27 @@ final class SmithyMemberLookup implements MemberLookup { Schema lookup(byte[] buf, int start, int end, int expectedNext) { int nameLen = end - start; - // Speculative fast path: Arrays.equals only, no hash. - if (expectedNext >= 0 && expectedNext < orderedNameBytes.length - && orderedNameBytes[expectedNext].length == nameLen - && Arrays.equals(buf, start, end, orderedNameBytes[expectedNext], 0, nameLen)) { - return orderedSchemas[expectedNext]; + // Short-name fast path: names of length 1..7 pack into a long that is an exact + // identity (see orderedPackedNames). A single linear scan of long== handles both + // the speculative-miss and out-of-order cases without FNV hashing or Arrays.equals. + // This is the union discriminator's hot path (S/N/B/M/L/BOOL/... in AttributeValue), + // where the speculative guess always misses for any member after the first. + if (nameLen >= 1 && nameLen <= PACK_MAX_LEN) { + long key = packName(buf, start, nameLen); + long[] packed = orderedPackedNames; + // Check the speculative position first to preserve in-order locality. + if (expectedNext >= 0 && expectedNext < packed.length && packed[expectedNext] == key) { + return orderedSchemas[expectedNext]; + } + for (int i = 0; i < packed.length; i++) { + if (packed[i] == key) { + return orderedSchemas[i]; + } + } + return null; } - // Slow path: compute hash lazily, then scan with hash + length + equals. + // Long-name path: compute hash lazily, then scan with hash + length + equals. long hash = fnvHash(buf, start, end); for (int i = 0; i < orderedHashes.length; i++) { if (orderedHashes[i] == hash diff --git a/codecs/json-codec/src/test/java/software/amazon/smithy/java/json/JsonDeserializerTest.java b/codecs/json-codec/src/test/java/software/amazon/smithy/java/json/JsonDeserializerTest.java index 8064cd58fd..f5af18715b 100644 --- a/codecs/json-codec/src/test/java/software/amazon/smithy/java/json/JsonDeserializerTest.java +++ b/codecs/json-codec/src/test/java/software/amazon/smithy/java/json/JsonDeserializerTest.java @@ -1238,6 +1238,39 @@ public void rejectsEpochSecondsOutOfRange(JsonSerdeProvider provider) { }); } + @ParameterizedTest + @MethodSource("epochSecondsWithExponentSource") + public void parsesEpochSecondsWithExponent(JsonSerdeProvider provider, String json, Instant expected) { + // Regression: a fractional epoch-seconds value that also carries an exponent (e.g. "1.5e3") + // used to take the nanosecond-precision fast path, which ignored the exponent — decoding the + // fraction as nanos and leaving the cursor before the 'e'. That both produced the wrong Instant + // and corrupted subsequent parsing. Such values must fall through to full double parsing. + var schema = Schema.createTimestamp( + ShapeId.from("smithy.foo#Time"), + new TimestampFormatTrait(TimestampFormatTrait.EPOCH_SECONDS)); + try (var codec = codecBuilder(provider).useTimestampFormat(true).build()) { + var de = codec.createDeserializer(json.getBytes(StandardCharsets.UTF_8)); + assertThat(de.readTimestamp(schema), equalTo(expected)); + } + } + + public static List epochSecondsWithExponentSource() { + List args = new ArrayList<>(); + for (var provider : List.of(JACKSON, SMITHY)) { + // Fraction + exponent: 1.5e3 == 1500s. The buggy path returned Instant(1, 500_000_000). + args.add(Arguments.of(provider, "1.5e3", Instant.ofEpochSecond(1500))); + // Uppercase exponent. + args.add(Arguments.of(provider, "1.5E3", Instant.ofEpochSecond(1500))); + // Fraction that survives into the result: 1.5e1 == 15s. + args.add(Arguments.of(provider, "1.5e1", Instant.ofEpochSecond(15))); + // Negative exponent keeps a sub-second fraction: 1500e-3 == 1.5s. + args.add(Arguments.of(provider, "1500e-3", Instant.ofEpochSecond(1, 500_000_000))); + // No fractional part, exponent only: 1e3 == 1000s (already worked; guards against regression). + args.add(Arguments.of(provider, "1e3", Instant.ofEpochSecond(1000))); + } + return args; + } + @PerProvider public void timestampFallbackForOffsetTimezone(JsonSerdeProvider provider) { var schema = Schema.createTimestamp( diff --git a/codecs/json-codec/src/test/java/software/amazon/smithy/java/json/JsonVirtualThreadPoolingTest.java b/codecs/json-codec/src/test/java/software/amazon/smithy/java/json/JsonVirtualThreadPoolingTest.java new file mode 100644 index 0000000000..4f36999f4a --- /dev/null +++ b/codecs/json-codec/src/test/java/software/amazon/smithy/java/json/JsonVirtualThreadPoolingTest.java @@ -0,0 +1,89 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.java.json; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.Test; +import software.amazon.smithy.java.json.bench.model.SimpleStruct; +import software.amazon.smithy.java.json.smithy.SmithyJsonSerdeProvider; + +/** + * Stresses the serializer pool and the deserializer string-dedup cache pool from many + * virtual threads concurrently. Both pools are shared across platform and virtual threads; + * if a pooled serializer or cache were ever handed to two threads at once, concurrent writes + * would corrupt output and the per-thread round-trip equality check below would fail. + */ +public class JsonVirtualThreadPoolingTest { + + private static final JsonCodec CODEC = JsonCodec.builder() + .overrideSerdeProvider(new SmithyJsonSerdeProvider()) + .useTimestampFormat(true) + .build(); + + @Test + void concurrentVirtualThreadRoundtripsAreIsolated() throws Exception { + int threads = 2_000; + int iterationsPerThread = 50; + + // Each thread serializes a struct unique to it (distinct string fields) so that any + // cross-thread cache/serializer bleed produces a value mismatch, not a silent pass. + var startGate = new CountDownLatch(1); + var failure = new AtomicReference(); + var mismatches = new AtomicInteger(); + var completed = new AtomicInteger(); + + try (ExecutorService exec = Executors.newVirtualThreadPerTaskExecutor()) { + List> futures = new ArrayList<>(threads); + for (int t = 0; t < threads; t++) { + final int id = t; + futures.add(exec.submit(() -> { + try { + startGate.await(); + for (int i = 0; i < iterationsPerThread; i++) { + var original = SimpleStruct.builder() + .name("thread-" + id + "-iter-" + i) + .age(id) + .active((id & 1) == 0) + .score(id + i / 100.0) + .build(); + ByteBuffer serialized = CODEC.serialize(original); + byte[] bytes = new byte[serialized.remaining()]; + serialized.get(bytes); + var roundtripped = CODEC.deserializeShape(bytes, SimpleStruct.builder()); + if (!roundtripped.equals(original)) { + mismatches.incrementAndGet(); + } + completed.incrementAndGet(); + } + } catch (Throwable e) { + failure.compareAndSet(null, e); + } + })); + } + startGate.countDown(); + for (var f : futures) { + f.get(60, TimeUnit.SECONDS); + } + } + + if (failure.get() != null) { + throw new AssertionError("Virtual thread task threw", failure.get()); + } + assertThat(completed.get()).isEqualTo(threads * iterationsPerThread); + assertThat(mismatches.get()).as("round-trip mismatches under concurrent VT pooling").isZero(); + } +} diff --git a/codecs/json-codec/src/test/java/software/amazon/smithy/java/json/smithy/SmithyMemberLookupTest.java b/codecs/json-codec/src/test/java/software/amazon/smithy/java/json/smithy/SmithyMemberLookupTest.java new file mode 100644 index 0000000000..df6f68a09d --- /dev/null +++ b/codecs/json-codec/src/test/java/software/amazon/smithy/java/json/smithy/SmithyMemberLookupTest.java @@ -0,0 +1,98 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.java.json.smithy; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.nio.charset.StandardCharsets; +import java.util.List; +import org.junit.jupiter.api.Test; +import software.amazon.smithy.java.core.schema.PreludeSchemas; +import software.amazon.smithy.java.core.schema.Schema; +import software.amazon.smithy.model.shapes.ShapeId; + +/** + * Tests the packed-long short-name fast path in {@link SmithyMemberLookup#lookup}. + * + *

Names of length 1..7 are matched by packing their bytes into a long together with + * the length; these tests guard that this packing is an exact identity (no collisions + * across different lengths or against control-byte inputs) and that longer names still + * resolve via the FNV fallback. + */ +public class SmithyMemberLookupTest { + + private static SmithyMemberLookup lookupOf(String... memberNames) { + var builder = Schema.structureBuilder(ShapeId.from("test#S")); + for (String n : memberNames) { + builder.putMember(n, PreludeSchemas.STRING); + } + Schema schema = builder.build(); + return new SmithyMemberLookup(schema.members(), false); + } + + private static Schema lookup(SmithyMemberLookup l, String name) { + byte[] b = name.getBytes(StandardCharsets.UTF_8); + // Embed the name in a larger buffer at a non-zero offset to catch offset bugs. + byte[] buf = new byte[b.length + 5]; + System.arraycopy(b, 0, buf, 3, b.length); + return l.lookup(buf, 3, 3 + b.length, /* expectedNext */ -1); + } + + @Test + public void matchesShortUnionDiscriminators() { + // The DynamoDB AttributeValue union shape: all discriminators are <= 4 bytes. + var l = lookupOf("S", "N", "B", "SS", "NS", "BS", "M", "L", "NULL", "BOOL"); + for (String name : List.of("S", "N", "B", "SS", "NS", "BS", "M", "L", "NULL", "BOOL")) { + assertThat(lookup(l, name)).as(name).isNotNull(); + assertThat(lookup(l, name).memberName()).isEqualTo(name); + } + } + + @Test + public void differentLengthSamePrefixDoNotCollide() { + // "S" and "SS" share a prefix but differ in length: folding length into the + // packed key must keep them distinct. + var l = lookupOf("S", "SS"); + assertThat(lookup(l, "S").memberName()).isEqualTo("S"); + assertThat(lookup(l, "SS").memberName()).isEqualTo("SS"); + } + + @Test + public void unknownShortNameReturnsNull() { + var l = lookupOf("S", "N"); + assertThat(lookup(l, "X")).isNull(); + assertThat(lookup(l, "NS")).isNull(); // not a member here + } + + @Test + public void leadingControlByteDoesNotSpuriouslyMatch() { + // A 2-byte input {0x00, 'S'} must not match member "S" (1 byte) or any other. + // Without folding length into the key, naive big-endian packing of {0x00,'S'} + // equals the packing of {'S'} and would mis-dispatch. + var l = lookupOf("S", "N"); + byte[] buf = {0x00, (byte) 'S'}; + assertThat(l.lookup(buf, 0, 2, -1)).isNull(); + } + + @Test + public void boundaryLengthsSevenAndEight() { + // 7 bytes uses the packed path; 8 bytes falls back to FNV. Both must resolve. + var l = lookupOf("seven77", "eight888"); + assertThat(lookup(l, "seven77").memberName()).isEqualTo("seven77"); + assertThat(lookup(l, "eight888").memberName()).isEqualTo("eight888"); + assertThat(lookup(l, "seven78")).isNull(); + assertThat(lookup(l, "eight889")).isNull(); + } + + @Test + public void longNamesUseFnvFallback() { + var l = lookupOf("TableName", "CapacityUnits", "ReadCapacityUnits"); + assertThat(lookup(l, "TableName").memberName()).isEqualTo("TableName"); + assertThat(lookup(l, "CapacityUnits").memberName()).isEqualTo("CapacityUnits"); + assertThat(lookup(l, "ReadCapacityUnits").memberName()).isEqualTo("ReadCapacityUnits"); + assertThat(lookup(l, "WriteCapacityUnits")).isNull(); + } +}