Skip to content

perf(rpc): per-source-file eviction to bound polyglot RewriteRpc server memory - #8297

Merged
jkschneider merged 2 commits into
mainfrom
rpc-reset
Jul 20, 2026
Merged

jkschneider merged 2 commits into
mainfrom
rpc-reset

Conversation

@jkschneider

Copy link
Copy Markdown
Member

Summary

Bounds the memory of the polyglot RewriteRpc servers (Java/JS/Python/Go/C#), whose object + ref caches previously grew insert-only for the whole process lifetime, and adds the Phase-0 profiling instrumentation to measure it. Two commits:

  1. perf(rpc): evict each source file from RPC caches after the recipe stack visits it — the eviction primitive.
  2. perf(rpc): emit per-call metrics CSV for the Python and C# RPC servers — profiling parity across all four servers.

Problem

Every RewriteRpc peer keyed visited SourceFiles into localObjects/remoteObjects + ref maps and never self-evicted; only an explicit Reset cleared them. Within a repo, every file stayed resident across every recipe cycle, so peak memory scaled with file count and was only reclaimed when the subprocess was killed per repo.

Fix — per-source-file Evict

RecipeRunCycle iterates source-outer / recipe-inner (editSource runs the whole recipe stack over one file before the next), so the end of editSource (and the scan lambda) is a clean per-file boundary. A new Evict{id} primitive drops that file's tree and rolls each touched peer's ref maps back to a pre-file checkpoint:

  • Fire-and-forget JSON-RPC notification (NON_NULL serialization omits the null id, so it is a true notification everywhere); handlers are total, and the Go/Python read loops skip replying to a null-id request so an errored notification can't fail in-flight requests.
  • Ref rollback is symmetric/lockstep: dropping the send-side ref forces the next file to re-ADD the interned JavaType (shared across files), so the receiver never sees a dangling REF_USE. Send-side rollback = protocol correctness; receive-side = the memory bound.
  • Handlers in all five peers (Java, JS onNotification, Python, Go via ReferenceMap.RollbackTo, C#). Recipe/accumulator/execution-context state is keyed separately and preserved, so a scanning recipe's accumulator survives across files within a cycle.

Eviction is unconditional (no config flag).

Profiling (Phase 0)

  • Per-call metrics CSVs now carry local_objects,remote_objects,refs residency columns for all four language servers (Go/JS in commit 1; Python + C# in commit 2), so a run shows the cache-size ramp (evict off) vs sawtooth (evict on). Python's --metrics-csv was previously a no-op stub; C# had no metrics CSV at all. C# records via a message-handler wrapper since StreamJsonRpc dispatches by attribute and has no central loop.
  • RewriteRpcProcess gains an opt-in subprocess RSS sampler (REWRITE_RPC_RSS_MS -> rpc-rss-<pid>.csv) that uniformly covers all four languages, including native/off-heap growth the in-process counters miss.

Measured

Same-binary A/B on the Python server (per-repo peak subprocess RSS, evict off -> on): flask 133->94 MB (-30%), poetry 285->86 MB (-70%), scrapy 374->68 MB (-82%). The "before" curve climbs monotonically with file count; "after" is bounded and does not scale with repo size. Cost: ~30% longer wall-clock from the per-file evict notifications.

Testing

Full RewriteRpcTest (19 tests; loopback recipe runs exercise the hook + client rollback + inbound Evict) passes, including a new evictDropsTreeFromBothPeers. The metrics commit is additive and does not touch the Java loopback test path; it is verified to compile via py_compile, dotnet build, and :rewrite-csharp:compileJava.

Follow-up (separate repo)

The Moderne CLI already wires each server's metricsCsv; C# (csharp-rpc.csv) needs a one-line CLI change to pass the path, which lands after this releases.

…ack visits it

Polyglot RewriteRpc peers (Java/JS/Python/Go/C#) kept every visited SourceFile in
their localObjects/remoteObjects + ref caches for the process lifetime; only an
explicit Reset cleared them, so within a repo every file stayed resident across
every recipe cycle.

Because RecipeRunCycle iterates source-outer / recipe-inner (editSource applies the
whole recipe stack to one file before the next), the end of editSource (and the scan
lambda) is a clean per-file boundary. Add a targeted Evict primitive:

- New Evict{id} request + RewriteRpc.evict / inbound handler / refCheckpoint, driven
  from a RecipeRunCycle hook that, after the stack finishes a file, drops that file's
  tree and rolls each touched peer's ref maps back to the pre-file checkpoint.
- Evict is a fire-and-forget JSON-RPC notification (NON_NULL serialization omits the
  null id => a true notification everywhere); handlers are total, and the Go/Python
  read loops skip replying to a null-id request so an errored notification can never
  fail in-flight requests.
- Ref rollback is symmetric/lockstep: dropping the send-side ref forces the next file
  to re-ADD (interned JavaType is shared across files), so the rolled-back receiver
  never sees a dangling REF_USE. Send-side rollback = protocol correctness; receive-
  side = the memory bound.
- Handlers added to all five peers (Java, JS onNotification, Python, Go with
  ReferenceMap.RollbackTo, C#), preserving recipe/accumulator/execution-context state.

Recipe/accumulator/prepared-recipe state is keyed separately and left intact, so a
scanning recipe's accumulator survives across files within a cycle.

Profiling (Phase 0): Go and JS metrics CSVs gain local_objects/remote_objects/refs
residency columns, and RewriteRpcProcess gains an opt-in subprocess RSS sampler
(REWRITE_RPC_RSS_MS -> rpc-rss-<pid>.csv) that uniformly covers all four languages,
including native/off-heap growth the in-process counters miss.

Full RewriteRpcTest (19 tests, loopback recipe runs exercise the hook + rollback +
inbound Evict) passes, including a new evictDropsTreeFromBothPeers.
Phase-0 profiling parity. Go and JS already write local_objects/remote_objects/refs
residency columns per RPC call, so a run shows cache-size ramp (no evict) vs sawtooth
(per-file Evict). Python's --metrics-csv was a no-op stub and C# had no metrics CSV at
all. Both now emit the Go 9-column schema: timestamp, method, duration_ms, error,
memory_used_bytes, memory_max_bytes, local_objects, remote_objects, refs.

- Python server.py: metrics writer + a per-call row from the dispatch loop. refs counts
  remote_refs only; Python's send-side refs live on a per-call RpcSendQueue, so
  remote_refs is the only cross-call ref cache handle_evict rolls back.
- C# has no central dispatch loop (StreamJsonRpc [JsonRpcMethod] dispatch), so a
  MetricsMessageHandler wraps the IJsonRpcMessageHandler and records a row when each
  inbound request's response is written back: post-dispatch, so counts reflect the
  handler's mutations. Notifications (Evict) expect no response and are not recorded;
  the drop shows on the next recorded call. Adds RpcMetricsWriter,
  RewriteRpcServer.RunAsync(metricsCsv:), Program.cs parsing of --metrics-csv=, and
  CSharpRewriteRpc.Builder.metricsCsv(Path) threaded into all three command builders.

Memory columns are RSS best-effort; the RewriteRpcProcess RSS sampler remains the
source of truth for process memory. The object-count columns are the point.

Verified to compile: py_compile, dotnet build, :rewrite-csharp:compileJava.
@jkschneider
jkschneider merged commit b3008cc into main Jul 20, 2026
1 check passed
@jkschneider
jkschneider deleted the rpc-reset branch July 20, 2026 20:03
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Jul 20, 2026
jkschneider added a commit that referenced this pull request Sep 27, 2026
`RewriteRpc.evict` dropped a source file's tree from both peers and *also*
rolled each side's ref maps back to a checkpoint taken before that file. Ref
ids are assigned sequentially and every file's checkpoint is effectively 0, so
nothing interned survived from one source file to the next: each file re-sent
the full transitive closure of everything it referenced.

Measured over 25 consecutive spring-boot files from one source set, evicting
refs costs 3,716,349,765 bytes against 324,869,841 with refs retained — 11.4x.
The ratio grows with file count; by file 25 the marginal cost is 149 MB versus
216 KB.

Tree eviction stays. `Evict`'s payload does not change, so there is no
wire-format change and the five language servers are independently landable.

Retention is cheap on the receiver: peak today is one file, almost entirely a
`JavaSourceSet` it reconstructs and discards once per file. Keeping refs it
holds one copy. The interned type set grows about 2x (132,439 types across 25
files against 64,573 for the largest single file), roughly +3 MB against a
150 MB peak.

Three rollbacks are deliberately left in place. They undo the refs a single
*failed transfer* allocated, which is orthogonal to eviction:

- `rewrite-core/.../rpc/request/GetObject.java` (`savedRefCount` / `removeIf`)
- `rewrite-javascript/rewrite/src/rpc/request/get-object.ts`
  (`snapshot`/`rollbackTo`), which is why `ReferenceMap.snapshot`/`rollbackTo`
  stay in `reference.ts`
- `rewrite-go/cmd/rpc/main.go` `SendQueue.DiscardNewReferences`

Python had two layers that had to go together — the child's receive-side
`_ref_checkpoints` and the facade's send-side `_hub_send_checkpoint`. Removing
either alone produces `Received reference to unknown object` in facade mode
only, which the leaf-server tests do not reproduce. The monotonic
`_hub_send_next` counter is kept: it is what makes cross-file reuse work.

`RewriteRpc.evict(String, int, int)` becomes `evict(String)`. Its only callers
are `RecipeRunCycle` and `RewriteRpcTest`, and it shipped only in #8297, so
there is no deprecated shim.

The three tests that asserted lockstep rollback now assert the opposite
invariant rather than being deleted, so they still distinguish the new behavior
from the old.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant