Skip to content

perf(core): cache generated JSON schema per Class/Type to cut JsonSchemaUtils lock contention - #2

Draft
java-dependency-upgrade-fixer[bot] wants to merge 1 commit into
evaluation/agentscope-pr-2796from
jaipilot/pr-1-7tDTG1hkrVzf
Draft

perf(core): cache generated JSON schema per Class/Type to cut JsonSchemaUtils lock contention#2
java-dependency-upgrade-fixer[bot] wants to merge 1 commit into
evaluation/agentscope-pr-2796from
jaipilot/pr-1-7tDTG1hkrVzf

Conversation

@java-dependency-upgrade-fixer

Copy link
Copy Markdown

What

The base PR (mirroring agentscope-ai#2796) fixed a real thread-safety bug by serializing every call to the shared victools SchemaGenerator through a single SCHEMA_LOCK, since the generator's JacksonModule keeps an unsynchronized introspection cache. That fix is correct but serializes every call, even repeat calls for the same class/type that the hot paths in ReActAgent.doStructuredCall and ToolSchemaGenerator issue on every agent call / tool registration.

This change caches the generated JsonNode per Class/Type in a ConcurrentHashMap, so SCHEMA_LOCK (and the expensive reflective schemaGenerator.generateSchema(...) call it guards) is only needed the first time a given class or type is seen. Every call still performs its own JsonUtils conversion from the cached node to a brand-new, independently mutable Map, so callers that mutate the returned map (e.g. ToolSchemaGenerator adding a description key) do not see or cause cross-call contamination.

Why it's safe

  • The generated schema is a pure, deterministic function of the (Class/Type, static generator config) pair for the process lifetime, so caching by that key cannot become stale.
  • The cached JsonNode itself is never mutated or exposed outside this class; only a fresh Map copy is ever returned, preserving the exact byte-for-byte schema content and the existing independent-mutability contract relied upon by ToolSchemaGenerator.
  • Cache misses still run under the exact same SCHEMA_LOCK, so the generator's internal introspection cache is still never touched concurrently — thread safety is unchanged.
  • Exception behavior is unchanged, including the pre-existing (if slightly awkward) NullPointerException raised for a null class/type argument, verified by new characterization tests.
  • Cache keys are the finite, compile-time-fixed set of structured-output/tool-parameter classes and types declared in application code, not user-controlled input, so the unbounded ConcurrentHashMap cannot grow without bound.

Evidence

  • Added 4 focused characterization tests locking repeated-call schema equality, returned-map mutation independence, and null-argument exception shape; ran them against the original PR-head implementation (pass) and the final candidate (pass), using the same command both times.
  • Ran the downstream production call sites (ToolSchemaGeneratorTest, ReActAgentStructuredOutputTest, ReActAgentStructuredOutputWithToolsTest) against the candidate; all pass.
  • Benchmarked 16 threads x 5000 calls (80000 total) alternating between 2 classes, 5 observations per side: median wall time dropped from 20323ms to 899ms (~22x) and p95 from 26779ms to 3836ms (~7x).
  • Full mvn -pl agentscope-core -am verify passes on the final candidate.

Limitations

  • Benefit scales with how often the same class/type recurs across calls in a given process; a workload that only ever generates each schema once would see no measurable benefit (but also no measurable regression, since the extra map lookup is negligible next to reflection-based schema generation).
  • Benchmarked on a shared sandbox machine rather than an isolated perf lab; only the relative improvement (consistent across all runs) is claimed, not absolute production numbers.

Generated by JAIPilot Cloud for #1 from Anthropic session sesn_0186Z4xnoTvr7tDTG1hkrVzf.

@skrcode

skrcode commented Aug 22, 2026

Copy link
Copy Markdown
Owner

Campaign acceptance #2 (independently reviewed)

  • Identity: companion commit 8f9e00c has the exact input head 15859a9 as its sole parent.
  • Scope: one changed production file plus its directly relevant test; no control/workflow paths.
  • Behavior proof: repeated Class/Type schemas remain equal, each returned Map remains independently mutable, null exception shape is pinned, and ToolSchemaGenerator/ReActAgent callers pass.
  • Performance proof: identical 16-thread x 5,000-call workload (80,000 calls), five observations per side; median 20,323 ms -> 899 ms and p95 26,779 ms -> 3,836 ms in the managed sandbox.
  • Final verification: mvn -pl agentscope-core -am verify passed.
  • Review boundary: both caches retain one entry per encountered Class/Type. That is acceptable for the documented application-declared schema set and is disclosed in the draft, but upstream should confirm this lifecycle assumption for environments that dynamically load unbounded schema types.
  • Independent fork CI did not attach to the generated feature-branch draft; repository-native verification above came from the managed exact-head session.

Accepted as a genuine, reviewable performance improvement; remains draft and unmerged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant