jvm: use argfiles for JVM and JVM-compiler process arguments - #23620
jvm: use argfiles for JVM and JVM-compiler process arguments#23620robertpi wants to merge 4 commits into
Conversation
The Java 9+ argfile optimization moved the classpath and program
arguments into an `@__jvm_args.txt` file to avoid OS-level "Argument
list too long" errors. This broke `pants run` and deploy-jar execution
against a Java 9+ JDK: those code paths embed a literal `{chroot}`
placeholder in classpath entries (substituted by the engine only within
a process's argv at spawn time, never within file contents), and the
InteractiveProcess run in the workspace doesn't have its cwd set to the
sandbox, so the bare `@__jvm_args.txt` reference was also unresolvable.
Skip the argfile and fall back to inline argv whenever a `{chroot}`
placeholder is present in the classpath or argv, which is exactly the
set of callers (run.py, run_deploy_jar.py) relying on that
substitution. All other callers, which don't use `{chroot}`, keep using
the argfile.
Also updates a stale OpenAPI generator test that asserted on argv
content that now lives in the argfile, and adds a release note.
There was a problem hiding this comment.
Would you be able to expand on the cases where you hit this? Given where these limits generally sit it would have to be quite extreme to hit it for classpaths. Essentially on the order of thousands of classpath entries on a single target.
This PR effectively disables nailgun for JDK 9+, and nailgun is what gives warm JVMs, so that is a significant and unconditional cost on things like compile. But this isn't a problem when running nailgun in the first place, because then the potentially long arg list isn't passed via command arguments at all. So this disables nailgun, making it a universal problem, then applies the fix, and applies the file-indirection to every jvm_process whether or not it's needed. For anyone without the extreme classpath case that is strictly a regression.
The launcher args, the ones incompatible with nailgun, are also the ones that are bounded for anything nailgun-able. Under nailgun the only command line is the server spawn, carrying just the tool classpath. Everything that varies per invocation goes over the socket. So they are only unbounded for things that will not run under nailgun anyway, like tests.
The compile-side argfiles are nailgun-compatible, though also unnecessary under nailgun, since those args aren't passed via the command line there either.
So both are only needed when nailgun isn't used. The compile-side ones pay for that with a small unconditional overhead while the jvm_process one pays for it by disabling nailgun for everyone.
There is also a nailgun implementation that supports all modern JDK versions which Pants can be configured to use, so this would entirely disable the option of using that.
I would not like to see the blanket disabling of nailgun, but I can imagine other directions
- Only do this when nailgun is not used, which is not immediately known here. You would essentially need to predict if nailgun will be used, which needs to combine several pieces of state. This can easily drift.
- Doing it only if
use_nailgun=Falsewould allow for the fix when you know nailgun won't be used, but it being True only means it might. - Possibly rewriting the args at a later stage when you know how it will execute (nailgun or not) and would also know the actual argv length
| use_argfile = jdk.jre_major_version >= 9 and not contains_chroot_placeholder | ||
| use_nailgun = [] | ||
| if request.use_nailgun: | ||
| if request.use_nailgun and not use_argfile: |
There was a problem hiding this comment.
This effectively disables nailgun, where the fix is not needed since the long args are not passed on command line already.
There was a problem hiding this comment.
Fixed — see the full reply above and the new commit; gated the argfile on not request.use_nailgun instead of just the JDK version, so nailgun stays enabled here unless the caller already isn't going to use it.
Nailgun sends the classpath and program arguments to the already-running server over a socket rather than via argv, so it was never subject to the OS argument-length limit the argfile works around, and gets no benefit from routing through it. Gating the argfile on JDK version alone unconditionally disabled nailgun for every Java 9+ process, including the common case (e.g. compilation) where nailgun was requested and the argfile wasn't needed. Gate on `not request.use_nailgun` instead, so the argfile is only used when the caller already isn't going to use nailgun (as JUnit/ScalaTest test execution and `pants run`/deploy-jar already do), rather than disabling nailgun universally to make room for it. Verified with `PY=python3.14 ./pants test` on: - src/python/pants/jvm/jdk_rules_test.py - src/python/pants/jvm/run_integration_test.py - src/python/pants/backend/openapi/util_rules/generator_process_test.py - src/python/pants/backend/java/compile/javac_test.py - src/python/pants/backend/scala/compile/scalac_test.py - src/python/pants/backend/kotlin/compile/kotlinc_test.py - src/python/pants/jvm/test/junit_test.py - src/python/pants/backend/scala/test/scalatest_test.py and `PY=python3.14 ./pants fmt lint check` on all changed files.
|
Good catch — you're right this disables nailgun for every JDK 9+ process, not just the ones that need the argfile, and that nailgun already keeps the classpath off the command line entirely, so the fix bought nothing there and only cost something. Pushed a fix: the gate is now On the concrete case: it's Verified |
2 participants
Problem
JVM processes (
javac,scalac,kotlinc, and thejavabinary itself) are invoked with their full classpath and argument list passed directly on the command line viaargv. For components with many dependencies or long classpaths, this can exceed OS-level command-line length limits (e.g.ARG_MAXon Linux/macOS), causing compilation to fail with an "Argument list too long" style error.Fix
JvmProcessnow writes thejavainvocation's arguments (classpath plus JVM options and program args) to a@argfile(__jvm_args.txt) instead of passing them on the command line, per thejavacommand-line argument files support added in JDK 9. Since Java 9+ argfiles supersede the classpath-length problem nailgun was also working around, nailgun is disabled in that path in favor of the argfile.jvm_argfile_contenthelper for building a correctly quoted/escaped argfile from a list of arguments.javac,scalac, andkotlinccompiler invocations themselves (-d <dir>, classpath, source file lists, etc.), which independently suffer from the same command-line length limits regardless of JDK version.Test plan
src/python/pants/jvm/jdk_rules_test.py(new/updated coverage for argfile generation, escaping, and the Java 9+ nailgun-vs-argfile branch)./pants fmt linton all changed files