feat: DH-23013: Keep query compiler output in memory rather than on disk#8186
feat: DH-23013: Keep query compiler output in memory rather than on disk#8186niloc132 wants to merge 9 commits into
Conversation
|
|
||
| /** | ||
| * Java compiler api, specifically geared towards query language expressions. Implementations should be constructed with | ||
| * a path on disk to existing bytecode to use in addition to `java.class.path`. Likewise, callers must ensure that the |
There was a problem hiding this comment.
Why should they have the extra path? If you are not a Groovy thing that is writing to disk, I don't think you would need it. Mabye they "may" be constructed that way?
There was a problem hiding this comment.
RIght - should be entirely optional, I'll change the name of the createForUnitTest() so we have a no-arg impl
There was a problem hiding this comment.
Also removed the annotation processor arg, since it is always null... except a test that makes it broken and non-null, which is silly.
There was a problem hiding this comment.
Pull request overview
This PR refactors the Java query compiler implementation to keep compilation outputs in-memory (rather than writing generated .class files to disk), enabling safer/cleaner class-cache directory replacement and better reuse via weakly-cached compiled classes.
Changes:
- Reworks
QueryCompilerImplto capture compiler outputs in memory, define classes via per-batch classloaders, and cache results with weak references. - Updates script sessions, tests, and benchmarks to use the revised
QueryCompilerImpl.create(...)/createForUnitTests()APIs. - Adjusts Groovy/Python execution context documentation examples to match the new API signature.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| extensions/performance/src/test/java/io/deephaven/engine/table/impl/util/TestUpdateAncestorViz.java | Updates test setup to use createForUnitTests() instead of a temp cache dir + explicit classloader. |
| extensions/parquet/benchmark/src/benchmark/java/io/deephaven/benchmark/parquet/table/TableWriteBenchmark.java | Switches benchmark execution context to the unit-test compiler factory. |
| engine/table/src/test/java/io/deephaven/engine/updategraph/impl/TestEventDrivenUpdateGraph.java | Simplifies unit test compiler creation to createForUnitTests(). |
| engine/table/src/test/java/io/deephaven/engine/table/impl/select/TestConditionFilterGeneration.java | Removes now-unused QueryCompilerImpl import. |
| engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java | Stops writing class bytes into the on-disk cache directory during class-cache updates. |
| engine/table/src/main/java/io/deephaven/engine/util/AbstractScriptSession.java | Updates compiler construction to the new QueryCompilerImpl.create(File) API. |
| engine/table/src/main/java/io/deephaven/engine/context/QueryCompilerImpl.java | Core refactor: compile output captured in-memory, per-batch classloading, weak caching / eviction, updated classpath handling. |
| engine/context/src/test/java/io/deephaven/engine/context/TestQueryCompiler.java | Removes temp-folder usage and updates compiler construction for tests. |
| engine/context/src/main/java/io/deephaven/engine/context/QueryCompiler.java | Adds interface-level Javadoc describing compiler expectations/contract. |
| docs/python/conceptual/execution-context.md | Updates Python example to match QueryCompilerImpl.create(File) signature. |
| docs/groovy/conceptual/execution-context.md | Updates Groovy examples to match QueryCompilerImpl.create(File) signature. |
| @Override | ||
| public JavaFileObject getJavaFileForOutput( | ||
| Location location, String className, JavaFileObject.Kind kind, FileObject sibling) { | ||
| final InMemoryClassFileObject fileObject = new InMemoryClassFileObject(className); | ||
| outputClasses.put(className, fileObject); | ||
| return fileObject; | ||
| } |
There was a problem hiding this comment.
I'm adding the if check, but throwing if output isn't bytecode.
* restore private access to method * infinite wait on compilation
While QueryCompilerImpl continues to read bytecode from disk (as required by JavaCompiler), this patch reads the generated classes into memory and loads them all up in a single classloader per batch, and keeps weak references to them for reuse while still live. This enables users to replace the classes directory at will, including deleting class files from within it, and not needing to worry about expectations of the query compiler.
Unblocks #7451
Co-authored-by: Claude Opus 4.6 noreply@anthropic.com