Python: a lone recipe bundle runs in the facade's own process - #8745
Open
knutwannheden wants to merge 2 commits into
Open
knutwannheden wants to merge 2 commits into
knutwannheden wants to merge 2 commits into
Conversation
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).
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.
A
mod runof 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) withorg.openrewrite.python.migrate.UpgradeToPython313, three interleaved pairs:Java→facade is untouched by this change, and its spread is
Printre-fetches — the one flow the baseline document records as non-deterministic. What goes is the hop after it: the facade re-serializing_hub_treethroughRpcSendQueue(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
rewrite8.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 fromHEAD^:That home starts with no bundle venv, so the CLI creates one and
built_by_running_interpreterjudges 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 runexecutes 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._discoveris the one place that decides. A bundle installed while no other exists is activated in this process:sys.pathviasite.addsitedir, which appends, so the engine keeps the precedence_child_envgives it in a childdiscover_root_recipesscopes discovery to that distribution, as--child-bundledoes for a childhandle_requestfollows that decision.InstallRecipesandGetMarketplacealways go to the facade;SetDataTableStoreandEvictgo to the facade and the local handler; everything else only whenFacade.routes_to_children().With one bundle that predicate is false, so
PrepareRecipe,Visit,BatchVisit,Generate,PrintandGetObjectreach the plain handlers — the same code a child runs — and no_hub_*state is touched.What bounds the hosting
is_usable_venvchecks only thatpyvenv.cfg'shomestill exists, never the version, and a 3.11site-packagesappended to a 3.12 path breaks any compiled dependency.built_by_running_interpreterreads the recorded version._importedis sticky where_hostedis not: a bundle installed after the hosted one is uninstalled starts behind a child.activateis injected, not importedbundle_childrenfirst reached the hook withfrom rewrite.rpc.server import activate_bundle_in_process. The server runs aspython -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.activateis therefore a required keyword: missing wiring is aTypeError, not a silent fall back to the two-process path this change removes.java_rpc_client.py:137has the same shape and is untouched.Tests
Seven, each pinning one line:
_importedoutliving_hostedacross an uninstallroutes_to_childrenguard, asserting_hub_treestays emptyreturnthat gets a hosted bundle its data-table storeCrossBundleBatchVisitIntegTestcovers the two-bundle path the unit tests fake: two bundles behind two children, one BatchVisit spanning both, renames chained (alpha→beta,beta→gamma) sogammaproves the first bundle's edit reached the second's input. It drivesbatchVisitfrom the JVM because the CLI cannot reach this case — it installs only the bundles a recipe needs, and a declarativerecipeListresolves from the JVM classpath so it cannot name pip recipes. It has not been executed: theintegTestsource set does not resolve on my machine (junit-platform-suite-apiSNAPSHOT, 401), so CI is the first thing to run it.Scope
The
_hub_*block,child_connectionand the facade's routing are now dead code on the path amod runtakes. 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
PipRecipeBundleResolverdoes by running during marketplace resolution. If that stopped holding, aVisitfails withNo child owns visitor— loudly.