diff --git a/.agents/languages/java.md b/.agents/languages/java.md index 9367fd1ddf..d1005216a1 100644 --- a/.agents/languages/java.md +++ b/.agents/languages/java.md @@ -110,6 +110,11 @@ Load this file when changing anything under `java/` or when Java drives a cross- published. Do not extend this exception to another cache or retained value. - Concrete serializers may opt into sharing only after auditing retained fields. Treat serializers retaining `TypeResolver`, `RefResolver`, mutable scratch buffers, runtime state, or classloader-sensitive state as non-shareable unless that state is externalized. - Resolver and serializer hot paths should keep the fast-path/null-slow-path shape obvious. Hoist repeated buffer or cache-state access into locals for multi-step operations and keep rebuild/restoration logic cold. +- Java compatible metadata hash caches and depth hints retain the source `TypeInfo`, before + requested-target adaptation. Store target-specific results in the existing `transformedTypeInfo` + cache, keyed by target `Class` identity with source `Class` and primitive header-hash comparisons + in its entries; do not allocate tuple keys. Resolve local schemas only on metadata-cache or + target-conversion-cache misses. A hit must not repeat `matchingLocalTypeDef` or `getTypeDef`. - Remote metadata and class-token paths that materialize Java classes must keep `TypeResolver.loadClass` or an equivalent owner in the path so `TypeChecker.checkType` and `DisallowedList` run on the remote class name diff --git a/AGENTS.md b/AGENTS.md index b6d9202a1e..8f9c62c787 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,6 +29,9 @@ This is the entry point for AI guidance in Apache Fory. Read this file first, th ## Agent Operating Rules +- Keep only rules shared across multiple languages in `AGENTS.md`. Put language-specific rules + and corrections in `.agents/languages/.md`, including language-specific details of a + shared rule. Do not duplicate those rules in `AGENTS.md`. - Preserve architecture. Do not introduce new layers, parallel flows, or public APIs unless explicitly requested; prefer local repair in the existing owner over shared-infra expansion, and stop if a fix conflicts with an ADR, spec, or invariant. - Do not change an existing `RefReader`/`RefWriter` architecture or API to support compatible skip. Compatible skip must not add alternate reference slots or tables, alternate reference lookup or publication methods, or forwarding APIs in read/write contexts, builders, serializers, or generated-code plumbing. Keep ordinary reference publication and lookup unchanged and resolve the case in the existing compatible generated owner. For an authorized removed-field read of an unregistered Struct, the empty object created by the skip reader is that path's final owner: publish that same object for `RefValue`, consume the Struct fields, and let later `RefFlag` values resolve to it. This preserves reference numbering and identity without registering the Struct; an independent dynamic root still requires normal registration. Do not add parallel reference state, a sentinel, a rejection, or a common-path branch for this case. - Respect ownership. Keep logic, state, and helpers in their natural owner, and do not move serializer-local, context-local, runtime-type-local, or protocol-local problems into global utilities. diff --git a/docs/specification/xlang_implementation_guide.md b/docs/specification/xlang_implementation_guide.md index e4c38caacf..84335495fd 100644 --- a/docs/specification/xlang_implementation_guide.md +++ b/docs/specification/xlang_implementation_guide.md @@ -1205,6 +1205,12 @@ Do not retain or compare metadata bytes or fields, thread extra expected-type parameters through callers for revalidation, or add parallel accepted-header state. Cache hits never repeat miss-time work. +In Java, the header hash identifies the wire schema, while a requested target class can require a +different `TypeInfo` for that same schema. Hash-only metadata caches and depth hints retain the +source `TypeInfo`. The existing target-conversion cache retains the result for each target class, +source class, and header hash. Local-schema selection occurs only on a metadata or target-conversion +cache miss; subsequent hits reuse the selected result without querying local TypeDef metadata. + When a statically declared compatible named enum, ext, or union field reads shared metadata, the decoded metadata must match the declared type id, namespace, and type name before the metadata owner publishes it to the diff --git a/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java b/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java index 1feb1bee7f..648bfd8b0d 100644 --- a/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java +++ b/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java @@ -726,7 +726,8 @@ public final TypeInfo readTypeInfo(ReadContext readContext, Class targetClass case Types.COMPATIBLE_STRUCT: case Types.NAMED_COMPATIBLE_STRUCT: typeInfo = readSharedClassMeta(readContext, targetClass, cachedTypeInfo); - break; + // readSharedClassMeta caches the source TypeInfo, before target adaptation. + return typeInfo.serializer == null ? ensureSerializerForTypeInfo(typeInfo) : typeInfo; case Types.NAMED_ENUM: case Types.NAMED_STRUCT: case Types.NAMED_EXT: @@ -735,6 +736,7 @@ public final TypeInfo readTypeInfo(ReadContext readContext, Class targetClass typeInfo = readTypeInfoFromBytes(readContext, cachedTypeInfo, typeId); } else { typeInfo = readSharedClassMeta(readContext, targetClass, cachedTypeInfo); + return typeInfo.serializer == null ? ensureSerializerForTypeInfo(typeInfo) : typeInfo; } break; case Types.LIST: @@ -906,6 +908,9 @@ public final TypeInfo readSharedClassMeta(ReadContext readContext, Class targ private TypeInfo readSharedClassMeta( ReadContext readContext, Class targetClass, TypeInfo cachedTypeInfo) { TypeInfo typeInfo = readSharedClassTypeInfo(readContext, targetClass, cachedTypeInfo); + // A hash identifies the wire schema, not the requested target class. Keep source hints + // unchanged so alternating targets reuse their own entries in transformedTypeInfo. + typeInfoCache[readContext.getDepth()] = typeInfo; Class readClass = typeInfo.getType(); if (targetClass != readClass) { return getTargetTypeInfo(typeInfo, targetClass); @@ -938,21 +943,7 @@ private TypeInfo readSharedClassTypeInfo( long header = buffer.readInt64(); long headerHash = TypeDef.headerHash(header); typeInfo = null; - if (targetClass != null) { - TypeDef localTypeDef = matchingLocalTypeDef(headerHash, targetClass); - if (localTypeDef != null) { - // An expected local schema owns this header before transformed or remote hints. A - // transformed hint can carry the same hash while retaining a remote TypeDef owner. - if (cachedTypeInfo != null - && cachedTypeInfo.getType() == targetClass - && cachedTypeInfo.getTypeDef() == localTypeDef) { - typeInfo = cachedTypeInfo; - } else { - typeInfo = getOrCreateLocalTypeInfo(localTypeDef, targetClass); - } - } - } - if (typeInfo == null && cachedTypeInfo != null) { + if (cachedTypeInfo != null) { TypeDef cachedTypeDef = cachedTypeInfo.getTypeDef(); // The 52-bit hash is the schema identity. Low header bits describe only this frame and // must not reopen validation of a concrete TypeInfo already bound by a checked miss. @@ -981,6 +972,15 @@ private TypeInfo readSharedTypeDefInfo( TypeDef.skipTypeDef(buffer, header); return buildCachedMetaSharedTypeInfo(typeDef); } + // Local schema resolution belongs to a cache miss. Repeating it before the cache lookups + // sends every scoped read through the shared TypeDef maps, even after all types are known. + if (targetClass != null) { + TypeDef localTypeDef = matchingLocalTypeDef(headerHash, targetClass); + if (localTypeDef != null) { + TypeDef.skipTypeDef(buffer, header); + return getOrCreateLocalTypeInfo(localTypeDef, targetClass); + } + } typeDef = TypeDef.readTypeDef(this, buffer, header); // The target check is needed only for a newly parsed TypeDef, before it can be // cached or counted. Cache hits were already accepted; the caller applies target @@ -1040,9 +1040,14 @@ private TypeInfo transformTypeInfo( TypeInfo typeInfo, Class targetClass, long typeDefHeaderHash) { Class readClass = typeInfo.getType(); TypeInfo newTypeInfo; + // Select a local schema only once for this source/target cache entry. Target-specific + // TypeInfo must not replace the source owner in the hash-only metadata cache. + TypeDef localTypeDef = matchingLocalTypeDef(typeDefHeaderHash, targetClass); // Keep assignable target matches cached here. Calling Class.isAssignableFrom for every // collection element is a hot-path regression for wildcard/object element targets. - if (targetClass.isAssignableFrom(readClass)) { + if (localTypeDef != null) { + newTypeInfo = createMetaSharedTypeInfo(localTypeDef, targetClass); + } else if (targetClass.isAssignableFrom(readClass)) { newTypeInfo = typeInfo; } else { TypeDef typeDef = typeInfo.getTypeDef(); @@ -1160,8 +1165,8 @@ private TypeInfo cacheMetaSharedTypeInfo(TypeDef typeDef, Class cls) { private TypeInfo getOrCreateLocalTypeInfo(TypeDef localTypeDef, Class cls) { long headerHash = TypeDef.headerHash(localTypeDef.getId()); TypeInfo typeInfo = extRegistry.typeInfoByHeaderHash.get(headerHash); - // A target-local match must replace a remote hint, but once the exact local owner is cached it - // must be reused. Recreating it resubmits compatible codec generation for every scoped read. + // Reuse the source schema owner across scoped reads. Target adaptations are cached separately + // in transformedTypeInfo and must not replace this hash-only entry. if (typeInfo != null && typeInfo.getType() == cls && typeInfo.getTypeDef() == localTypeDef) { return typeInfo; } diff --git a/java/fory-core/src/test/java/org/apache/fory/resolver/TypeDefHeaderHashTest.java b/java/fory-core/src/test/java/org/apache/fory/resolver/TypeDefHeaderHashTest.java index 03f2ae5be3..fd0dba823e 100644 --- a/java/fory-core/src/test/java/org/apache/fory/resolver/TypeDefHeaderHashTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/resolver/TypeDefHeaderHashTest.java @@ -24,7 +24,7 @@ import static org.testng.Assert.assertNotSame; import static org.testng.Assert.assertSame; -import java.lang.reflect.Method; +import java.lang.reflect.Field; import java.util.concurrent.atomic.AtomicInteger; import org.apache.fory.Fory; import org.apache.fory.TestUtils; @@ -34,6 +34,7 @@ import org.apache.fory.meta.TypeDef; import org.apache.fory.serializer.UnknownClass; import org.testng.Assert; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; public class TypeDefHeaderHashTest { @@ -49,6 +50,15 @@ public static class OtherHeaderHashType { public long value; } + public static class FirstTarget { + public int value; + } + + public static class SecondTarget { + public int value; + public int extra; + } + @Test public void testLocalHashHit() { Fory writer = newFory(null); @@ -70,8 +80,12 @@ public void testLocalHashHit() { assertSame(typeInfo.getTypeDef(), localTypeDef); assertEquals(frame.readerIndex(), frame.size()); - assertSame(resolver.extRegistry.typeInfoByHeaderHash.get(headerHash), typeInfo); + assertSame(resolver.extRegistry.typeInfoByHeaderHash.get(headerHash), remoteOwner); assertSame(sharedRegistry.remoteTypeDefByHeaderHash.get(headerHash), wireTypeDef); + assertSame( + resolver.readSharedClassMeta( + prepare(reader, opaqueMetaFrame(wireTypeDef, 5, 5)), HeaderHashType.class), + typeInfo); } @Test @@ -129,7 +143,7 @@ public void testLocalMissStaysLocal() { } @Test - public void testTargetLocalBeatsHint() throws Exception { + public void testTargetLocalBeatsHint() { Fory writer = newFory(null); writer.register(HeaderHashType.class, TYPE_NAME); TypeResolver writerResolver = writer.getTypeResolver(); @@ -140,24 +154,139 @@ public void testTargetLocalBeatsHint() throws Exception { reader.register(HeaderHashType.class, TYPE_NAME); TypeResolver resolver = reader.getTypeResolver(); TypeDef localTypeDef = resolver.getTypeDef(HeaderHashType.class, true); - Method getTargetTypeInfo = - TypeResolver.class.getDeclaredMethod("getTargetTypeInfo", TypeInfo.class, Class.class); - getTargetTypeInfo.setAccessible(true); - TypeInfo transformedHint = - (TypeInfo) - getTargetTypeInfo.invoke( - resolver, new TypeInfo(Object.class, localTypeDef), HeaderHashType.class); + TypeInfo sourceHint = new TypeInfo(Object.class, wireTypeDef); TypeInfo[] typeInfoCache = TestUtils.getFieldValue(resolver, "typeInfoCache"); - typeInfoCache[0] = transformedHint; + typeInfoCache[0] = sourceHint; MemoryBuffer frame = opaqueTypeFrame(typeId, wireTypeDef, 6, 6); TypeInfo typeInfo = resolver.readTypeInfo(prepare(reader, frame), HeaderHashType.class); - assertNotSame(typeInfo, transformedHint); + assertNotSame(typeInfo, sourceHint); assertSame(typeInfo.getTypeDef(), localTypeDef); + assertSame(typeInfoCache[0], sourceHint); assertEquals(frame.readerIndex(), frame.size()); } + @DataProvider + public Object[][] modes() { + return new Object[][] {{false, false}, {false, true}, {true, false}, {true, true}}; + } + + @Test(dataProvider = "modes") + public void testTargetTypeInfoReuse(boolean xlang, boolean codegen) throws Exception { + Fory writer = compatibleFory(xlang, codegen); + writer.register(HeaderHashType.class, 201); + HeaderHashType value = new HeaderHashType(); + value.value = 42; + byte[] bytes = writer.serialize(value); + TypeDef typeDef = writer.getTypeResolver().getTypeDef(HeaderHashType.class, true); + int typeId = writer.getTypeResolver().getTypeInfo(HeaderHashType.class).getTypeId(); + + Fory reader = compatibleFory(xlang, codegen); + reader.register(HeaderHashType.class, 201); + reader.register(FirstTarget.class, 202); + reader.register(SecondTarget.class, 203); + TypeResolver resolver = reader.getTypeResolver(); + TypeInfo first = + resolver.readTypeInfo(prepare(reader, typeFrame(typeId, typeDef)), FirstTarget.class); + TypeInfo source = + resolver.extRegistry.typeInfoByHeaderHash.get(TypeDef.headerHash(typeDef.getId())); + assertSame(source.getType(), HeaderHashType.class); + Field cacheField = TypeResolver.class.getDeclaredField("typeInfoCache"); + cacheField.setAccessible(true); + TypeInfo[] hints = (TypeInfo[]) cacheField.get(resolver); + assertSame(hints[0], source); + TypeInfo second = + resolver.readTypeInfo(prepare(reader, typeFrame(typeId, typeDef)), SecondTarget.class); + assertNotSame(first, second); + assertSame(first.getType(), FirstTarget.class); + assertSame(second.getType(), SecondTarget.class); + MemoryBuffer references = MemoryBuffer.newHeapBuffer(typeDef.getEncoded().length + 16); + references.writeUInt8(typeId); + references.writeVarUInt32(0); + references.writeBytes(typeDef.getEncoded()); + references.writeUInt8(typeId); + references.writeVarUInt32(1); + references.writeUInt8(typeId); + references.writeVarUInt32(1); + ReadContext context = prepare(reader, readable(references)); + assertSame(resolver.readTypeInfo(context, FirstTarget.class), first); + assertSame(resolver.readTypeInfo(context, SecondTarget.class), second); + assertSame(resolver.readTypeInfo(context), source); + for (int i = 0; i < 3; i++) { + assertSame( + resolver.readTypeInfo(prepare(reader, typeFrame(typeId, typeDef)), FirstTarget.class), + first); + assertSame( + resolver.readTypeInfo(prepare(reader, typeFrame(typeId, typeDef)), SecondTarget.class), + second); + assertSame(hints[0], source); + assertSame(reader.getReadContext().getMetaReadContext().readTypeInfos.get(0), source); + assertSame( + resolver.extRegistry.typeInfoByHeaderHash.get(TypeDef.headerHash(typeDef.getId())), + source); + assertEquals(reader.deserialize(bytes, FirstTarget.class).value, 42); + SecondTarget converted = reader.deserialize(bytes, SecondTarget.class); + assertEquals(converted.value, 42); + assertEquals(converted.extra, 0); + assertEquals(((HeaderHashType) reader.deserialize(bytes)).value, 42); + } + } + + @Test + public void testCachedLocalMetadata() { + Fory reader = compatibleFory(false, false); + TypeResolver template = reader.getTypeResolver(); + AtomicInteger localQueries = new AtomicInteger(); + ClassResolver resolver = + new ClassResolver( + template.config, + template.extRegistry.classLoader, + template.sharedRegistry, + template.jitContext) { + @Override + public TypeInfo getTypeInfo(Class cls, boolean createIfAbsent) { + if (!createIfAbsent) { + localQueries.incrementAndGet(); + } + return super.getTypeInfo(cls, createIfAbsent); + } + }; + resolver.initialize(); + resolver.register(HeaderHashType.class, 201); + resolver.register(FirstTarget.class, 202); + TypeDef typeDef = resolver.getTypeDef(HeaderHashType.class, true); + TypeInfo source = + resolver.readSharedClassMeta( + prepare(reader, opaqueMetaFrame(typeDef, 5, 5)), HeaderHashType.class); + TypeInfo target = + resolver.readSharedClassMeta( + prepare(reader, opaqueMetaFrame(typeDef, 5, 5)), FirstTarget.class); + int initialQueries = localQueries.get(); + Assert.assertTrue(initialQueries > 0); + for (int i = 0; i < 3; i++) { + assertSame( + resolver.readSharedClassMeta( + prepare(reader, opaqueMetaFrame(typeDef, 5, 5)), HeaderHashType.class), + source); + assertSame( + resolver.readSharedClassMeta( + prepare(reader, opaqueMetaFrame(typeDef, 5, 5)), FirstTarget.class), + target); + } + assertEquals(localQueries.get(), initialQueries); + } + + private static Fory compatibleFory(boolean xlang, boolean codegen) { + return Fory.builder() + .withXlang(xlang) + .withCodegen(codegen) + .withCompatible(true) + .withScopedMetaShare(true) + .withAsyncCompilation(false) + .build(); + } + @Test public void testUnregisteredTarget() { Fory writer = newFory(null); @@ -248,7 +377,12 @@ private static TypeInfo readTypeInfo(Fory fory, MemoryBuffer buffer) { private static ReadContext prepare(Fory fory, MemoryBuffer buffer) { ReadContext readContext = fory.getReadContext(); - readContext.setMetaReadContext(new MetaReadContext()); + MetaReadContext metaReadContext = readContext.getMetaReadContext(); + if (metaReadContext == null) { + readContext.setMetaReadContext(new MetaReadContext()); + } else { + metaReadContext.readTypeInfos.clear(); + } readContext.prepare(buffer, null, false); return readContext; }