Skip to content

Python: a lone recipe bundle runs in the facade's own process - #8745

Open
knutwannheden wants to merge 2 commits into
mainfrom
single-bundle-runs-shouldn-t-spawn-a-child
Open

knutwannheden wants to merge 2 commits into
mainfrom
single-bundle-runs-shouldn-t-spawn-a-child

Conversation

@knutwannheden

@knutwannheden knutwannheden commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

A mod run of a pip recipe bundle uses two Python processes and sends the whole LST between them. The facade deserializes the tree from Java and re-serializes it to a child that runs the recipes — a hop that produces no edits.

Measured on jd/tenacity (20 files, 5,896 LOC) with org.openrewrite.python.migrate.UpgradeToPython313, three interleaved pairs:

            wall   fix.patch   py CPU   procs   Java→facade   facade→child   child parses
main-1       94s   c0a2a2ab     66.0s     2       2,729,604      1,303,024      1,302,939
main-2      120s   c0a2a2ab     77.0s     2       2,830,075      1,303,024      1,302,939
main-3       86s   c0a2a2ab     61.7s     2       2,854,638      1,303,024      1,302,939
branch-1     68s   c0a2a2ab     49.8s     1       2,806,383              0              0
branch-2     51s   c0a2a2ab     37.5s     1       2,434,323              0              0
branch-3     68s   c0a2a2ab     49.6s     1       2,599,951              0              0

Java→facade is untouched by this change, and its spread is Print re-fetches — the one flow the baseline document records as non-deterministic. What goes is the hop after it: the facade re-serializing _hub_tree through RpcSendQueue (1,303,024 messages, identical in all three main arms) and the child parsing it back (1,302,939), in a second process.

c0a2a2ab… is the recorded baseline hash, so the patch is byte-identical in all six runs. Another session's RPC server competed for CPU throughout, so wall and CPU are directional; the two zeroed columns are deterministic.

On a CLI built from this branch

rewrite 8.92.0-SNAPSHOT, a fresh CLI home, and the engine installed from a real wheel rather than patched in place, against the same CLI carrying a wheel built from HEAD^:

custom-main     wall 167s   c0a2a2ab   py CPU 51.5s   2 procs (facade 27.2s + child 24.4s)
custom-branch   wall 107s   c0a2a2ab   py CPU 35.1s   1 proc

That home starts with no bundle venv, so the CLI creates one and built_by_running_interpreter judges it fresh — the case where a wrong answer would silently disable hosting and still pass the patch gate.

Why one bundle is the normal case

The facade came from #8275: recipes from different bundles shared one environment, so their dependencies could conflict. Serving each child a diff over that child's ref table means the facade holds the tree as live objects, so every tree is materialized twice.

But a mod run executes one recipe, so normally one bundle participates — the multi-bundle case is a YAML composite spanning two pip packages — and a lone bundle has nothing to be isolated from.

What changed

BundleChildren._discover is the one place that decides. A bundle installed while no other exists is activated in this process:

  • its venv joins sys.path via site.addsitedir, which appends, so the engine keeps the precedence _child_env gives it in a child
  • discover_root_recipes scopes discovery to that distribution, as --child-bundle does for a child
  • a second bundle moves the first behind a child and spawns one for the newcomer

handle_request follows that decision. InstallRecipes and GetMarketplace always go to the facade; SetDataTableStore and Evict go to the facade and the local handler; everything else only when Facade.routes_to_children().

With one bundle that predicate is false, so PrepareRecipe, Visit, BatchVisit, Generate, Print and GetObject reach the plain handlers — the same code a child runs — and no _hub_* state is touched.

What bounds the hosting

  • A venv built by another interpreter is left to a child. is_usable_venv checks only that pyvenv.cfg's home still exists, never the version, and a 3.11 site-packages appended to a 3.12 path breaks any compiled dependency. built_by_running_interpreter reads the recorded version.
  • A process takes one bundle's imports, ever. Imports are permanent, so _imported is sticky where _hosted is not: a bundle installed after the hosted one is uninstalled starts behind a child.

activate is injected, not imported

bundle_children first reached the hook with from rewrite.rpc.server import activate_bundle_in_process. The server runs as python -m rewrite.rpc.server, so it is __main__ and that binds a second module object with its own empty marketplace — activation filled that copy while the running server's stayed empty.

activate is therefore a required keyword: missing wiring is a TypeError, not a silent fall back to the two-process path this change removes. java_rpc_client.py:137 has the same shape and is untouched.

Tests

Seven, each pinning one line:

  • the hosting branch, and the eviction branch a second install takes
  • the interpreter check, and _imported outliving _hosted across an uninstall
  • the routes_to_children guard, asserting _hub_tree stays empty
  • the missing return that gets a hosted bundle its data-table store
  • the facade-scoped marketplace

CrossBundleBatchVisitIntegTest covers the two-bundle path the unit tests fake: two bundles behind two children, one BatchVisit spanning both, renames chained (alpha→beta, beta→gamma) so gamma proves the first bundle's edit reached the second's input. It drives batchVisit from the JVM because the CLI cannot reach this case — it installs only the bundles a recipe needs, and a declarative recipeList resolves from the JVM classpath so it cannot name pip recipes. It has not been executed: the integTest source set does not resolve on my machine (junit-platform-suite-api SNAPSHOT, 401), so CI is the first thing to run it.

Scope

The _hub_* block, child_connection and the facade's routing are now dead code on the path a mod run takes. They stay: they are the multi-bundle fallback, and deleting them would drop multi-bundle support rather than simplify it.

Eviction rests on bundle installs completing before any recipe is prepared, which PipRecipeBundleResolver does by running during marketplace resolution. If that stopped holding, a Visit fails with No child owns visitor — loudly.

A `mod run` normally installs one pip bundle, and the facade spawned a child
for it and relayed the whole LST there. With nothing to isolate the bundle
from, the venv is worth no second process: `BundleChildren._discover` hosts a
lone bundle here and falls back to spawn-and-route from the second bundle on.

On jd/tenacity with UpgradeToPython313 this drops the facade->child hop from
1,303,024 messages to none and leaves one Python process instead of two, with
fix.patch byte-identical over three interleaved pairs.
Two bundles installed means two child processes, and a BatchVisit spanning both
has to thread the first bundle's edit into the second's input. The renames chain
(alpha->beta, beta->gamma), so `gamma` proves the threading and `beta` would show
it broken.

The JVM drives batchVisit directly because the CLI cannot reach this: it installs
only the bundles a recipe needs, and a declarative recipeList resolves from the
JVM classpath, so it cannot name pip recipes.

Not yet executed -- the integTest source set does not resolve on the author's
machine (junit-platform-suite-api SNAPSHOT, 401).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

1 participant