Intern JavaType instances across a parse session and keep RPC refs across files - #8518
Merged
Merged
Conversation
…ross files PythonTypeMapping allocated its JavaType caches per file, so a project parse materialized one distinct JavaType object per file for every logical type. ty type ids are stable for a session's lifetime and its descriptor table is cumulative across files, so those caches can hang off the session instead: on a 257-file corpus, 991,885 distinct JavaType objects become 123,761 and peak RSS drops from 408 MB to about 240 MB. TypedDict keys stay per file. ty emits a typedDict descriptor carrying only a name, so a session-wide key would let one module's TypedDict report another module's fields. Separately, handle_get_object built a fresh RpcSendQueue per call, restarting ref numbering on every response so each file resent the full closure of the types it referenced. A connection-scoped ReferenceMap mirrors what Java, JavaScript, C# and Go already hold, with rollback on a failed transfer and on Evict, the latter symmetric with RewriteRpc.evict dropping its remoteRefs. The two halves only pay off together: dedup keys on object identity at both ends, so the interning gives the ref map something to dedup and the ref map carries it across the wire.
JavaType instances across a parse session and keep RPC refs across files
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Two independent things made a Python project parse rebuild and resend the same types once per file.
PythonTypeMappingallocated every one of itsJavaTypecaches per file, so a project parse materialized one distinctJavaTypeobject per file for every logical type. On a 257-file corpus that is 991,885 distinctJavaTypeobjects where about 124,000 suffice — roughly 0.4% sharing, i.e. effectively none.Separately,
handle_get_objectconstructed a freshRpcSendQueueper call with an empty ref map, so ref numbering restarted on every response and each file resent the full transitive closure of the types it referenced. Java, JavaScript, C# and Go all hold a connection-scoped send-side ref map (RewriteRpc.localRefs,rewrite-rpc.tslocalRefs,RewriteRpcServer._localRefs,s.localRefs); Python was the only peer without one, even though Java's receive side already passes connection-scopedremoteRefsinto its queue.Neither change is worth much alone: dedup keys on object identity at both ends, so the interning is what gives the ref map something to dedup, and the ref map is what carries that across the wire.
Examples
TyTypesClientnow owns theJavaTypeinstances resolved during its session, beside the descriptor table they come from:RpcSendQueuetakes an optionalReferenceMap, which spans the peer connection rather than the single response:Summary
SessionTypeCache, holding the threeJavaType-bearing caches (by_type_id,declaring_by_type_id,by_fqn).TyTypesClientowns one for its session and clears it whereversession_typesis cleared;PythonTypeMappingbinds to it when a client is present and to a private instance otherwise, so standalone parses are unchanged._create_class_typean optionalcache_keyand scope theTypedDictkey per file. ty emits atypedDictdescriptor with only anameand nomoduleName, so a session-wide FQN key would let one module'sMoviereport another module's fields.ReferenceMap(rpc/reference.py), mirroring the JavaScriptReferenceMapand Gopkg/rpc/reference.go: identity-keyed map plus counter, withsnapshot/rollback_to/clear. It keeps the object beside its ref so a recycledid()cannot alias a freed object onto a live ref.local_refsin the RPC server, pass it intohandle_get_object, and roll it back when a transfer fails — the rollback Java, JavaScript and Go each keep.local_refsback onEvictagainst a per-file checkpoint captured inhandle_visit/handle_batch_visit, symmetric withRewriteRpc.evictdropping itsremoteRefsabove the matching checkpoint.ReferenceMap, retiring the parallel_hub_send_nextcounter.Measured
A 257-file corpus (
rewrite-python/rewrite's ownsrcandtests), parsed throughhandle_parse_project:JavaTypeobjectsA 10-file synthetic corpus goes from 15,821 distinct objects to 1,749, against 15,831 summed per file and 1,468 for the richest single file.
Relationship to Keep interned RPC refs alive across source files #8512
Keep interned RPC refs alive across source files #8512 removes eviction-driven ref rollback across all five language servers, keeping only the failed-transfer rollbacks. This branch is deliberately correct against
mainas it stands:RewriteRpc.evictstill dropsremoteRefsabove a checkpoint andRecipeRunCyclecalls it per source file, so retaining sent refs without the matching rollback would desynchronize. When Keep interned RPC refs alive across source files #8512 lands,_local_ref_checkpointsand itshandle_evictbranch come out in the same sweep as_ref_checkpointsand_hub_send_checkpoint;local_refsitself stays, and is what makes cross-file reuse work. Expect conflicts with it inserver.py,test_facade.pyandtest_server.py.Test plan
test_type_interning.py: corpus-wide distinctJavaTypecount stays within reach of the richest single file; a class defined in one file and used in another is one object regardless of walk order; two same-namedTypedDicts in different modules keep their own fields. All three fail without the change.test_reference_map.py: 8 unit tests over id assignment, identity, recycled-id()safety, and snapshot/rollback.test_send_ref_reuse.py: the second file cites refs the first sent and assigns no id twice;Evictreleases exactly that file's refs; a failed transfer releases the refs it assigned;Resetclears them. All four fail without the change._hub_send_nextand raw-dict representation — same behavior, new type.pytest tests/ --timeout=180)../gradlew :rewrite-python:integTestover the real RPC boundary: same two pre-existing failures asmain(ChangeTypeIntegTest.changeBuiltinType,DependencyWorkspaceIntegTest.findMethodFromExternalPackage), nothing new. Neither runs in CI today.integTestexecutes this checkout's Python rather than an installed package, so the round trip against the Java receiver is genuine.