From ce95e4e48260d19c0447a92332623a70a8f0ba32 Mon Sep 17 00:00:00 2001 From: Kaz Date: Fri, 20 Sep 2024 12:45:01 -0700 Subject: [PATCH 01/16] Stateless parser API Stateless (static) parser interface. Buffer-reuse optimization is now hidden behind JNI FFI implementation. Fixes #11121 and prevents similar bugs. --- .../org/enso/runner/EnsoLibraryFeature.java | 5 +- .../scala/org/enso/compiler/Compiler.scala | 14 ++-- .../compiler/phase/BuiltinsIrBuilder.scala | 6 +- .../instrument/ChangesetBuilder.scala | 15 ++-- .../compiler/test/CompilerTestSetup.scala | 42 +--------- .../org/enso/compiler/test/CompilerTests.java | 17 +--- .../test/VectorArraySignatureTest.java | 21 +---- .../org/enso/compiler/test/CompilerTest.scala | 42 +--------- .../test/pass/resolve/GlobalNamesTest.scala | 4 +- .../org/enso/compiler/core/EnsoParser.java | 49 +++--------- .../enso/compiler/core/EnsoParserTest.java | 28 +------ .../src/main/java/org/enso/ydoc/Ydoc.java | 1 - .../enso/ydoc/polyfill/ParserPolyfill.java | 26 ++---- .../ydoc/polyfill/ParserPolyfillTest.java | 1 - .../java/org/enso/syntax2/Parser.java | 79 ++++++++----------- .../generate-java/src/bin/java-tests.rs | 4 +- lib/rust/parser/jni/src/lib.rs | 68 ++++------------ .../enso4igv/enso/EnsoErrorProvider.java | 3 +- .../tools/enso4igv/enso/EnsoStructure.java | 3 +- 19 files changed, 102 insertions(+), 326 deletions(-) diff --git a/engine/runner/src/main/java/org/enso/runner/EnsoLibraryFeature.java b/engine/runner/src/main/java/org/enso/runner/EnsoLibraryFeature.java index 3463df945e2b..433271da4639 100644 --- a/engine/runner/src/main/java/org/enso/runner/EnsoLibraryFeature.java +++ b/engine/runner/src/main/java/org/enso/runner/EnsoLibraryFeature.java @@ -6,6 +6,7 @@ import java.nio.file.Path; import java.util.LinkedHashSet; import java.util.TreeSet; +import org.enso.compiler.core.EnsoParser; import org.enso.compiler.core.ir.module.scope.imports.Polyglot; import org.enso.pkg.PackageManager$; import org.graalvm.nativeimage.hosted.Feature; @@ -45,14 +46,14 @@ public void beforeAnalysis(BeforeAnalysisAccess access) { */ var classes = new TreeSet(); - try (var parser = new org.enso.compiler.core.EnsoParser()) { + try { for (var p : libs) { var result = PackageManager$.MODULE$.Default().loadPackage(p.toFile()); if (result.isSuccess()) { var pkg = result.get(); for (var src : pkg.listSourcesJava()) { var code = Files.readString(src.file().toPath()); - var ir = parser.compile(code); + var ir = EnsoParser.compile(code); for (var imp : asJava(ir.imports())) { if (imp instanceof Polyglot poly && poly.entity() instanceof Polyglot.Java entity) { var name = new StringBuilder(entity.getJavaName()); diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/Compiler.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/Compiler.scala index 95d96e71399d..76c70fc453d6 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/Compiler.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/Compiler.scala @@ -35,6 +35,7 @@ import org.enso.compiler.phase.exports.{ ExportsResolution } import org.enso.syntax2.Tree +import org.enso.syntax2.Parser import java.io.PrintStream import java.util.concurrent.{ @@ -69,7 +70,6 @@ class Compiler( if (config.outputRedirect.isDefined) new PrintStream(config.outputRedirect.get) else context.getOut - private lazy val ensoCompiler: EnsoParser = new EnsoParser() /** Java accessor */ def getConfig(): CompilerConfig = config @@ -598,11 +598,8 @@ class Compiler( ) val src = context.getCharacters(module) - val idMap = context.getIdMap(module) - val tree = ensoCompiler.parse(src) - val expr = - if (idMap == null) ensoCompiler.generateIR(tree) - else ensoCompiler.generateModuleIr(tree, idMap.values) + val idMap = Option(context.getIdMap(module)) + val expr = EnsoParser.compile(src, idMap.map(_.values).orNull) val exprWithModuleExports = if (context.isSynthetic(module)) @@ -685,9 +682,8 @@ class Compiler( inlineContext: InlineContext ): Option[(InlineContext, Expression)] = { val newContext = inlineContext.copy(freshNameSupply = Some(freshNameSupply)) - val tree = ensoCompiler.parse(srcString) - ensoCompiler.generateIRInline(tree).map { ir => + EnsoParser.compileInline(srcString).map { ir => val compilerOutput = runCompilerPhasesInline(ir, newContext) runErrorHandlingInline(compilerOutput, newContext) (newContext, compilerOutput) @@ -700,7 +696,7 @@ class Compiler( * @return A Tree representation of `source` */ def parseInline(source: CharSequence): Tree = - ensoCompiler.parse(source) + Parser.parse(source) /** Enhances the provided IR with import/export statements for the provided list * of fully qualified names of modules. The statements are considered to be "synthetic" i.e. compiler-generated. diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/phase/BuiltinsIrBuilder.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/phase/BuiltinsIrBuilder.scala index 255aa98a2f99..2f1785d2f898 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/phase/BuiltinsIrBuilder.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/phase/BuiltinsIrBuilder.scala @@ -11,8 +11,6 @@ import org.enso.compiler.data.CompilerConfig import org.enso.common.CompilationStage import org.enso.compiler.phase.exports.ExportsResolution -import scala.util.Using - /** A phase responsible for initializing the builtins' IR from the provided * source. */ @@ -44,9 +42,7 @@ object BuiltinsIrBuilder { freshNameSupply = Some(freshNameSupply), compilerConfig = CompilerConfig(warningsEnabled = false) ) - val initialIr = Using(new EnsoParser) { compiler => - compiler.compile(module.getCharacters) - }.get + val initialIr = EnsoParser.compile(module.getCharacters) val irAfterModDiscovery = passManager.runPassesOnModule( initialIr, moduleContext, diff --git a/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/ChangesetBuilder.scala b/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/ChangesetBuilder.scala index 7ad527e5dafc..c872efd9fe71 100644 --- a/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/ChangesetBuilder.scala +++ b/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/ChangesetBuilder.scala @@ -13,7 +13,6 @@ import org.enso.text.editing.{IndexedSource, TextEditor} import java.util.UUID import scala.collection.mutable -import scala.util.Using /** The changeset of a module containing the computed list of invalidated * expressions. @@ -97,14 +96,12 @@ final class ChangesetBuilder[A: TextEditor: IndexedSource]( } val source = Source.newBuilder("enso", value, null).build - Using(new EnsoParser) { compiler => - compiler - .generateIRInline(compiler.parse(source.getCharacters())) - .flatMap(_ match { - case ir: Literal => Some(ir.setLocation(oldIr.location)) - case _ => None - }) - }.get + EnsoParser + .compileInline(source.getCharacters()) + .flatMap(_ match { + case ir: Literal => Some(ir.setLocation(oldIr.location)) + case _ => None + }) } oldIr match { diff --git a/engine/runtime-instrument-common/src/test/scala/org/enso/compiler/test/CompilerTestSetup.scala b/engine/runtime-instrument-common/src/test/scala/org/enso/compiler/test/CompilerTestSetup.scala index 17b353bd464e..c7fbe852fc66 100644 --- a/engine/runtime-instrument-common/src/test/scala/org/enso/compiler/test/CompilerTestSetup.scala +++ b/engine/runtime-instrument-common/src/test/scala/org/enso/compiler/test/CompilerTestSetup.scala @@ -21,42 +21,6 @@ import org.enso.common.CompilationStage trait CompilerTestSetup { // === IR Utilities ========================================================= - /** An extension method to allow converting string source code to IR as a - * module. - * - * @param source the source code to convert - */ - implicit private class ToIrModule(source: String) { - - /** Converts program text to a top-level Enso module. - * - * @return the [[IR]] representing [[source]] - */ - def toIrModule: Module = { - val compiler = new EnsoParser() - try compiler.compile(source) - finally compiler.close() - } - } - - /** An extension method to allow converting string source code to IR as an - * expression. - * - * @param source the source code to convert - */ - implicit private class ToIrExpression(source: String) { - - /** Converts the program text to an Enso expression. - * - * @return the [[IR]] representing [[source]], if it is a valid expression - */ - def toIrExpression: Option[Expression] = { - val compiler = new EnsoParser() - try compiler.generateIRInline(compiler.parse(source)) - finally compiler.close() - } - } - /** Provides an extension method allowing the running of a specified list of * passes on the provided IR. * @@ -112,7 +76,7 @@ trait CompilerTestSetup { * @return IR appropriate for testing the alias analysis pass as a module */ def preprocessModule(implicit moduleContext: ModuleContext): Module = { - source.toIrModule.runPasses(passManager, moduleContext) + EnsoParser.compile(source).runPasses(passManager, moduleContext) } /** Translates the source code into appropriate IR for testing this pass @@ -123,7 +87,9 @@ trait CompilerTestSetup { def preprocessExpression(implicit inlineContext: InlineContext ): Option[Expression] = { - source.toIrExpression.map(_.runPasses(passManager, inlineContext)) + EnsoParser + .compileInline(source) + .map(_.runPasses(passManager, inlineContext)) } } diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/CompilerTests.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/CompilerTests.java index 7b01bed83311..3a3affc082ba 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/CompilerTests.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/CompilerTests.java @@ -11,25 +11,10 @@ import org.enso.compiler.core.EnsoParser; import org.enso.compiler.core.IR; import org.enso.compiler.core.ir.Module; -import org.junit.AfterClass; -import org.junit.BeforeClass; public abstract class CompilerTests { - - protected static EnsoParser ensoCompiler; - - @BeforeClass - public static void initEnsoParser() { - ensoCompiler = new EnsoParser(); - } - - @AfterClass - public static void closeEnsoParser() throws Exception { - ensoCompiler.close(); - } - protected static Module parse(CharSequence code) { - Module ir = ensoCompiler.compile(code); + Module ir = EnsoParser.compile(code); assertNotNull("IR was generated", ir); return ir; } diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/VectorArraySignatureTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/VectorArraySignatureTest.java index 5cabcd4cb387..e8f03c272562 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/VectorArraySignatureTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/VectorArraySignatureTest.java @@ -19,25 +19,10 @@ import org.enso.compiler.core.ir.Name; import org.enso.compiler.core.ir.expression.Comment; import org.enso.compiler.core.ir.module.scope.Definition; -import org.junit.AfterClass; -import org.junit.BeforeClass; import org.junit.Test; import scala.Function1; public class VectorArraySignatureTest { - private static EnsoParser ensoCompiler; - - @BeforeClass - public static void initEnsoParser() { - ensoCompiler = new EnsoParser(); - } - - @AfterClass - public static void closeEnsoParser() throws Exception { - ensoCompiler.close(); - ensoCompiler = null; - } - @Test public void testParseVectorAndArray() throws Exception { var p = Paths.get("../../distribution/").toFile().getCanonicalFile(); @@ -81,8 +66,7 @@ public FileVisitResult postVisitDirectory(Path t, IOException ioe) throws IOExce var vectorSrc = Files.readString(vectorAndArray[1]); var arrayIR = - ensoCompiler - .compile(arraySrc) + EnsoParser.compile(arraySrc) .preorder() .filter( (v) -> { @@ -95,8 +79,7 @@ public FileVisitResult postVisitDirectory(Path t, IOException ioe) throws IOExce }) .head(); var vectorIR = - ensoCompiler - .compile(vectorSrc) + EnsoParser.compile(vectorSrc) .preorder() .filter( (v) -> { diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala index 7befe14b968a..a082157da432 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala @@ -31,42 +31,6 @@ trait CompilerTest extends AnyWordSpecLike with Matchers with CompilerRunner trait CompilerRunner { // === IR Utilities ========================================================= - /** An extension method to allow converting string source code to IR as a - * module. - * - * @param source the source code to convert - */ - implicit class ToIrModule(source: String) { - - /** Converts program text to a top-level Enso module. - * - * @return the [[IR]] representing [[source]] - */ - def toIrModule: Module = { - val compiler = new EnsoParser() - try compiler.compile(source) - finally compiler.close() - } - } - - /** An extension method to allow converting string source code to IR as an - * expression. - * - * @param source the source code to convert - */ - implicit class ToIrExpression(source: String) { - - /** Converts the program text to an Enso expression. - * - * @return the [[IR]] representing [[source]], if it is a valid expression - */ - def toIrExpression: Option[Expression] = { - val compiler = new EnsoParser() - try compiler.generateIRInline(compiler.parse(source)) - finally compiler.close() - } - } - /** Provides an extension method allowing the running of a specified list of * passes on the provided IR. * @@ -137,7 +101,7 @@ trait CompilerRunner { * @return IR appropriate for testing the alias analysis pass as a module */ def preprocessModule(implicit moduleContext: ModuleContext): Module = { - source.toIrModule.runPasses(passManager, moduleContext) + EnsoParser.compile(source).runPasses(passManager, moduleContext) } /** Translates the source code into appropriate IR for testing this pass @@ -148,7 +112,9 @@ trait CompilerRunner { def preprocessExpression(implicit inlineContext: InlineContext ): Option[Expression] = { - source.toIrExpression.map(_.runPasses(passManager, inlineContext)) + EnsoParser + .compileInline(source) + .map(_.runPasses(passManager, inlineContext)) } } diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/GlobalNamesTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/GlobalNamesTest.scala index e298f0c25051..f16c70bea1ee 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/GlobalNamesTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/GlobalNamesTest.scala @@ -2,7 +2,7 @@ package org.enso.compiler.test.pass.resolve import org.enso.compiler.Passes import org.enso.compiler.context.{FreshNameSupply, ModuleContext} -import org.enso.compiler.core.IR +import org.enso.compiler.core.{EnsoParser, IR} import org.enso.compiler.core.Implicits.AsMetadata import org.enso.compiler.core.ir.Expression import org.enso.compiler.core.ir.Function @@ -84,7 +84,7 @@ class GlobalNamesTest extends CompilerTest { |add_one x = x + 1 | |""".stripMargin - val parsed = code.toIrModule + val parsed = EnsoParser.compile(code) val moduleMapped = passManager.runPassesOnModule(parsed, ctx, group1) ModuleTestUtils.unsafeSetIr(both._2, moduleMapped) diff --git a/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java index a44cdac9b9ec..2b0a63efdd23 100644 --- a/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java +++ b/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java @@ -6,48 +6,23 @@ import org.enso.compiler.core.ir.Location; import org.enso.compiler.core.ir.Module; import org.enso.syntax2.Parser; -import org.enso.syntax2.Tree; -public final class EnsoParser implements AutoCloseable { - private final Parser parser; - - public EnsoParser() { - Parser p; - try { - p = Parser.create(); - } catch (LinkageError err) { - err.printStackTrace(); - throw err; - } - this.parser = p; +public final class EnsoParser { + public static Module compile(CharSequence src) { + return compile(src, null); } - @Override - public void close() throws Exception { - if (parser != null) { - parser.close(); + public static Module compile(CharSequence src, Map idMap) { + var tree = Parser.parse(src); + var treeToIr = TreeToIr.MODULE; + if (idMap != null) { + treeToIr = new TreeToIr(idMap); } + return treeToIr.translate(tree); } - public Module compile(CharSequence src) { - var tree = parser.parse(src); - return generateIR(tree); - } - - public Tree parse(CharSequence src) { - return parser.parse(src); - } - - public Module generateIR(Tree t) { - return TreeToIr.MODULE.translate(t); - } - - public Module generateModuleIr(Tree t, Map idMap) { - var treeToIr = new TreeToIr(idMap); - return treeToIr.translate(t); - } - - public scala.Option generateIRInline(Tree t) { - return TreeToIr.MODULE.translateInline(t); + public static scala.Option compileInline(CharSequence src) { + var tree = Parser.parse(src); + return TreeToIr.MODULE.translateInline(tree); } } diff --git a/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserTest.java b/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserTest.java index 249151bec5e2..256d41595239 100644 --- a/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserTest.java +++ b/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserTest.java @@ -19,28 +19,10 @@ import org.enso.compiler.core.ir.Module; import org.enso.compiler.core.ir.expression.Error; import org.enso.compiler.core.ir.module.scope.definition.Method; -import org.junit.AfterClass; -import org.junit.BeforeClass; import org.junit.Test; import scala.jdk.javaapi.CollectionConverters; public class EnsoParserTest { - private static EnsoParser ensoCompiler; - - @BeforeClass - public static void initEnsoParser() { - try { - ensoCompiler = new EnsoParser(); - } catch (LinkageError e) { - throw new AssertionError(e); - } - } - - @AfterClass - public static void closeEnsoParser() throws Exception { - if (ensoCompiler != null) ensoCompiler.close(); - } - @Test public void testParseMain7Foo() { parseTest(""" @@ -1529,7 +1511,9 @@ private static void equivalenceTest(String code1, String code2) throws IOExcepti } private static Module compile(String code) { - return compile(ensoCompiler, code); + var ir = EnsoParser.compile(code); + assertNotNull("IR was generated", ir); + return ir; } private void expectNoErrorsInIr(Module moduleIr) { @@ -1544,12 +1528,6 @@ private void expectNoErrorsInIr(Module moduleIr) { }); } - public static Module compile(EnsoParser c, String code) { - var ir = c.compile(code); - assertNotNull("IR was generated", ir); - return ir; - } - static void assertIR(String msg, Module old, Module now) throws IOException { Function filter = f -> simplifyIR(f, true, true, false); String ir1 = filter.apply(old); diff --git a/lib/java/ydoc-server/src/main/java/org/enso/ydoc/Ydoc.java b/lib/java/ydoc-server/src/main/java/org/enso/ydoc/Ydoc.java index 088a9151e1ec..228eed0b77a2 100644 --- a/lib/java/ydoc-server/src/main/java/org/enso/ydoc/Ydoc.java +++ b/lib/java/ydoc-server/src/main/java/org/enso/ydoc/Ydoc.java @@ -149,7 +149,6 @@ public void start() throws ExecutionException, InterruptedException, IOException public void close() throws Exception { executor.shutdownNow(); executor.awaitTermination(3, TimeUnit.SECONDS); - parser.close(); if (context != null) { context.close(true); } diff --git a/lib/java/ydoc-server/src/main/java/org/enso/ydoc/polyfill/ParserPolyfill.java b/lib/java/ydoc-server/src/main/java/org/enso/ydoc/polyfill/ParserPolyfill.java index 5cc989cfa27c..3923652287ac 100644 --- a/lib/java/ydoc-server/src/main/java/org/enso/ydoc/polyfill/ParserPolyfill.java +++ b/lib/java/ydoc-server/src/main/java/org/enso/ydoc/polyfill/ParserPolyfill.java @@ -9,7 +9,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public final class ParserPolyfill implements AutoCloseable, ProxyExecutable, Polyfill { +public final class ParserPolyfill implements ProxyExecutable, Polyfill { private static final Logger log = LoggerFactory.getLogger(ParserPolyfill.class); @@ -19,21 +19,10 @@ public final class ParserPolyfill implements AutoCloseable, ProxyExecutable, Pol private static final String PARSER_JS = "parser.js"; - private final Parser parser; - - public ParserPolyfill() { - Parser p; - try { - p = Parser.create(); - } catch (LinkageError e) { - log.error("Failed to create parser", e); - throw e; - } - this.parser = p; - } + public ParserPolyfill() {} @Override - public final void initialize(Context ctx) { + public void initialize(Context ctx) { Source parserJs = Source.newBuilder("js", ParserPolyfill.class.getResource(PARSER_JS)).buildLiteral(); @@ -50,7 +39,7 @@ public Object execute(Value... arguments) { case PARSE_TREE -> { var input = arguments[1].asString(); - yield parser.parseInputLazy(input); + yield Parser.parseInputLazy(input); } case XX_HASH_128 -> { @@ -62,15 +51,10 @@ public Object execute(Value... arguments) { case IS_IDENT_OR_OPERATOR -> { var input = arguments[1].asString(); - yield parser.isIdentOrOperator(input); + yield Parser.isIdentOrOperator(input); } default -> throw new IllegalStateException(command); }; } - - @Override - public void close() { - parser.close(); - } } diff --git a/lib/java/ydoc-server/src/test/java/org/enso/ydoc/polyfill/ParserPolyfillTest.java b/lib/java/ydoc-server/src/test/java/org/enso/ydoc/polyfill/ParserPolyfillTest.java index 6942403ba84b..882a6b9fd633 100644 --- a/lib/java/ydoc-server/src/test/java/org/enso/ydoc/polyfill/ParserPolyfillTest.java +++ b/lib/java/ydoc-server/src/test/java/org/enso/ydoc/polyfill/ParserPolyfillTest.java @@ -37,7 +37,6 @@ public void setup() throws Exception { public void tearDown() throws InterruptedException { super.tearDown(); context.close(); - parser.close(); } @Test diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java index 91731cdde522..f3d666506a84 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java @@ -5,11 +5,22 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; - -public final class Parser implements AutoCloseable { - private static void initializeLibraries() { +import java.util.concurrent.atomic.AtomicBoolean; + +public final class Parser { + static AtomicBoolean initialized = new AtomicBoolean(false); + + private static void ensureInitialized() { + // Optimization: Skip finding and initializing the library when it's definitely already been + // done. + // Note that this inexpensive check may produce false negatives: Nothing prevents multiple + // threads from proceeding to library initialization concurrently (before any thread has + // completed initialization), but that is harmless; `initializeLibraries` is thread-safe and + // idempotent. + if (initialized.get()) return; try { System.loadLibrary("enso_parser"); + initialized.set(true); return; } catch (LinkageError err) { // try harder to find the library @@ -45,10 +56,12 @@ private static void initializeLibraries() { System.load(path.getAbsolutePath()); } catch (NullPointerException | IllegalArgumentException | LinkageError e) { if (searchFromDirToTop(e, root, "target", "rust", "parser-jni", name)) { + initialized.set(true); return; } if (searchFromDirToTop( e, new File(".").getAbsoluteFile(), "target", "rust", "parser-jni", name)) { + initialized.set(true); return; } throw new IllegalStateException("Cannot load parser from " + root, e); @@ -75,25 +88,19 @@ private static boolean searchFromDirToTop(Throwable chain, File root, String... return false; } - private long stateUnlessClosed; - - private Parser(long stateIn) { - stateUnlessClosed = stateIn; - } - - private static native long allocState(); + private Parser() {} - private static native void freeState(long state); + private static native void freeBuffers(); - private static native ByteBuffer parseTree(long state, ByteBuffer input); + private static native ByteBuffer parseTree(ByteBuffer input); - private static native ByteBuffer parseTreeLazy(long state, ByteBuffer input); + private static native ByteBuffer parseTreeLazy(ByteBuffer input); private static native long isIdentOrOperator(ByteBuffer input); - private static native long getLastInputBase(long state); + private static native long getLastInputBase(); - private static native long getMetadata(long state); + private static native long getMetadata(); private static native String getWarningTemplate(int warningId); @@ -101,56 +108,38 @@ private Parser(long stateIn) { static native long getUuidLow(long metadata, long codeOffset, long codeLength); - public static Parser create() { - initializeLibraries(); - var state = allocState(); - return new Parser(state); - } - - public long isIdentOrOperator(CharSequence input) { + public static long isIdentOrOperator(CharSequence input) { + ensureInitialized(); byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); inputBuf.put(inputBytes); - return isIdentOrOperator(inputBuf); } - private long getState() { - if (stateUnlessClosed != 0) { - return stateUnlessClosed; - } else { - throw new IllegalStateException("Parser used after close()"); - } - } - - public ByteBuffer parseInputLazy(CharSequence input) { - var state = getState(); + public static ByteBuffer parseInputLazy(CharSequence input) { + ensureInitialized(); byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); inputBuf.put(inputBytes); - return parseTreeLazy(state, inputBuf); + return parseTreeLazy(inputBuf); } - public Tree parse(CharSequence input) { - var state = getState(); + public static Tree parse(CharSequence input) { + ensureInitialized(); byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); inputBuf.put(inputBytes); - var serializedTree = parseTree(state, inputBuf); - var base = getLastInputBase(state); - var metadata = getMetadata(state); + var serializedTree = parseTree(inputBuf); + var base = getLastInputBase(); + var metadata = getMetadata(); serializedTree.order(ByteOrder.LITTLE_ENDIAN); var message = new Message(serializedTree, input, base, metadata); return Tree.deserialize(message); } public static String getWarningMessage(Warning warning) { + // `ensureInitialized` is unnecessary here because a `Warning` will only be constructed by + // parsing something. return getWarningTemplate(warning.getId()); } - - @Override - public void close() { - freeState(stateUnlessClosed); - stateUnlessClosed = 0; - } } diff --git a/lib/rust/parser/generate-java/src/bin/java-tests.rs b/lib/rust/parser/generate-java/src/bin/java-tests.rs index daf00cc87dd7..49c551aee57f 100644 --- a/lib/rust/parser/generate-java/src/bin/java-tests.rs +++ b/lib/rust/parser/generate-java/src/bin/java-tests.rs @@ -28,7 +28,9 @@ fn main() { println!("import java.nio.ByteOrder;"); println!(); println!("class GeneratedFormatTests {{"); - println!(" private static final Object INIT = {package}.Parser.create();"); + // Force the parser to load its shared library. `parse` handles this because usually it is the + // entry point to the class, but we're doing low-level operations directly. + println!(" private static final Object INIT = {package}.Parser.parse(\"\");"); println!(" private static java.util.Vector accept;"); println!(" private static java.util.Vector reject;"); for (i, case) in cases.accept.iter().enumerate() { diff --git a/lib/rust/parser/jni/src/lib.rs b/lib/rust/parser/jni/src/lib.rs index 0ee8580153b0..b54b3255eafa 100644 --- a/lib/rust/parser/jni/src/lib.rs +++ b/lib/rust/parser/jni/src/lib.rs @@ -16,6 +16,7 @@ use jni::objects::JClass; use jni::sys::jobject; use jni::sys::jstring; use jni::JNIEnv; +use std::cell::UnsafeCell; @@ -31,19 +32,15 @@ static FAILED_SERIALIZE_AST: &str = "Failed to serialize AST to binary format."; /// /// # Safety /// -/// The state MUST be a value returned by `allocState` that has not been passed to `freeState`. -/// The input buffer contents MUST be valid UTF-8. -/// The contents of the returned buffer MUST not be accessed after another call to `parseInput`, or -/// a call to `freeState`. +/// The contents of the returned buffer MUST not be accessed after another call to `parseInput`. #[allow(unsafe_code)] #[no_mangle] pub extern "system" fn Java_org_enso_syntax2_Parser_parseTree( mut env: JNIEnv, _class: JClass, - state: u64, input: JByteBuffer, ) -> jobject { - let state = unsafe { &mut *(state as usize as *mut State) }; + let state = STATE.with(|state| unsafe { state.get().as_mut() }).unwrap(); let input = unsafe { decode_utf8_buffer(&env, &input) }; let mut code = input; let mut meta = None; @@ -76,19 +73,16 @@ pub extern "system" fn Java_org_enso_syntax2_Parser_parseTree( /// /// # Safety /// -/// The state MUST be a value returned by `allocState` that has not been passed to `freeState`. /// The input buffer contents MUST be valid UTF-8. -/// The contents of the returned buffer MUST not be accessed after another call to `parseInput`, or -/// a call to `freeState`. +/// The contents of the returned buffer MUST not be accessed after another call to `parseInput`. #[allow(unsafe_code)] #[no_mangle] pub extern "system" fn Java_org_enso_syntax2_Parser_parseTreeLazy( mut env: JNIEnv, _class: JClass, - state: u64, input: JByteBuffer, ) -> jobject { - let state = unsafe { &mut *(state as usize as *mut State) }; + let state = STATE.with(|state| unsafe { state.get().as_mut() }).unwrap(); let input = unsafe { decode_utf8_buffer(&env, &input) }; let tree = enso_parser::Parser::new().run(input); @@ -126,37 +120,24 @@ pub extern "system" fn Java_org_enso_syntax2_Parser_isIdentOrOperator( /// Return the `base` parameter to pass to the `Message` class along with the other output of the /// most recent call to `parseInput`. -/// -/// # Safety -/// -/// The input MUST have been returned by `allocState`, and MUST NOT have previously been passed to -/// `freeState`. #[allow(unsafe_code)] #[no_mangle] pub extern "system" fn Java_org_enso_syntax2_Parser_getLastInputBase( _env: JNIEnv, _class: JClass, - state: u64, ) -> u64 { - let state = unsafe { &mut *(state as usize as *mut State) }; - state.base + STATE.with(|state| unsafe { state.get().as_ref() }).unwrap().base } /// Return the metadata associated with the most recent parse. -/// -/// # Safety -/// -/// The input MUST have been returned by `allocState`, and MUST NOT have previously been passed to -/// `freeState`. #[allow(unsafe_code)] #[no_mangle] pub extern "system" fn Java_org_enso_syntax2_Parser_getMetadata( _env: JNIEnv, _class: JClass, - state: u64, ) -> u64 { - let state = unsafe { &mut *(state as usize as *mut State) }; - match &state.metadata { + let metadata = &STATE.with(|state| unsafe { state.get().as_ref() }).unwrap().metadata; + match metadata { Some(metadata) => { let metadata: *const _ = metadata; metadata as usize as u64 @@ -165,34 +146,11 @@ pub extern "system" fn Java_org_enso_syntax2_Parser_getMetadata( } } -/// Allocate a new parser state object. The returned value should be passed to `freeState` when no -/// longer needed. -#[allow(unsafe_code, clippy::box_default)] -#[no_mangle] -pub extern "system" fn Java_org_enso_syntax2_Parser_allocState( - _env: JNIEnv, - _class: JClass, -) -> u64 { - Box::into_raw(Box::new(State::default())) as _ -} - -/// Free the resources owned by the state object. -/// -/// # Safety -/// -/// The input MUST have been returned by `allocState`, and MUST NOT have previously been passed to -/// `freeState`. +/// Free the thread's parsing resources. #[allow(unsafe_code)] #[no_mangle] -pub extern "system" fn Java_org_enso_syntax2_Parser_freeState( - _env: JNIEnv, - _class: JClass, - state: u64, -) { - if state != 0 { - let state = unsafe { Box::from_raw(state as usize as *mut State) }; - drop(state); - } +pub extern "system" fn Java_org_enso_syntax2_Parser_freeBuffers(_env: JNIEnv, _class: JClass) { + *STATE.with(|state| unsafe { state.get().as_mut() }).unwrap() = Default::default(); } /// Returns the string template corresponding to the given warning ID. @@ -294,3 +252,7 @@ struct State { output: Vec, metadata: Option, } + +thread_local! { + static STATE: UnsafeCell = Default::default(); +} diff --git a/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoErrorProvider.java b/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoErrorProvider.java index 813107e90095..e166a9313ad2 100644 --- a/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoErrorProvider.java +++ b/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoErrorProvider.java @@ -27,12 +27,11 @@ public List computeErrors(Context ctx) { try { if (ctx.errorKind() == Kind.ERRORS) { LOG.log(Level.FINE, "Processing errors for {0}", ctx.file().getPath()); - var parser = new EnsoParser(); var text = toText(ctx); Function1 where = (loc) -> { return text.substring(loc.start(), loc.end()); }; - var moduleIr = parser.compile(text); + var moduleIr = EnsoParser.compile(text); moduleIr.preorder().foreach((p) -> { if (p instanceof Syntax err && err.location().isDefined()) { var loc = err.location().get(); diff --git a/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoStructure.java b/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoStructure.java index 13e2d169cceb..6094b5f9c4e6 100644 --- a/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoStructure.java +++ b/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoStructure.java @@ -32,9 +32,8 @@ static List collectStructure(Document dcmnt) { } var arr = new ArrayList(); try { - var parser = new EnsoParser(); var text = dcmnt.getText(0, dcmnt.getLength()); - var moduleIr = parser.compile(text); + var moduleIr = EnsoParser.compile(text); var it = moduleIr.bindings().iterator(); collectStructure(file, arr, it); return arr; From 285e2e6e15d0c5acbdab845ef5d4722580e60793 Mon Sep 17 00:00:00 2001 From: Kaz Date: Fri, 20 Sep 2024 21:25:27 -0700 Subject: [PATCH 02/16] Fix initialization --- build.sbt | 1 + 1 file changed, 1 insertion(+) diff --git a/build.sbt b/build.sbt index 7bc80a41789f..78d3731e5843 100644 --- a/build.sbt +++ b/build.sbt @@ -2640,6 +2640,7 @@ lazy val `engine-runner` = project "io.methvin.watchservice.jna.CarbonAPI", "zio.internal.ZScheduler$$anon$4", "org.enso.runner.Main$", + "org.enso.syntax2.Parser", "sun.awt", "sun.java2d", "sun.font", From 23e594f278f87959bb17f2dbc6b656772f74e5d4 Mon Sep 17 00:00:00 2001 From: Kaz Date: Fri, 20 Sep 2024 21:27:15 -0700 Subject: [PATCH 03/16] Fix docs --- lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java index f3d666506a84..39c29f2d9a42 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java @@ -15,7 +15,7 @@ private static void ensureInitialized() { // done. // Note that this inexpensive check may produce false negatives: Nothing prevents multiple // threads from proceeding to library initialization concurrently (before any thread has - // completed initialization), but that is harmless; `initializeLibraries` is thread-safe and + // completed initialization), but that is harmless; library initialization is thread-safe and // idempotent. if (initialized.get()) return; try { From 94ac9a40e99e6e72c8728efbeee138c11cdbb720 Mon Sep 17 00:00:00 2001 From: Kaz Date: Mon, 23 Sep 2024 08:41:43 -0700 Subject: [PATCH 04/16] Revert "Fix initialization" This reverts commit 285e2e6e15d0c5acbdab845ef5d4722580e60793. --- build.sbt | 1 - 1 file changed, 1 deletion(-) diff --git a/build.sbt b/build.sbt index 78d3731e5843..7bc80a41789f 100644 --- a/build.sbt +++ b/build.sbt @@ -2640,7 +2640,6 @@ lazy val `engine-runner` = project "io.methvin.watchservice.jna.CarbonAPI", "zio.internal.ZScheduler$$anon$4", "org.enso.runner.Main$", - "org.enso.syntax2.Parser", "sun.awt", "sun.java2d", "sun.font", From 22923628c9cccf0dbaf90f1f3fb1d2849f2fffbd Mon Sep 17 00:00:00 2001 From: Kaz Date: Mon, 23 Sep 2024 09:08:01 -0700 Subject: [PATCH 05/16] WIP --- .../java/org/enso/compiler/core/EnsoParser.java | 2 ++ .../java/org/enso/syntax2/Parser.java | 14 ++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java index 2b0a63efdd23..4dcd913618bf 100644 --- a/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java +++ b/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java @@ -8,6 +8,8 @@ import org.enso.syntax2.Parser; public final class EnsoParser { + private EnsoParser() {} + public static Module compile(CharSequence src) { return compile(src, null); } diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java index 39c29f2d9a42..50a730ee2937 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java @@ -5,10 +5,9 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; -import java.util.concurrent.atomic.AtomicBoolean; public final class Parser { - static AtomicBoolean initialized = new AtomicBoolean(false); + private static volatile boolean initialized = false; private static void ensureInitialized() { // Optimization: Skip finding and initializing the library when it's definitely already been @@ -17,10 +16,15 @@ private static void ensureInitialized() { // threads from proceeding to library initialization concurrently (before any thread has // completed initialization), but that is harmless; library initialization is thread-safe and // idempotent. - if (initialized.get()) return; + // FIXME: Native image issue + //if (initialized) return; + initializeLibraries(); + initialized = true; + } + + private static void initializeLibraries() { try { System.loadLibrary("enso_parser"); - initialized.set(true); return; } catch (LinkageError err) { // try harder to find the library @@ -56,12 +60,10 @@ private static void ensureInitialized() { System.load(path.getAbsolutePath()); } catch (NullPointerException | IllegalArgumentException | LinkageError e) { if (searchFromDirToTop(e, root, "target", "rust", "parser-jni", name)) { - initialized.set(true); return; } if (searchFromDirToTop( e, new File(".").getAbsoluteFile(), "target", "rust", "parser-jni", name)) { - initialized.set(true); return; } throw new IllegalStateException("Cannot load parser from " + root, e); From 851fda97b62c94b60fddffb842f31c1e0c438a35 Mon Sep 17 00:00:00 2001 From: Kaz Date: Mon, 23 Sep 2024 10:43:06 -0700 Subject: [PATCH 06/16] WIP --- .../java/org/enso/syntax2/Message.java | 10 +- .../java/org/enso/syntax2/Parser.java | 271 +++++++++++------- lib/rust/parser/jni/src/lib.rs | 68 ++++- 3 files changed, 215 insertions(+), 134 deletions(-) diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Message.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Message.java index 0a3f009c8330..43a2341c5d0d 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Message.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Message.java @@ -62,15 +62,7 @@ String getLocation() { } java.util.UUID getUuid(long nodeOffset, long nodeLength) { - long high = Parser.getUuidHigh(metadata, nodeOffset, nodeLength); - long low = Parser.getUuidLow(metadata, nodeOffset, nodeLength); - if (high == 0 && low == 0) { - // The native interface uses the Nil UUID value as a marker to indicate that no UUID was - // attached. - // The Nil UUID will never collide with a real UUID generated by any scheme. - return null; - } - return new java.util.UUID(high, low); + return Parser.getUuid(metadata, nodeOffset, nodeLength); } long position() { diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java index 50a730ee2937..b21ec091d149 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java @@ -2,146 +2,197 @@ import java.io.File; import java.net.URISyntaxException; +import java.nio.BufferUnderflowException; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; +import org.slf4j.LoggerFactory; public final class Parser { - private static volatile boolean initialized = false; - - private static void ensureInitialized() { - // Optimization: Skip finding and initializing the library when it's definitely already been - // done. - // Note that this inexpensive check may produce false negatives: Nothing prevents multiple - // threads from proceeding to library initialization concurrently (before any thread has - // completed initialization), but that is harmless; library initialization is thread-safe and - // idempotent. - // FIXME: Native image issue - //if (initialized) return; - initializeLibraries(); - initialized = true; + private Parser() {} + + public static String getWarningMessage(Warning warning) { + return getWarningTemplate(warning.getId()); } - private static void initializeLibraries() { - try { - System.loadLibrary("enso_parser"); - return; - } catch (LinkageError err) { - // try harder to find the library - } - String os = System.getProperty("os.name"); - String name; - if (os.startsWith("Mac")) { - name = "libenso_parser.dylib"; - } else if (os.startsWith("Windows")) { - name = "enso_parser.dll"; - } else { - name = "libenso_parser.so"; - } + public static long isIdentOrOperator(CharSequence input) { + return worker.get().isIdentOrOperator(input); + } - var whereAmI = Parser.class.getProtectionDomain().getCodeSource().getLocation(); - File root; - try { - root = new File(whereAmI.toURI()).getParentFile(); - } catch (URISyntaxException ex) { - root = new File(".").getAbsoluteFile(); + public static ByteBuffer parseInputLazy(CharSequence input) { + return worker.get().parseInputLazy(input); + } + + public static Tree parse(CharSequence input) { + return worker.get().parse(input); + } + + public static java.util.UUID getUuid(long metadata, long nodeOffset, long nodeLength) { + long high = getUuidHigh(metadata, nodeOffset, nodeLength); + long low = getUuidLow(metadata, nodeOffset, nodeLength); + if (high == 0 && low == 0) { + // The native interface uses the Nil UUID value as a marker to indicate that no UUID was + // attached. + // The Nil UUID will never collide with a real UUID generated by any scheme. + return null; } - try { - var d = root; - File path = null; - while (d != null) { - path = new File(new File(d, "component"), name); - if (path.exists()) break; - d = d.getParentFile(); - } - if (d == null || path == null) { - throw new LinkageError("Cannot find parser in " + root); - } - System.load(path.getAbsolutePath()); - } catch (NullPointerException | IllegalArgumentException | LinkageError e) { - if (searchFromDirToTop(e, root, "target", "rust", "parser-jni", name)) { + return new java.util.UUID(high, low); + } + + /* Worker-thread state */ + + private static final ThreadLocal worker = ThreadLocal.withInitial(Worker::create); + + private static class Worker implements AutoCloseable { + private static void initializeLibraries() { + try { + System.loadLibrary("enso_parser"); return; + } catch (LinkageError err) { + // try harder to find the library } - if (searchFromDirToTop( - e, new File(".").getAbsoluteFile(), "target", "rust", "parser-jni", name)) { - return; + String os = System.getProperty("os.name"); + String name; + if (os.startsWith("Mac")) { + name = "libenso_parser.dylib"; + } else if (os.startsWith("Windows")) { + name = "enso_parser.dll"; + } else { + name = "libenso_parser.so"; } - throw new IllegalStateException("Cannot load parser from " + root, e); - } - } - private static boolean searchFromDirToTop(Throwable chain, File root, String... names) { - while (root != null) { - var parser = root; - for (var e : names) { - parser = new File(parser, e); + var whereAmI = Parser.class.getProtectionDomain().getCodeSource().getLocation(); + File root; + try { + root = new File(whereAmI.toURI()).getParentFile(); + } catch (URISyntaxException ex) { + root = new File(".").getAbsoluteFile(); } try { - System.load(parser.getAbsolutePath()); - return true; - } catch (LinkageError err) { - while (chain.getCause() != null) { - chain = chain.getCause(); + var d = root; + File path = null; + while (d != null) { + path = new File(new File(d, "component"), name); + if (path.exists()) break; + d = d.getParentFile(); + } + if (d == null || path == null) { + throw new LinkageError("Cannot find parser in " + root); + } + System.load(path.getAbsolutePath()); + } catch (NullPointerException | IllegalArgumentException | LinkageError e) { + if (searchFromDirToTop(e, root, "target", "rust", "parser-jni", name)) { + return; + } + if (searchFromDirToTop( + e, new File(".").getAbsoluteFile(), "target", "rust", "parser-jni", name)) { + return; } - chain.initCause(err); - root = root.getParentFile(); + throw new IllegalStateException("Cannot load parser from " + root, e); } } - return false; - } - - private Parser() {} - private static native void freeBuffers(); + private static boolean searchFromDirToTop(Throwable chain, File root, String... names) { + while (root != null) { + var parser = root; + for (var e : names) { + parser = new File(parser, e); + } + try { + System.load(parser.getAbsolutePath()); + return true; + } catch (LinkageError err) { + while (chain.getCause() != null) { + chain = chain.getCause(); + } + chain.initCause(err); + root = root.getParentFile(); + } + } + return false; + } - private static native ByteBuffer parseTree(ByteBuffer input); + private long stateUnlessClosed; - private static native ByteBuffer parseTreeLazy(ByteBuffer input); + private Worker(long stateIn) { + stateUnlessClosed = stateIn; + } - private static native long isIdentOrOperator(ByteBuffer input); + public static Worker create() { + initializeLibraries(); + var state = allocState(); + return new Worker(state); + } - private static native long getLastInputBase(); + public long isIdentOrOperator(CharSequence input) { + byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); + ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); + inputBuf.put(inputBytes); - private static native long getMetadata(); + return Parser.isIdentOrOperator(inputBuf); + } - private static native String getWarningTemplate(int warningId); + private long getState() { + if (stateUnlessClosed != 0) { + return stateUnlessClosed; + } else { + throw new IllegalStateException("Parser used after close()"); + } + } - static native long getUuidHigh(long metadata, long codeOffset, long codeLength); + public ByteBuffer parseInputLazy(CharSequence input) { + var state = getState(); + byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); + ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); + inputBuf.put(inputBytes); + return parseTreeLazy(state, inputBuf); + } - static native long getUuidLow(long metadata, long codeOffset, long codeLength); + public Tree parse(CharSequence input) { + var state = getState(); + byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); + ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); + inputBuf.put(inputBytes); + var serializedTree = parseTree(state, inputBuf); + var base = getLastInputBase(state); + var metadata = getMetadata(state); + serializedTree.order(ByteOrder.LITTLE_ENDIAN); + var message = new Message(serializedTree, input, base, metadata); + try { + return Tree.deserialize(message); + } catch (BufferUnderflowException | IllegalArgumentException e) { + LoggerFactory.getLogger(this.getClass()) + .error("Unrecoverable parser failure for: {}", input, e); + throw e; + } + } - public static long isIdentOrOperator(CharSequence input) { - ensureInitialized(); - byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); - ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); - inputBuf.put(inputBytes); - return isIdentOrOperator(inputBuf); + @Override + public void close() { + freeState(stateUnlessClosed); + stateUnlessClosed = 0; + } } - public static ByteBuffer parseInputLazy(CharSequence input) { - ensureInitialized(); - byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); - ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); - inputBuf.put(inputBytes); - return parseTreeLazy(inputBuf); - } + /* JNI declarations */ - public static Tree parse(CharSequence input) { - ensureInitialized(); - byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); - ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); - inputBuf.put(inputBytes); - var serializedTree = parseTree(inputBuf); - var base = getLastInputBase(); - var metadata = getMetadata(); - serializedTree.order(ByteOrder.LITTLE_ENDIAN); - var message = new Message(serializedTree, input, base, metadata); - return Tree.deserialize(message); - } + private static native long allocState(); - public static String getWarningMessage(Warning warning) { - // `ensureInitialized` is unnecessary here because a `Warning` will only be constructed by - // parsing something. - return getWarningTemplate(warning.getId()); - } + private static native void freeState(long state); + + private static native ByteBuffer parseTree(long state, ByteBuffer input); + + private static native ByteBuffer parseTreeLazy(long state, ByteBuffer input); + + private static native long isIdentOrOperator(ByteBuffer input); + + private static native long getLastInputBase(long state); + + private static native long getMetadata(long state); + + private static native String getWarningTemplate(int warningId); + + private static native long getUuidHigh(long metadata, long codeOffset, long codeLength); + + private static native long getUuidLow(long metadata, long codeOffset, long codeLength); } diff --git a/lib/rust/parser/jni/src/lib.rs b/lib/rust/parser/jni/src/lib.rs index b54b3255eafa..0ee8580153b0 100644 --- a/lib/rust/parser/jni/src/lib.rs +++ b/lib/rust/parser/jni/src/lib.rs @@ -16,7 +16,6 @@ use jni::objects::JClass; use jni::sys::jobject; use jni::sys::jstring; use jni::JNIEnv; -use std::cell::UnsafeCell; @@ -32,15 +31,19 @@ static FAILED_SERIALIZE_AST: &str = "Failed to serialize AST to binary format."; /// /// # Safety /// -/// The contents of the returned buffer MUST not be accessed after another call to `parseInput`. +/// The state MUST be a value returned by `allocState` that has not been passed to `freeState`. +/// The input buffer contents MUST be valid UTF-8. +/// The contents of the returned buffer MUST not be accessed after another call to `parseInput`, or +/// a call to `freeState`. #[allow(unsafe_code)] #[no_mangle] pub extern "system" fn Java_org_enso_syntax2_Parser_parseTree( mut env: JNIEnv, _class: JClass, + state: u64, input: JByteBuffer, ) -> jobject { - let state = STATE.with(|state| unsafe { state.get().as_mut() }).unwrap(); + let state = unsafe { &mut *(state as usize as *mut State) }; let input = unsafe { decode_utf8_buffer(&env, &input) }; let mut code = input; let mut meta = None; @@ -73,16 +76,19 @@ pub extern "system" fn Java_org_enso_syntax2_Parser_parseTree( /// /// # Safety /// +/// The state MUST be a value returned by `allocState` that has not been passed to `freeState`. /// The input buffer contents MUST be valid UTF-8. -/// The contents of the returned buffer MUST not be accessed after another call to `parseInput`. +/// The contents of the returned buffer MUST not be accessed after another call to `parseInput`, or +/// a call to `freeState`. #[allow(unsafe_code)] #[no_mangle] pub extern "system" fn Java_org_enso_syntax2_Parser_parseTreeLazy( mut env: JNIEnv, _class: JClass, + state: u64, input: JByteBuffer, ) -> jobject { - let state = STATE.with(|state| unsafe { state.get().as_mut() }).unwrap(); + let state = unsafe { &mut *(state as usize as *mut State) }; let input = unsafe { decode_utf8_buffer(&env, &input) }; let tree = enso_parser::Parser::new().run(input); @@ -120,24 +126,37 @@ pub extern "system" fn Java_org_enso_syntax2_Parser_isIdentOrOperator( /// Return the `base` parameter to pass to the `Message` class along with the other output of the /// most recent call to `parseInput`. +/// +/// # Safety +/// +/// The input MUST have been returned by `allocState`, and MUST NOT have previously been passed to +/// `freeState`. #[allow(unsafe_code)] #[no_mangle] pub extern "system" fn Java_org_enso_syntax2_Parser_getLastInputBase( _env: JNIEnv, _class: JClass, + state: u64, ) -> u64 { - STATE.with(|state| unsafe { state.get().as_ref() }).unwrap().base + let state = unsafe { &mut *(state as usize as *mut State) }; + state.base } /// Return the metadata associated with the most recent parse. +/// +/// # Safety +/// +/// The input MUST have been returned by `allocState`, and MUST NOT have previously been passed to +/// `freeState`. #[allow(unsafe_code)] #[no_mangle] pub extern "system" fn Java_org_enso_syntax2_Parser_getMetadata( _env: JNIEnv, _class: JClass, + state: u64, ) -> u64 { - let metadata = &STATE.with(|state| unsafe { state.get().as_ref() }).unwrap().metadata; - match metadata { + let state = unsafe { &mut *(state as usize as *mut State) }; + match &state.metadata { Some(metadata) => { let metadata: *const _ = metadata; metadata as usize as u64 @@ -146,11 +165,34 @@ pub extern "system" fn Java_org_enso_syntax2_Parser_getMetadata( } } -/// Free the thread's parsing resources. +/// Allocate a new parser state object. The returned value should be passed to `freeState` when no +/// longer needed. +#[allow(unsafe_code, clippy::box_default)] +#[no_mangle] +pub extern "system" fn Java_org_enso_syntax2_Parser_allocState( + _env: JNIEnv, + _class: JClass, +) -> u64 { + Box::into_raw(Box::new(State::default())) as _ +} + +/// Free the resources owned by the state object. +/// +/// # Safety +/// +/// The input MUST have been returned by `allocState`, and MUST NOT have previously been passed to +/// `freeState`. #[allow(unsafe_code)] #[no_mangle] -pub extern "system" fn Java_org_enso_syntax2_Parser_freeBuffers(_env: JNIEnv, _class: JClass) { - *STATE.with(|state| unsafe { state.get().as_mut() }).unwrap() = Default::default(); +pub extern "system" fn Java_org_enso_syntax2_Parser_freeState( + _env: JNIEnv, + _class: JClass, + state: u64, +) { + if state != 0 { + let state = unsafe { Box::from_raw(state as usize as *mut State) }; + drop(state); + } } /// Returns the string template corresponding to the given warning ID. @@ -252,7 +294,3 @@ struct State { output: Vec, metadata: Option, } - -thread_local! { - static STATE: UnsafeCell = Default::default(); -} From b948b2e1ba0e55271c3ab51c85c00f729959c6f2 Mon Sep 17 00:00:00 2001 From: Kaz Date: Wed, 25 Sep 2024 13:18:22 -0700 Subject: [PATCH 07/16] Rust memory reclamation --- .../org/enso/syntax2/FinalizationManager.java | 53 ++++++++++++++ .../java/org/enso/syntax2/Parser.java | 73 +++++++++++-------- 2 files changed, 94 insertions(+), 32 deletions(-) create mode 100644 lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java new file mode 100644 index 000000000000..13eaf73255e0 --- /dev/null +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java @@ -0,0 +1,53 @@ +package org.enso.syntax2; + +import java.lang.ref.*; +import java.util.Collections; +import java.util.IdentityHashMap; +import java.util.Set; + +public final class FinalizationManager { + private final ReferenceQueue referenceQueue = new ReferenceQueue<>(); + private final Set finalizers = + Collections.synchronizedSet(Collections.newSetFromMap(new IdentityHashMap<>())); + + /** + * Associate the given callback with an object so that if the object is freed, the callback will + * be scheduled to be run. Note that the callback is not guaranteed to be executed, e.g. if the + * process exits first. + * + * @param referent Object whose lifetime determines when the finalizer will be run. + * @param finalize Callback to run after {@code referent} has been garbage-collected. + */ + public void attachFinalizer(T referent, Runnable finalize) { + finalizers.add(new FinalizationReference(referent, finalize, referenceQueue)); + } + + public FinalizerRunner createRunner() { + return this.new FinalizerRunner(); + } + + public class FinalizerRunner implements Runnable { + @Override + public void run() { + while (true) { + try { + var ref = referenceQueue.remove(); + if (ref instanceof FinalizationReference finalizationReference) { + finalizationReference.finalize.run(); + finalizers.remove(finalizationReference); + } + } catch (InterruptedException ignored) { + } + } + } + } + + private static class FinalizationReference extends PhantomReference { + final Runnable finalize; + + FinalizationReference(Object referent, Runnable finalize, ReferenceQueue q) { + super(referent, q); + this.finalize = finalize; + } + } +} diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java index b21ec091d149..905ee4759ec8 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java @@ -1,11 +1,14 @@ package org.enso.syntax2; import java.io.File; +import java.lang.ref.Reference; +import java.lang.ref.SoftReference; import java.net.URISyntaxException; import java.nio.BufferUnderflowException; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; +import java.util.UUID; import org.slf4j.LoggerFactory; public final class Parser { @@ -16,18 +19,18 @@ public static String getWarningMessage(Warning warning) { } public static long isIdentOrOperator(CharSequence input) { - return worker.get().isIdentOrOperator(input); + return getWorker().isIdentOrOperator(input); } public static ByteBuffer parseInputLazy(CharSequence input) { - return worker.get().parseInputLazy(input); + return getWorker().parseInputLazy(input); } public static Tree parse(CharSequence input) { - return worker.get().parse(input); + return getWorker().parse(input); } - public static java.util.UUID getUuid(long metadata, long nodeOffset, long nodeLength) { + public static UUID getUuid(long metadata, long nodeOffset, long nodeLength) { long high = getUuidHigh(metadata, nodeOffset, nodeLength); long low = getUuidLow(metadata, nodeOffset, nodeLength); if (high == 0 && low == 0) { @@ -36,14 +39,29 @@ public static java.util.UUID getUuid(long metadata, long nodeOffset, long nodeLe // The Nil UUID will never collide with a real UUID generated by any scheme. return null; } - return new java.util.UUID(high, low); + return new UUID(high, low); } /* Worker-thread state */ - private static final ThreadLocal worker = ThreadLocal.withInitial(Worker::create); + private static final FinalizationManager finalizationManager = new FinalizationManager(); + private static final Thread finalizationThread = new Thread(finalizationManager.createRunner()); - private static class Worker implements AutoCloseable { + private static Worker getWorker() { + var threadWorker = worker.get(); + if (threadWorker != null) { + var reusableWorker = threadWorker.get(); + if (reusableWorker != null) return reusableWorker; + } + var newWorker = Worker.create(); + finalizationManager.attachFinalizer(newWorker, newWorker.finalizer()); + worker.set(new SoftReference<>(newWorker)); + return newWorker; + } + + private static final ThreadLocal> worker = new ThreadLocal<>(); + + private static class Worker { private static void initializeLibraries() { try { System.loadLibrary("enso_parser"); @@ -112,19 +130,26 @@ private static boolean searchFromDirToTop(Throwable chain, File root, String... return false; } - private long stateUnlessClosed; + private final long state; + + private Worker() { + state = allocState(); + } - private Worker(long stateIn) { - stateUnlessClosed = stateIn; + /** + * @return A function that can be called to free the associated resources. The function *must + * not* be called more than once. + */ + Runnable finalizer() { + return () -> freeState(state); } - public static Worker create() { + static Worker create() { initializeLibraries(); - var state = allocState(); - return new Worker(state); + return new Worker(); } - public long isIdentOrOperator(CharSequence input) { + long isIdentOrOperator(CharSequence input) { byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); inputBuf.put(inputBytes); @@ -132,24 +157,14 @@ public long isIdentOrOperator(CharSequence input) { return Parser.isIdentOrOperator(inputBuf); } - private long getState() { - if (stateUnlessClosed != 0) { - return stateUnlessClosed; - } else { - throw new IllegalStateException("Parser used after close()"); - } - } - - public ByteBuffer parseInputLazy(CharSequence input) { - var state = getState(); + ByteBuffer parseInputLazy(CharSequence input) { byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); inputBuf.put(inputBytes); return parseTreeLazy(state, inputBuf); } - public Tree parse(CharSequence input) { - var state = getState(); + Tree parse(CharSequence input) { byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); inputBuf.put(inputBytes); @@ -166,12 +181,6 @@ public Tree parse(CharSequence input) { throw e; } } - - @Override - public void close() { - freeState(stateUnlessClosed); - stateUnlessClosed = 0; - } } /* JNI declarations */ From 8491ce1a33ecb4bb51d2f969e2f3df0bfafdd69b Mon Sep 17 00:00:00 2001 From: Kaz Date: Wed, 25 Sep 2024 21:52:53 -0700 Subject: [PATCH 08/16] Java 11 --- .../java/org/enso/syntax2/FinalizationManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java index 13eaf73255e0..028e53761f74 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java @@ -32,7 +32,8 @@ public void run() { while (true) { try { var ref = referenceQueue.remove(); - if (ref instanceof FinalizationReference finalizationReference) { + if (ref instanceof FinalizationReference) { + var finalizationReference = (FinalizationReference)ref; finalizationReference.finalize.run(); finalizers.remove(finalizationReference); } From 690812968d704de0e8071e9d63e5367b40a0f696 Mon Sep 17 00:00:00 2001 From: Kaz Date: Wed, 25 Sep 2024 21:57:41 -0700 Subject: [PATCH 09/16] fmt --- .../java/org/enso/syntax2/FinalizationManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java index 028e53761f74..b1f9cff14a44 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java @@ -33,7 +33,7 @@ public void run() { try { var ref = referenceQueue.remove(); if (ref instanceof FinalizationReference) { - var finalizationReference = (FinalizationReference)ref; + var finalizationReference = (FinalizationReference) ref; finalizationReference.finalize.run(); finalizers.remove(finalizationReference); } From 137abbc237c52965b55311ebcd3624ac5bf46fae Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 26 Sep 2024 07:48:10 +0200 Subject: [PATCH 10/16] Stress test to verify how EnsoParser behaves under multi threaded load --- .../core/EnsoParserMultiThreadedTest.java | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserMultiThreadedTest.java diff --git a/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserMultiThreadedTest.java b/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserMultiThreadedTest.java new file mode 100644 index 000000000000..9d434dd3b2bf --- /dev/null +++ b/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserMultiThreadedTest.java @@ -0,0 +1,52 @@ +package org.enso.compiler.core; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.ForkJoinPool; +import org.enso.compiler.core.ir.Module; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public class EnsoParserMultiThreadedTest { + private EnsoParser parser; + + @Before + public void initParser() throws Exception { + parser = new EnsoParser(); + } + + @After + public void closeParser() throws Exception { + parser.close(); + } + + @Test + public void stressLoadFromManyThreads() throws Exception { + List> cases = new ArrayList<>(); + final var testCases = 1000; + for (var i = 0; i < 2 * testCases; i++) { + var number = i % testCases; + var code = + """ + from Standard.Base import all + main = %n + """ + .replace("%n", "" + number); + cases.add( + () -> { + return parser.compile(code); + }); + } + + var results = ForkJoinPool.commonPool().invokeAll(cases); + + for (var i = 0; i < testCases; i++) { + var r1 = results.get(i).get(); + var r2 = results.get(testCases + i).get(); + + EnsoParserTest.assertIR("Run #" + i + " should produce identical IR", r1, r2); + } + } +} From be6f9d27e215757e41bdf30e82413a69c83038f4 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 26 Sep 2024 08:04:35 +0200 Subject: [PATCH 11/16] Adjusting the EnsoParserMultiThreadedTest to current API --- .../core/EnsoParserMultiThreadedTest.java | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserMultiThreadedTest.java b/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserMultiThreadedTest.java index 9d434dd3b2bf..46ed3a8e790b 100644 --- a/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserMultiThreadedTest.java +++ b/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserMultiThreadedTest.java @@ -5,23 +5,9 @@ import java.util.concurrent.Callable; import java.util.concurrent.ForkJoinPool; import org.enso.compiler.core.ir.Module; -import org.junit.After; -import org.junit.Before; import org.junit.Test; public class EnsoParserMultiThreadedTest { - private EnsoParser parser; - - @Before - public void initParser() throws Exception { - parser = new EnsoParser(); - } - - @After - public void closeParser() throws Exception { - parser.close(); - } - @Test public void stressLoadFromManyThreads() throws Exception { List> cases = new ArrayList<>(); @@ -36,7 +22,7 @@ public void stressLoadFromManyThreads() throws Exception { .replace("%n", "" + number); cases.add( () -> { - return parser.compile(code); + return EnsoParser.compile(code); }); } From 5358f327194c37d8be5aa83df079617d752a8604 Mon Sep 17 00:00:00 2001 From: Kaz Date: Thu, 26 Sep 2024 10:09:56 -0700 Subject: [PATCH 12/16] Expunge parsers on allocate or shutdown --- .../org/enso/compiler/core/EnsoParser.java | 9 ++ .../enso/interpreter/runtime/EnsoContext.java | 2 + .../org/enso/syntax2/FinalizationManager.java | 42 ++++--- .../java/org/enso/syntax2/Parser.java | 106 +++++++++++------- lib/rust/parser/jni/src/lib.rs | 9 ++ 5 files changed, 111 insertions(+), 57 deletions(-) diff --git a/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java index 4dcd913618bf..04d934d3b785 100644 --- a/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java +++ b/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java @@ -27,4 +27,13 @@ public static scala.Option compileInline(CharSequence src) { var tree = Parser.parse(src); return TreeToIr.MODULE.translateInline(tree); } + + /** + * Free retained state of all parsers. Parser buffers are retained per-thread for reuse; this + * function drops those reusable buffers. If the parser is used again after this call, it will + * allocate new buffers as needed. + */ + public static void freeAll() { + Parser.freeAll(); + } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java index dc4dc3f4a465..46fcb3a2df2c 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java @@ -42,6 +42,7 @@ import org.enso.common.LanguageInfo; import org.enso.common.RuntimeOptions; import org.enso.compiler.Compiler; +import org.enso.compiler.core.EnsoParser; import org.enso.compiler.data.CompilerConfig; import org.enso.compiler.dump.IRDumper; import org.enso.distribution.DistributionManager; @@ -296,6 +297,7 @@ public void shutdown() { guestJava = null; topScope = null; hostClassLoader.close(); + EnsoParser.freeAll(); } private boolean shouldAssertionsBeEnabled() { diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java index b1f9cff14a44..21e3ae026bb9 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/FinalizationManager.java @@ -5,7 +5,7 @@ import java.util.IdentityHashMap; import java.util.Set; -public final class FinalizationManager { +final class FinalizationManager { private final ReferenceQueue referenceQueue = new ReferenceQueue<>(); private final Set finalizers = Collections.synchronizedSet(Collections.newSetFromMap(new IdentityHashMap<>())); @@ -18,28 +18,34 @@ public final class FinalizationManager { * @param referent Object whose lifetime determines when the finalizer will be run. * @param finalize Callback to run after {@code referent} has been garbage-collected. */ - public void attachFinalizer(T referent, Runnable finalize) { + void attachFinalizer(T referent, Runnable finalize) { finalizers.add(new FinalizationReference(referent, finalize, referenceQueue)); } - public FinalizerRunner createRunner() { - return this.new FinalizerRunner(); + void runPendingFinalizers() { + var ref = referenceQueue.poll(); + while (ref != null) { + runFinalizer(ref); + ref = referenceQueue.poll(); + } + } + + /** + * @return The finalizers that have been registered, and have not yet been run. + *

This does not de-register the finalizers; they will still be run as usual after their + * reference objects become unreachable. + */ + Iterable getRegisteredFinalizers() { + synchronized (finalizers) { + return finalizers.stream().map(ref -> ref.finalize).toList(); + } } - public class FinalizerRunner implements Runnable { - @Override - public void run() { - while (true) { - try { - var ref = referenceQueue.remove(); - if (ref instanceof FinalizationReference) { - var finalizationReference = (FinalizationReference) ref; - finalizationReference.finalize.run(); - finalizers.remove(finalizationReference); - } - } catch (InterruptedException ignored) { - } - } + private void runFinalizer(Reference ref) { + if (ref instanceof FinalizationReference) { + var finalizationReference = (FinalizationReference) ref; + finalizationReference.finalize.run(); + finalizers.remove(finalizationReference); } } diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java index 905ee4759ec8..39f5e1b61e23 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java @@ -1,14 +1,14 @@ package org.enso.syntax2; import java.io.File; -import java.lang.ref.Reference; -import java.lang.ref.SoftReference; import java.net.URISyntaxException; import java.nio.BufferUnderflowException; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; import java.util.UUID; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Function; import org.slf4j.LoggerFactory; public final class Parser { @@ -45,21 +45,23 @@ public static UUID getUuid(long metadata, long nodeOffset, long nodeLength) { /* Worker-thread state */ private static final FinalizationManager finalizationManager = new FinalizationManager(); - private static final Thread finalizationThread = new Thread(finalizationManager.createRunner()); private static Worker getWorker() { - var threadWorker = worker.get(); - if (threadWorker != null) { - var reusableWorker = threadWorker.get(); - if (reusableWorker != null) return reusableWorker; + var worker = threadWorker.get(); + if (worker == null) { + finalizationManager.runPendingFinalizers(); + worker = Worker.create(); + finalizationManager.attachFinalizer(worker, worker.finalizer()); + threadWorker.set(worker); } - var newWorker = Worker.create(); - finalizationManager.attachFinalizer(newWorker, newWorker.finalizer()); - worker.set(new SoftReference<>(newWorker)); - return newWorker; + return worker; } - private static final ThreadLocal> worker = new ThreadLocal<>(); + private static final ThreadLocal threadWorker = new ThreadLocal<>(); + + public static void freeAll() { + for (var finalizer : finalizationManager.getRegisteredFinalizers()) finalizer.run(); + } private static class Worker { private static void initializeLibraries() { @@ -130,18 +132,25 @@ private static boolean searchFromDirToTop(Throwable chain, File root, String... return false; } - private final long state; + private final AtomicLong state = new AtomicLong(0); + + private Worker() {} - private Worker() { - state = allocState(); + private static class Finalizer implements Runnable { + private final AtomicLong state; + + private Finalizer(AtomicLong state) { + this.state = state; + } + + @Override + public void run() { + freeState(state.getAndSet(0)); + } } - /** - * @return A function that can be called to free the associated resources. The function *must - * not* be called more than once. - */ Runnable finalizer() { - return () -> freeState(state); + return new Finalizer(state); } static Worker create() { @@ -149,6 +158,19 @@ static Worker create() { return new Worker(); } + private T withState(Function stateConsumer) { + // Take the state for the duration of the operation so that it can't be freed by another + // thread. + var privateState = state.getAndSet(0); + if (privateState == 0) privateState = allocState(); + var result = stateConsumer.apply(privateState); + // We don't need to check the value before setting here: A state may be freed by another + // thread, but is only allocated by its associated `Worker`, so after taking it above, the + // shared value remains 0 until we restore it. + state.set(privateState); + return result; + } + long isIdentOrOperator(CharSequence input) { byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); @@ -158,28 +180,34 @@ long isIdentOrOperator(CharSequence input) { } ByteBuffer parseInputLazy(CharSequence input) { - byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); - ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); - inputBuf.put(inputBytes); - return parseTreeLazy(state, inputBuf); + return withState( + state -> { + byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); + ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); + inputBuf.put(inputBytes); + return parseTreeLazy(state, inputBuf); + }); } Tree parse(CharSequence input) { - byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); - ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); - inputBuf.put(inputBytes); - var serializedTree = parseTree(state, inputBuf); - var base = getLastInputBase(state); - var metadata = getMetadata(state); - serializedTree.order(ByteOrder.LITTLE_ENDIAN); - var message = new Message(serializedTree, input, base, metadata); - try { - return Tree.deserialize(message); - } catch (BufferUnderflowException | IllegalArgumentException e) { - LoggerFactory.getLogger(this.getClass()) - .error("Unrecoverable parser failure for: {}", input, e); - throw e; - } + return withState( + state -> { + byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); + ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); + inputBuf.put(inputBytes); + var serializedTree = parseTree(state, inputBuf); + var base = getLastInputBase(state); + var metadata = getMetadata(state); + serializedTree.order(ByteOrder.LITTLE_ENDIAN); + var message = new Message(serializedTree, input, base, metadata); + try { + return Tree.deserialize(message); + } catch (BufferUnderflowException | IllegalArgumentException e) { + LoggerFactory.getLogger(this.getClass()) + .error("Unrecoverable parser failure for: {}", input, e); + throw e; + } + }); } } diff --git a/lib/rust/parser/jni/src/lib.rs b/lib/rust/parser/jni/src/lib.rs index 0ee8580153b0..4c5e2a504de8 100644 --- a/lib/rust/parser/jni/src/lib.rs +++ b/lib/rust/parser/jni/src/lib.rs @@ -294,3 +294,12 @@ struct State { output: Vec, metadata: Option, } + +mod static_trait_check { + fn assert_send() {} + fn assert_state_send() { + // Require `State` to be `Send`-safe so that in Java it can be deallocated (or potentially + // reused) by any thread. + assert_send::() + } +} From f81679ad7fe47d1bdc16ad7d8a997f013a8175a8 Mon Sep 17 00:00:00 2001 From: Kaz Date: Thu, 26 Sep 2024 13:32:54 -0700 Subject: [PATCH 13/16] Fix NI --- .../generate-java/java/module-info.java | 1 + .../java/org/enso/syntax2/Parser.java | 39 +++++++++++-------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/lib/rust/parser/generate-java/java/module-info.java b/lib/rust/parser/generate-java/java/module-info.java index 0d64592703e5..2f5a634d63d9 100644 --- a/lib/rust/parser/generate-java/java/module-info.java +++ b/lib/rust/parser/generate-java/java/module-info.java @@ -1,5 +1,6 @@ module org.enso.syntax { requires org.slf4j; + requires org.graalvm.nativeimage; exports org.enso.syntax2; } diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java index 39f5e1b61e23..b03efe234552 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java @@ -9,6 +9,7 @@ import java.util.UUID; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Function; +import org.graalvm.nativeimage.ImageInfo; import org.slf4j.LoggerFactory; public final class Parser { @@ -51,8 +52,12 @@ private static Worker getWorker() { if (worker == null) { finalizationManager.runPendingFinalizers(); worker = Worker.create(); - finalizationManager.attachFinalizer(worker, worker.finalizer()); - threadWorker.set(worker); + if (!ImageInfo.inImageBuildtimeCode()) { + // At build-time, we eagerly free parser buffers; runtime should start out with an empty + // `finalizationManager`. + finalizationManager.attachFinalizer(worker, worker.finalizer()); + threadWorker.set(worker); + } } return worker; } @@ -164,10 +169,15 @@ private T withState(Function stateConsumer) { var privateState = state.getAndSet(0); if (privateState == 0) privateState = allocState(); var result = stateConsumer.apply(privateState); - // We don't need to check the value before setting here: A state may be freed by another - // thread, but is only allocated by its associated `Worker`, so after taking it above, the - // shared value remains 0 until we restore it. - state.set(privateState); + if (ImageInfo.inImageBuildtimeCode()) { + // At build-time, eagerly free buffers. We don't want them included in the heap snapshot! + freeState(privateState); + } else { + // We don't need to check the value before setting here: A state may be freed by another + // thread, but is only allocated by its associated `Worker`, so after taking it above, the + // shared value remains 0 until we restore it. + state.set(privateState); + } return result; } @@ -180,21 +190,18 @@ long isIdentOrOperator(CharSequence input) { } ByteBuffer parseInputLazy(CharSequence input) { - return withState( - state -> { - byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); - ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); - inputBuf.put(inputBytes); - return parseTreeLazy(state, inputBuf); - }); + byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); + ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); + inputBuf.put(inputBytes); + return withState(state -> parseTreeLazy(state, inputBuf)); } Tree parse(CharSequence input) { + byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); + ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); + inputBuf.put(inputBytes); return withState( state -> { - byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); - ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); - inputBuf.put(inputBytes); var serializedTree = parseTree(state, inputBuf); var base = getLastInputBase(state); var metadata = getMetadata(state); From 6edac90836082b4cba575e587b237cdad0113eb8 Mon Sep 17 00:00:00 2001 From: Kaz Date: Thu, 26 Sep 2024 15:55:45 -0700 Subject: [PATCH 14/16] Fix flaky test --- .../RuntimeVisualizationsTest.scala | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala index 76d55ef788cb..55761fefc2ca 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala @@ -1351,6 +1351,17 @@ class RuntimeVisualizationsTest extends AnyFlatSpec with Matchers { ) ) + val attachVisualizationResponses = + context.receiveNIgnoreExpressionUpdates(2) + + attachVisualizationResponses.filter( + _.payload.isInstanceOf[Api.VisualizationAttached] + ) shouldEqual List( + Api.Response(requestId, Api.VisualizationAttached()), + Api.Response(requestId, Api.VisualizationAttached()) + ) + + // Modify the file context.send( Api.Request( Api.EditFileNotification( @@ -1367,23 +1378,17 @@ class RuntimeVisualizationsTest extends AnyFlatSpec with Matchers { ) ) - val responses = - context.receiveNIgnoreExpressionUpdates(7) + val editFileResponses = + context.receiveNIgnoreExpressionUpdates(5) - responses should contain allOf ( - Api.Response(requestId, Api.VisualizationAttached()), + editFileResponses should contain( context.executionComplete(contextId) ) - responses.filter( - _.payload.isInstanceOf[Api.VisualizationAttached] - ) shouldEqual List( - Api.Response(requestId, Api.VisualizationAttached()), - Api.Response(requestId, Api.VisualizationAttached()) - ) - val visualizationUpdatesResponses = - responses.filter(_.payload.isInstanceOf[Api.VisualizationUpdate]) + (attachVisualizationResponses ::: editFileResponses).filter( + _.payload.isInstanceOf[Api.VisualizationUpdate] + ) val expectedExpressionId = context.Main.idMainX val visualizationUpdates = visualizationUpdatesResponses.map( _.payload.asInstanceOf[Api.VisualizationUpdate] From 732d1f02bc70412e33a3fcbf1cb2f5317317d9da Mon Sep 17 00:00:00 2001 From: Kaz Date: Thu, 26 Sep 2024 18:45:21 -0700 Subject: [PATCH 15/16] Fix test --- .../test/instrument/RuntimeVisualizationsTest.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala index 55761fefc2ca..f736c898627b 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala @@ -1352,7 +1352,7 @@ class RuntimeVisualizationsTest extends AnyFlatSpec with Matchers { ) val attachVisualizationResponses = - context.receiveNIgnoreExpressionUpdates(2) + context.receiveNIgnoreExpressionUpdates(4) attachVisualizationResponses.filter( _.payload.isInstanceOf[Api.VisualizationAttached] @@ -1379,7 +1379,7 @@ class RuntimeVisualizationsTest extends AnyFlatSpec with Matchers { ) val editFileResponses = - context.receiveNIgnoreExpressionUpdates(5) + context.receiveNIgnoreExpressionUpdates(3) editFileResponses should contain( context.executionComplete(contextId) From d22c8afc7d082887d5f856215b256bac8922369d Mon Sep 17 00:00:00 2001 From: Kaz Date: Fri, 27 Sep 2024 06:25:00 -0700 Subject: [PATCH 16/16] Review --- .../java/org/enso/syntax2/Parser.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java index b03efe234552..748a081d94c7 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java @@ -48,21 +48,23 @@ public static UUID getUuid(long metadata, long nodeOffset, long nodeLength) { private static final FinalizationManager finalizationManager = new FinalizationManager(); private static Worker getWorker() { - var worker = threadWorker.get(); - if (worker == null) { - finalizationManager.runPendingFinalizers(); - worker = Worker.create(); - if (!ImageInfo.inImageBuildtimeCode()) { - // At build-time, we eagerly free parser buffers; runtime should start out with an empty - // `finalizationManager`. - finalizationManager.attachFinalizer(worker, worker.finalizer()); - threadWorker.set(worker); - } + finalizationManager.runPendingFinalizers(); + return threadWorker.get(); + } + + private static Worker createWorker() { + var worker = Worker.create(); + if (!ImageInfo.inImageBuildtimeCode()) { + // At build-time, we eagerly free parser buffers; runtime should start out with an empty + // `finalizationManager`. + finalizationManager.attachFinalizer(worker, worker.finalizer()); + threadWorker.set(worker); } return worker; } - private static final ThreadLocal threadWorker = new ThreadLocal<>(); + private static final ThreadLocal threadWorker = + ThreadLocal.withInitial(Parser::createWorker); public static void freeAll() { for (var finalizer : finalizationManager.getRegisteredFinalizers()) finalizer.run();