bank-ro opened a new issue, #4020: URL: https://github.com/apache/fory/issues/4020
### Search before asking - [x] I had searched in the [issues](https://github.com/apache/fory/issues) and found no similar issues. ### Version 1.7.1 ### Component(s) Java ### Minimal reproduce step ## Summary In `CompatibleMode.COMPATIBLE`, deserialization on 1.7.1 is about **2x slower than 1.6.1** for graphs of small registered structs. Profiling shows ~59% of CPU is spent under `TypeResolver.matchingLocalTypeDef(...)`, which is new in 1.7.x and runs on **every `deserialize()` call, for every type in the graph**, doing 3-4 map lookups whose result is constant for a given `(headerHash, targetClass)`. This is much smaller than the 1.7.0 regression fixed by #4000 — correctness is fine and large payloads are unaffected — but the per-call cost looks unintended and memoizable rather than inherent to the feature. ## Environment - fory-core **1.6.1** (baseline) vs **1.7.1** - JDK 26, Windows 11 - `Language.JAVA`, `CompatibleMode.COMPATIBLE`, `requireClassRegistration(true)`, `withRefTracking(false)`, `withAsyncCompilation(false)` - Same `Fory` instance reused for every call; same compiled classes, only the jar differs ## Measurements Container holding 8 distinct registered struct types (322-byte payload), deserialize throughput, 5 interleaved rounds: | version | ops/s | us/op | |---|---|---| | 1.6.1 | 5,556,362 - 5,927,393 | ~0.17 | | 1.7.1 | 2,934,122 - 3,056,515 | ~0.34 | Ranges do not overlap. Scaling with the number of distinct types in the graph (us/op): | distinct types | 1.6.1 | 1.7.1 | |---|---|---| | 1 | 0.08 | 0.11 | | 4 | 0.14 | 0.23 | | 8 | 0.23 | 0.47 | | 20 | 0.68 | 0.93 | | 40 | 1.34 | 1.73 | `SCHEMA_CONSISTENT` is unaffected (~0.13 us/op on both), consistent with this being specific to the meta-shared read path. ## Profile (JFR, `settings=profile`, same workload) Samples whose stack contains `matchingLocalTypeDef`: **202 / 345 (~59%)** on 1.7.1, **0 / 327** on 1.6.1 (the method does not exist there). Top self frames: **1.6.1** — actual decoding dominates: ``` 113 org.apache.fory.memory.MemoryBuffer.readVarUint36Small() 95 org.apache.fory.collection.LongMap.get(long) 12 java.util.HashMap.getNode(Object) ``` **1.7.1** — map lookups dominate: ``` 71 java.util.concurrent.ConcurrentHashMap.computeIfAbsent(Object, Function) 47 java.util.HashMap.getNode(Object) 45 org.apache.fory.collection.IdentityMap.get(Object) 44 org.apache.fory.memory.MemoryBuffer.readVarUint36Small() 34 org.apache.fory.collection.LongMap.get(long) 28 org.apache.fory.collection.ConcurrentIdentityMap.computeIfAbsent(Object, Function) ``` Call chain of those lookups: `readTypeInfo -> readSharedClassTypeInfo -> readSharedClassMeta -> matchingLocalTypeDef -> getTypeDef / getTypeInfo` ## Cause `readSharedClassTypeInfo` calls the probe before the existing cheap caches: ```java long header = buffer.readInt64(); long headerHash = TypeDef.headerHash(header); typeInfo = null; if (targetClass != null) { TypeDef localTypeDef = matchingLocalTypeDef(headerHash, targetClass); // <-- every call, every type ... } ... if (typeInfo == null) { typeInfo = extRegistry.typeInfoByHeaderHash.get(headerHash); // cheap cache, consulted later } ``` and the probe itself is not cheap: ```java private TypeDef matchingLocalTypeDef(long headerHash, Class<?> cls) { ... if (getTypeInfo(cls, false) == null) { // map lookup return null; } TypeDef localTypeDef = getTypeDef(cls, true); // computeIfAbsent + cacheTypeDef -> another map op return TypeDef.headerHash(localTypeDef.getId()) == headerHash ? localTypeDef : null; } ``` with ```java public final TypeDef getTypeDef(Class<?> cls, boolean resolveParent) { if (resolveParent) { return cacheTypeDef(typeDefMap.computeIfAbsent(cls, k -> TypeDef.buildTypeDef(this, cls))); } ... } ``` For comparison, 1.6.1's equivalent path was a field compare plus **one** primitive-keyed lookup, and never called `getTypeDef` while reading: ```java long id = buffer.readInt64(); TypeDef cachedTypeDef = cachedTypeInfo == null ? null : cachedTypeInfo.getTypeDef(); if (cachedTypeDef != null && cachedTypeDef.getId() == id) { typeInfo = cachedTypeInfo; } else { typeInfo = extRegistry.typeInfoByTypeDefId.get(id); // LongMap.get } ``` `getTypeDef` is byte-identical between 1.6.1 and 1.7.1 — the difference is purely that 1.7.x now calls it on the read hot path. ## Suggested fixes 1. **Memoize the probe.** `matchingLocalTypeDef(headerHash, targetClass)` is a pure function of its arguments once registration is finished, so its result (including the negative result) can be cached — e.g. keyed by `(headerHash, targetClass)`, or by caching per class the pair `(localTypeDefHeaderHash, localTypeDef)` so the steady-state probe becomes one field read and a compare. This preserves the "an expected local schema owns this header" semantics the current ordering is there to guarantee, while restoring 1.6.1's single-lookup steady state. 2. **Independently: add a `get()` fast path before `computeIfAbsent` in `getTypeDef`.** `ConcurrentHashMap.computeIfAbsent` is materially more expensive than `get` on a hit (and may block), and this is the single largest self-frame in the profile. `SharedRegistry.getOrCreateTypeDef` already uses `get` then `putIfAbsent`; `getTypeDef` does not. Happy to test a patch against the repro below. ## Reproduction The same standalone class as the 1.7.0 report; build once and run under each jar (it prints the loaded version). Add `-Dfory.mode=SCHEMA_CONSISTENT` to confirm that mode is unaffected. ```java import org.apache.fory.Fory; import org.apache.fory.ThreadSafeFory; import org.apache.fory.config.CompatibleMode; import org.apache.fory.config.Language; public class ForyCompatibleRegression { public static class A { public long a; public int b; public String c; public A() {} } public static class B { public long a; public int b; public String c; public B() {} } public static class C { public long a; public int b; public String c; public C() {} } public static class D { public long a; public int b; public String c; public D() {} } public static class E { public long a; public int b; public String c; public E() {} } public static class F { public long a; public int b; public String c; public F() {} } public static class G { public long a; public int b; public String c; public G() {} } public static class H { public long a; public int b; public String c; public H() {} } public static class Container { public A a; public B b; public C c; public D d; public E e; public F f; public G g; public H h; public Container() {} } public static void main(String[] args) { CompatibleMode mode = CompatibleMode.valueOf(System.getProperty("fory.mode", "COMPATIBLE")); ThreadSafeFory fory = Fory.builder() .withLanguage(Language.JAVA) .withCompatibleMode(mode) .withRefTracking(false) .requireClassRegistration(true) .withAsyncCompilation(false) .buildThreadSafeFory(); fory.register(A.class, 201); fory.register(B.class, 202); fory.register(C.class, 203); fory.register(D.class, 204); fory.register(E.class, 205); fory.register(F.class, 206); fory.register(G.class, 207); fory.register(H.class, 208); fory.register(Container.class, 200); Container obj = new Container(); obj.a = new A(); obj.b = new B(); obj.c = new C(); obj.d = new D(); obj.e = new E(); obj.f = new F(); obj.g = new G(); obj.h = new H(); obj.a.c = "x"; byte[] payload = fory.serialize(obj); long warmEnd = System.nanoTime() + 2_000_000_000L; while (System.nanoTime() < warmEnd) sink += id(fory.deserialize(payload, Container.class)); long ops = 0, start = System.nanoTime(), end = start + 3_000_000_000L; while (System.nanoTime() < end) { sink += id(fory.deserialize(payload, Container.class)); ops++; } double opsPerSec = ops / ((System.nanoTime() - start) / 1e9); System.out.printf("fory-core %s mode=%s %d bytes%n", version(), mode, payload.length); System.out.printf(" deserialize: %,.0f ops/s (%.2f us/op)%n", opsPerSec, 1e6 / opsPerSec); System.exit(0); } private static long sink; private static int id(Object o) { return o == null ? 0 : System.identityHashCode(o); } private static String version() { Package p = Fory.class.getPackage(); String v = p == null ? null : p.getImplementationVersion(); return v == null ? "<unknown>" : v; } } ``` ### What did you expect to see? a ### What did you see instead? a ### Anything Else? a ### Are you willing to submit a PR? - [ ] I'm willing to submit a PR! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
