[
https://issues.apache.org/jira/browse/GROOVY-12711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18141240#comment-18141240
]
ASF GitHub Bot commented on GROOVY-12711:
-----------------------------------------
daniellansun commented on code in PR #3242:
URL: https://github.com/apache/groovy/pull/3242#discussion_r4513076345
##########
src/main/java/org/codehaus/groovy/tools/javac/MemClassOutputClasspath.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.tools.javac;
+
+import groovy.lang.GroovyClassLoader;
+
+import java.io.ByteArrayInputStream;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLConnection;
+import java.net.URLDecoder;
+import java.net.URLStreamHandler;
+import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+import java.util.UUID;
+import java.util.WeakHashMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Publishes in-memory class bytes through a {@link GroovyClassLoader} so
+ * joint compilation can resolve them without a class-output directory.
+ * <p>
+ * The protocol is not registered with the JVM. Only URLs created here can
+ * be opened, and each URL serves just the map it was given. Paths that
+ * escape that map are rejected. No filesystem or network access is performed.
+ */
+final class MemClassOutputClasspath {
+
+ private static final String PROTOCOL = "x-groovy-mem";
+
+ private static final Map<GroovyClassLoader, Map<String, byte[]>> INSTALLED
=
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ private MemClassOutputClasspath() {
+ }
+
+ /**
+ * Attaches an in-memory classpath to {@code loader}, or returns the map
+ * already attached to that loader. A second compile on the same loader
+ * therefore replaces entries in place and stays visible to
+ * {@code getResource}.
+ *
+ * @param loader the compilation unit's class loader
+ * @return the mutable resource map, keyed by {@code
com/example/Type.class}
+ */
+ @SuppressWarnings("deprecation") // custom handler; URL.of(URI, handler)
is not on the Java 17 baseline
+ static Map<String, byte[]> install(final GroovyClassLoader loader) {
Review Comment:
I looked at the callers again instead of defending the test.
`groovyc` (`FileSystemCompiler`) does `new
JavaAwareCompilationUnit(configuration)`, which builds a new
`GroovyClassLoader` for that unit. The Ant task does the same in
`Groovyc#buildClassLoaderFor`: a new `GroovyClassLoader` on every run, then one
unit. Neither reuses a loader across units.
`GroovyClassLoader#parseClass` does reuse the outer loader, but not in the
way the map assumed. `createCompilationUnit` builds a plain `CompilationUnit`
on that loader, not a `JavaAwareCompilationUnit`. The classes are defined on a
new `InnerLoader` per `parseClass`. That inner loader delegates `getResource`
and `addURL` to the outer loader. Joint compilation never goes through this
path, so the map was not installed for it.
The one production place that builds a new `JavaAwareCompilationUnit` on an
existing loader is groovy-lsp (`GroovyCompiler#loaderFor`). It reuses the
loader while compiler settings are unchanged. It sets `memStub` and still
writes javac output to a temp directory. It does not set `memClassOutput`, and
a typical recompile overlaps the previous one.
So there is no current caller for which a loader-to-store cache is the thing
that makes `memClassOutput` work. The test
`testSecondCompileOnTheSameLoaderSeesTheNewJavaApi` was the situation. I
removed the cache and that test. A recompile of the same classes in this PR is
a new `GroovyClassLoader`, which is what those entry points already do. The
same unit can still `compile` again, including after
`compile(Phases.SEMANTIC_ANALYSIS)` and after `addSources`, because the store
is held by the unit.
##########
src/main/java/org/codehaus/groovy/tools/javac/MemClassOutputClasspath.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.tools.javac;
+
+import groovy.lang.GroovyClassLoader;
+
+import java.io.ByteArrayInputStream;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLConnection;
+import java.net.URLDecoder;
+import java.net.URLStreamHandler;
+import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+import java.util.UUID;
+import java.util.WeakHashMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Publishes in-memory class bytes through a {@link GroovyClassLoader} so
+ * joint compilation can resolve them without a class-output directory.
+ * <p>
+ * The protocol is not registered with the JVM. Only URLs created here can
+ * be opened, and each URL serves just the map it was given. Paths that
+ * escape that map are rejected. No filesystem or network access is performed.
+ */
+final class MemClassOutputClasspath {
+
+ private static final String PROTOCOL = "x-groovy-mem";
+
+ private static final Map<GroovyClassLoader, Map<String, byte[]>> INSTALLED
=
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ private MemClassOutputClasspath() {
+ }
+
+ /**
+ * Attaches an in-memory classpath to {@code loader}, or returns the map
+ * already attached to that loader. A second compile on the same loader
+ * therefore replaces entries in place and stays visible to
+ * {@code getResource}.
+ *
+ * @param loader the compilation unit's class loader
+ * @return the mutable resource map, keyed by {@code
com/example/Type.class}
+ */
+ @SuppressWarnings("deprecation") // custom handler; URL.of(URI, handler)
is not on the Java 17 baseline
+ static Map<String, byte[]> install(final GroovyClassLoader loader) {
Review Comment:
Agreed. I took the map out of this PR.
A second unit on the same loader only does something useful when the class
names do not overlap, or when nothing has been defined yet. I could not find a
normal API that is that situation. `parseClass` is the long-lived loader you
described, and it already uses a short-lived `InnerLoader` for the definitions.
A map keyed by the compilation unit's loader does not follow that split:
`parseClass` does not build a `JavaAwareCompilationUnit`, and
`InnerLoader#addURL` would publish on the parent anyway. The language server
does reuse a loader across units, but it recompiles overlapping files and does
not use in-memory class output. Caching on that loader would be the wrong
structure even there.
What remains is the part this issue is actually about. With `memStub` and
`memClassOutput`, javac `CLASS_OUTPUT` stays in memory for that compilation
unit, the unit's loader can resolve it, and no stub directory or target
directory is required. The store lives on the unit. A failed compile drops the
stage. A later unit does not share it. Compiling the same classes again is a
new loader.
I would like to file the cross-unit question separately once this is in, and
look at it against `parseClass` / a real embedder rather than a test that
passes the same loader into two units. The outline I have in mind:
- Title along the lines of "GROOVY-12711 follow-up: in-memory class output
across compilation units".
- Show, through an existing API, which loader is long-lived and which loader
defines classes.
- Respect that a URL cannot be removed and that a defined class cannot be
replaced, so appending another mem URL on a long-lived loader is not an update
mechanism.
- Do not turn `memClassOutput` on inside `parseClass` or the language server
as part of that issue; those are consumers of whatever rule we settle.
No such cache is in this PR anymore.
> Joint compilation: keep javac CLASS_OUTPUT in memory when memStub is enabled
> ----------------------------------------------------------------------------
>
> Key: GROOVY-12711
> URL: https://issues.apache.org/jira/browse/GROOVY-12711
> Project: Groovy
> Issue Type: Improvement
> Components: Compiler
> Reporter: Daniel Sun
> Assignee: Daniel Sun
> Priority: Major
>
> h2. Summary
> With {{memStub}} and {{memClassOutput}} both set to {{Boolean.TRUE}}, joint
> compilation keeps the generated Java stubs and the class files on the heap.
> The class files are published on the compilation class loader, and the
> compile needs neither a stub directory nor a class-output directory.
> h2. Background
> Joint compilation can already keep generated Java stubs in memory. The joint
> option {{memStub}} ({{CompilerConfiguration.MEM_STUB}}) has been available
> since 3.0.0. {{JavaAwareCompilationUnit}} writes those stubs at
> {{Phases.CONVERSION}}, including signatures contributed by GEP-21 transforms,
> and runs javac at {{Phases.SEMANTIC_ANALYSIS}}.
> Class files were the remaining disk step. Javac wrote {{CLASS_OUTPUT}} to a
> target directory, so the core API could not keep both the stubs and the class
> files off disk.
> {{memClassOutput}} adds that step. It takes effect only together with
> {{memStub}}, and the compilation class loader publishes the class files.
> {{groovy-lsp}} does not set the option. It still uses its own Java compiler
> and an owner-only temporary directory for javac output.
> h2. Motivation
> * Tests, IDEs, and agent tools that edit Groovy and Java together need a
> joint compile that does not write stubs or class files into the project tree
> or into a temporary directory that must be locked down and removed.
> * A stub directory and a class-output directory add latency, race with file
> watchers, and can disagree with the sources the compiler just parsed.
> * In-memory stubs were already available. Keeping the class files in memory
> as well completes the joint cycle inside groovy-core. Later introspection
> tools can build on that cycle, and those tools remain separate work.
> h2. Goals
> * {{memClassOutput}} ({{CompilerConfiguration.MEM_CLASS_OUTPUT}}) is an
> explicit joint option, new in 7.0.0. It is honored only when {{memStub}} is
> also {{Boolean.TRUE}}. A missing target directory does not turn it on.
> * With both options set, {{JavaAwareCompilationUnit}} and the default javac
> compiler finish a mixed Groovy and Java compile with no {{stubDir}} and no
> filesystem target directory. The class bytes are published on the compilation
> class loader. Resolution reads that loader, in the order described under
> "What callers should expect".
> * {{groovyc}} and the Ant task leave the option unset, so disk {{stubDir}}
> and target-directory output remain the default.
> * The joint-compilation section of the groovyc guide documents {{memStub}}
> and {{memClassOutput}}.
> * Callers enable this through {{CompilerConfiguration}} and
> {{JavaAwareCompilationUnit}}. The file manager that stores the bytes is
> package-private, in {{org.codehaus.groovy.tools.javac}}.
> * Tests show a Groovy and Java cycle with no stub directory and no
> class-output directory, a javac type error on that path, and the unchanged
> disk path.
> h2. Non-Goals
> * groovy-core does not gain an MCP server, a JSON-RPC API, or an MCP library,
> and it does not change the LSP protocol.
> * {{groovy-lsp}} and {{groovy-mcp}} stay as they are. {{groovy-lsp}} still
> uses its own Java compiler and an owner-only temporary directory. Only
> {{JavacJavaCompiler}} implements {{memClassOutput}}, so using the option
> there is separate work.
> * This issue does not add an overlay filesystem or a multi-tenant sandbox.
> * Dynamic Groovy is not switched over to static type checking.
> * Builds that leave the option unset keep disk {{stubDir}} and
> target-directory output. The system property {{groovy.mem.stub}} enables
> in-memory stubs only.
> * GEP-21 stub generation, including {{MemJavaFileObject}}, stays as it is.
> Signature changes that must be visible to javac still come from the GEP-21
> {{Phases.CONVERSION}} stubbers. Other AST transforms are not moved ahead of
> stub emission.
> h2. Enabling the option
> {{memClassOutput}} ({{CompilerConfiguration.MEM_CLASS_OUTPUT}}) is new in
> 7.0.0. {{CompilerConfiguration.isMemClassOutput()}} is true only when
> {{memStub}} ({{CompilerConfiguration.MEM_STUB}}) and {{memClassOutput}} are
> both {{Boolean.TRUE}}.
> {code:java}
> CompilerConfiguration config = new CompilerConfiguration();
> Map<String, Object> options = new HashMap<>();
> options.put(CompilerConfiguration.MEM_STUB, Boolean.TRUE);
> options.put(CompilerConfiguration.MEM_CLASS_OUTPUT, Boolean.TRUE);
> config.setJointCompilationOptions(options);
> {code}
> * {{memClassOutput}} without {{memStub}} fails the compile with
> {{memClassOutput requires memStub}}. The system property {{groovy.mem.stub}}
> enables {{memStub}} only.
> * Only {{JavacJavaCompiler}} implements the option. Any other
> {{JavaCompiler}} fails the compile with {{memClassOutput requires the default
> javac joint compiler}}. Using that javac implementation with a plain
> {{CompilationUnit}} fails the compile with {{memClassOutput requires
> JavaAwareCompilationUnit}}.
> * When both options are set, {{stubDir}} is not required. The compiler does
> not add {{-d}}. If the joint options still pass {{-d}}, the file manager
> consumes it and keeps the class files in memory. {{targetDirectory}} is not
> added to the compilation class loader, because an earlier classpath entry
> would hide those classes.
> h2. How classes are published
> {{JavaAwareCompilationUnit}} captures javac {{CLASS_OUTPUT}} with
> {{InMemoryJavaFileManager}}, in package {{org.codehaus.groovy.tools.javac}},
> when javac runs. It writes the Groovy class files from the same compile into
> that same map during output. Classpath and source input still come from the
> standard file manager. The manager does no network I/O. The file manager and
> the classpath store are package-private; callers enable the behavior with the
> joint option above.
> One URL attaches the map to the compilation {{GroovyClassLoader}}:
> * The protocol is {{x-groovy-mem}} and the host is empty, so hashing or
> comparing the {{URL}} does not look up a DNS name. On the Java 17 baseline a
> security manager can still require {{NetPermission("specifyStreamHandler")}}
> to install the handler.
> * The protocol is not registered with the JVM. Only URLs created for this
> classpath can be opened. A path that escapes the map is rejected. A plus sign
> in a resource name stays a plus sign.
> * The URL is appended to the loader. {{URLClassLoader}} cannot remove a URL,
> so another {{JavaAwareCompilationUnit}} on the same loader reuses the store.
> {{groovyc}} and the Ant task build one unit per run.
> The unit opens one stage on that store. Lookup prefers the stage and
> otherwise returns the last committed bytes. A class written during this
> compile is visible to later phases. A class this compile has not written is
> still read from the previous commit, when one exists.
> The stage is committed when {{compile}} returns normally after class output
> has completed. Commit adds and replaces entries and leaves every other name
> as it was. A failed compile discards the stage, and the previously committed
> classes stay in place.
> A compile that stops at semantic analysis leaves the stage open. Javac has
> already run by then. A later {{compile}} on the same unit reuses the stage
> and does not start javac again. A different unit cannot publish while that
> stage is open: its {{compile}} throws {{IllegalStateException}} with the
> message {{another compilation is publishing to this class loader}}.
> h2. What callers should expect
> * A later successful compile replaces the stored bytes of each class it
> writes. {{loadClass}} returns the class the loader has already defined. A
> fresh loader is required to run the new bytes.
> * Lookup searches the parent loader first, then URLs in insertion order. The
> in-memory URL is last. A parent loader or an earlier classpath entry is what
> resolution and {{getResource}} see. Commit does not change that order, so the
> earlier entry still hides the bytes.
> * The compile records a warning at {{WarningMessage.LIKELY_ERRORS}}, which
> the default warning level includes. The warning text contains {{hidden by}}.
> Type checking uses the visible class. The hidden bytes are committed only
> when that compile succeeds. A failure discards the stage.
> * Class files that javac produces for the Groovy stubs stay inside javac and
> are not published. The stub is an input to javac.
> * Annotation processing is turned off with {{-proc:none}} unless the joint
> options request {{processor}}, {{processorpath}}, {{processor-module-path}},
> {{proc}}, or a flag whose name starts with {{proc:}}. Generated Java sources
> ({{SOURCE_OUTPUT}}) and non-class {{CLASS_OUTPUT}} resources stay in memory.
> Module-oriented file-manager locations are not supported.
> * {{import module}} skips loader URLs whose protocol is not {{file}}. The
> groovysh package scan ignores a URL that is neither a file nor a jar, instead
> of failing the listing. The console classpath dialog lists {{file}} URLs
> only. The in-memory URL is left out of all three.
> * The decompiler caches class stubs for {{file}}, {{jar}}, and {{jrt}} URLs
> only. The in-memory URL keeps a stable identity while its bytes can change,
> so those bytes are read again on the next lookup.
> h2. Tests
> {{MemClassOutputTest}} and {{InMemoryJavaFileManagerTest}} cover:
> * a Groovy and Java cycle with no stub directory and no class-output
> directory, including a type error reported from javac
> * disk {{stubDir}} with the option off, and {{memStub}} alone still writing
> the target directory
> * {{memClassOutput}} without {{memStub}}, a joint compiler other than javac,
> and javac invoked on a plain {{CompilationUnit}}
> * a second unit on the same loader, a unit opened while a stage is still
> open, and a failed recompile that keeps the last committed bytes
> * a compile stopped at semantic analysis and then finished on the same unit,
> classes a later compile does not write again, and stub class files left
> unpublished
> * {{import module}}, a shadowed classpath entry, and a class the loader has
> already defined
> * a plus sign in a resource name, a path that escapes the map, and generated
> Java sources kept in memory
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
