From eefc08efb0af197db18c4d3c2d2e5ba108c62a27 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 6 Feb 2024 16:06:49 +0100 Subject: [PATCH 1/3] Remove ParserCache * Unfortunately it cannot work anymore because we need to find libyarpbindings.so to use the Prism parser but there is no way to locate it for the case of embedding truffleruby as a native image, while running in a static initializer. For example when using https://github.com/graalvm/polyglot-embedding-demo and building via Maven. In that case the resources/ direcotry is only created after the image is built, and also Class#getResourceAsStream() does not seem to work + would be messy. * Going forward we likely want to cache the serialized bytes of core and maybe stdlib sources, on the filesystem or in a jar, by running some java process to do so while building TruffleRuby. * This cache was only useful for native and when pre-initialization is not used, which is a narrow use case. --- .../org.graalvm.ruby/resource-config.json | 8 ++ .../java/org/truffleruby/aot/ParserCache.java | 74 ------------------- .../org/truffleruby/core/CoreLibrary.java | 9 +-- .../language/loader/ResourceLoader.java | 10 +-- .../parser/YARPTranslatorDriver.java | 38 ++-------- 5 files changed, 19 insertions(+), 120 deletions(-) create mode 100644 src/main/java/META-INF/native-image/org.graalvm.ruby/resource-config.json delete mode 100644 src/main/java/org/truffleruby/aot/ParserCache.java diff --git a/src/main/java/META-INF/native-image/org.graalvm.ruby/resource-config.json b/src/main/java/META-INF/native-image/org.graalvm.ruby/resource-config.json new file mode 100644 index 000000000000..ce922647e25d --- /dev/null +++ b/src/main/java/META-INF/native-image/org.graalvm.ruby/resource-config.json @@ -0,0 +1,8 @@ +{ + "resources": { + "includes": [ + { "pattern": "^truffleruby/core/.+\\.rb$" }, + { "pattern": "^truffleruby/post-boot/.+\\.rb$" } + ] + } +} diff --git a/src/main/java/org/truffleruby/aot/ParserCache.java b/src/main/java/org/truffleruby/aot/ParserCache.java deleted file mode 100644 index 8f44e4d851a9..000000000000 --- a/src/main/java/org/truffleruby/aot/ParserCache.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright (c) 2017, 2024 Oracle and/or its affiliates. All rights reserved. This - * code is released under a tri EPL/GPL/LGPL license. You can use it, - * redistribute it and/or modify it under the terms of the: - * - * Eclipse Public License version 2.0, or - * GNU General Public License version 2, or - * GNU Lesser General Public License version 2.1. - */ -package org.truffleruby.aot; - -import java.io.IOException; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; - -import com.oracle.truffle.api.CompilerDirectives; -import com.oracle.truffle.api.source.Source; -import org.graalvm.collections.Pair; -import org.prism.ParseResult; -import org.truffleruby.RubyLanguage; -import org.truffleruby.core.CoreLibrary; -import org.truffleruby.language.loader.ResourceLoader; -import org.truffleruby.parser.RubySource; -import org.truffleruby.parser.YARPTranslatorDriver; -import org.truffleruby.shared.options.OptionsCatalog; - -import com.oracle.truffle.api.TruffleOptions; - -public final class ParserCache { - - public static final Map> INSTANCE; - - static { - if (TruffleOptions.AOT) { - final String defaultCoreLibraryPath = OptionsCatalog.CORE_LOAD_PATH_KEY.getDefaultValue(); - final Map> cache = new HashMap<>(); - - for (String coreFile : CoreLibrary.CORE_FILES) { - // intern() to improve footprint - final String path = (defaultCoreLibraryPath + coreFile).intern(); - final RubySource source = loadSource(path); - cache.put(path, parse(source)); - } - - INSTANCE = cache; - } else { - INSTANCE = null; - } - } - - private static RubySource loadSource(String feature) { - try { - final Source source = ResourceLoader - .loadResource(feature, OptionsCatalog.CORE_AS_INTERNAL_KEY.getDefaultValue()); - return new RubySource(source, feature); - } catch (IOException e) { - throw CompilerDirectives.shouldNotReachHere(e); - } - } - - private static Pair parse(RubySource source) { - var yarpSource = YARPTranslatorDriver.createYARPSource(source.getBytes()); - var sourcePath = RubyLanguage.getCorePath(source.getSource()).intern(); - - var parseResult = YARPTranslatorDriver.parseToYARPAST(source, sourcePath, yarpSource, Collections.emptyList(), - false); - - YARPTranslatorDriver.handleWarningsErrorsNoContext(parseResult, sourcePath, yarpSource); - - return Pair.create(parseResult, source.getSource()); - } - -} diff --git a/src/main/java/org/truffleruby/core/CoreLibrary.java b/src/main/java/org/truffleruby/core/CoreLibrary.java index 7d5558fa949b..a708a4077647 100644 --- a/src/main/java/org/truffleruby/core/CoreLibrary.java +++ b/src/main/java/org/truffleruby/core/CoreLibrary.java @@ -29,7 +29,6 @@ import org.truffleruby.RubyLanguage; import org.truffleruby.annotations.CoreMethod; import org.truffleruby.annotations.SuppressFBWarnings; -import org.truffleruby.aot.ParserCache; import org.truffleruby.builtins.BuiltinsClasses; import org.truffleruby.builtins.CoreMethodNodeManager; import org.truffleruby.core.array.RubyArray; @@ -73,7 +72,6 @@ import com.oracle.truffle.api.CompilerDirectives.CompilationFinal; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.Truffle; -import com.oracle.truffle.api.TruffleOptions; import com.oracle.truffle.api.object.DynamicObjectLibrary; import com.oracle.truffle.api.source.Source; import com.oracle.truffle.api.source.SourceSection; @@ -785,12 +783,7 @@ public void loadRubyCoreLibraryAndPostBoot() { public RubySource loadCoreFileSource(String path) throws IOException { if (path.startsWith(RubyLanguage.RESOURCE_SCHEME)) { - if (TruffleOptions.AOT || ParserCache.INSTANCE != null) { - Source source = ParserCache.INSTANCE.get(path).getRight(); - return new RubySource(source, path); - } else { - return new RubySource(ResourceLoader.loadResource(path, language.options.CORE_AS_INTERNAL), path); - } + return new RubySource(ResourceLoader.loadResource(path, language.options.CORE_AS_INTERNAL), path); } else { final FileLoader fileLoader = new FileLoader(context, language); return fileLoader.loadFile(path); diff --git a/src/main/java/org/truffleruby/language/loader/ResourceLoader.java b/src/main/java/org/truffleruby/language/loader/ResourceLoader.java index c7ee3a00b6e7..75a3b6678061 100644 --- a/src/main/java/org/truffleruby/language/loader/ResourceLoader.java +++ b/src/main/java/org/truffleruby/language/loader/ResourceLoader.java @@ -14,13 +14,10 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Locale; import org.truffleruby.RubyContext; import org.truffleruby.RubyLanguage; -import org.truffleruby.core.string.StringUtils; import org.truffleruby.shared.TruffleRuby; import com.oracle.truffle.api.source.Source; @@ -38,12 +35,11 @@ public static Source loadResource(String path, boolean internal) throws IOExcept } final Class relativeClass = RubyContext.class; - final Path relativePath = Paths.get(path.substring(RubyLanguage.RESOURCE_SCHEME.length())); - final String normalizedPath = StringUtils.replace(relativePath.normalize().toString(), '\\', '/'); - final InputStream stream = relativeClass.getResourceAsStream(normalizedPath); + final String resourcePath = path.substring(RubyLanguage.RESOURCE_SCHEME.length()); + final InputStream stream = relativeClass.getResourceAsStream(resourcePath); if (stream == null) { - throw new FileNotFoundException(path); + throw new FileNotFoundException(resourcePath); } final Source source; diff --git a/src/main/java/org/truffleruby/parser/YARPTranslatorDriver.java b/src/main/java/org/truffleruby/parser/YARPTranslatorDriver.java index 29dfcfa932ac..f33fb0ba0470 100644 --- a/src/main/java/org/truffleruby/parser/YARPTranslatorDriver.java +++ b/src/main/java/org/truffleruby/parser/YARPTranslatorDriver.java @@ -48,7 +48,6 @@ import org.truffleruby.RubyContext; import org.truffleruby.RubyLanguage; import org.truffleruby.annotations.Split; -import org.truffleruby.aot.ParserCache; import org.truffleruby.core.CoreLibrary; import org.truffleruby.core.DummyNode; import org.truffleruby.core.binding.BindingNodes; @@ -158,22 +157,13 @@ public RootCallTarget parse(RubySource rubySource, ParserContext parserContext, final String sourcePath = rubySource.getSourcePath(language).intern(); - // Only use the cache while loading top-level core library files, as eval() later could use - // the same Source name but should not use the cache. For instance, - // TOPLEVEL_BINDING.eval("self") would use the cache which is wrong. - final ParseResult parseResult; - if (ParserCache.INSTANCE != null && parserContext == ParserContext.TOP_LEVEL && - ParserCache.INSTANCE.containsKey(source.getName())) { - parseResult = ParserCache.INSTANCE.get(source.getName()).getLeft(); - } else { - printParseTranslateExecuteMetric("before-parsing", context, source); - parseResult = context.getMetricsProfiler().callWithMetrics( - "parsing", - source.getName(), - () -> parseToYARPAST(rubySource, sourcePath, yarpSource, localsInScopes, - language.options.FROZEN_STRING_LITERALS)); - printParseTranslateExecuteMetric("after-parsing", context, source); - } + printParseTranslateExecuteMetric("before-parsing", context, source); + final ParseResult parseResult = context.getMetricsProfiler().callWithMetrics( + "parsing", + source.getName(), + () -> parseToYARPAST(rubySource, sourcePath, yarpSource, localsInScopes, + language.options.FROZEN_STRING_LITERALS)); + printParseTranslateExecuteMetric("after-parsing", context, source); handleWarningsErrorsPrimitives(context, parseResult, rubySource, sourcePath, parseEnvironment, rubyWarnings); @@ -458,20 +448,6 @@ public static void handleWarningsErrorsPrimitives(RubyContext context, ParseResu parseEnvironment.allowTruffleRubyPrimitives = allowTruffleRubyPrimitives; } - public static void handleWarningsErrorsNoContext(ParseResult parseResult, String sourcePath, - Nodes.Source yarpSource) { - if (parseResult.errors.length > 0) { - var error = parseResult.errors[0]; - throw CompilerDirectives.shouldNotReachHere("Parse error in " + - sourcePath + ":" + yarpSource.line(error.location.startOffset) + ": " + error.message); - } - - for (var warning : parseResult.warnings) { - throw CompilerDirectives.shouldNotReachHere("Warning in " + - sourcePath + ":" + yarpSource.line(warning.location.startOffset) + ": " + warning.message); - } - } - public static Nodes.Source createYARPSource(byte[] sourceBytes) { return new Nodes.Source(sourceBytes); } From 5e59a45746b6fecd12c0a3f9ee4e0ae611e69ac0 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 6 Feb 2024 17:09:32 +0100 Subject: [PATCH 2/3] Read core library files as bytes in ResourceLoader * Reading as char[] is useless and extra work. --- .../org/truffleruby/core/CoreLibrary.java | 2 +- .../language/loader/ResourceLoader.java | 34 +++++++++++-------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/truffleruby/core/CoreLibrary.java b/src/main/java/org/truffleruby/core/CoreLibrary.java index a708a4077647..cd34556b0948 100644 --- a/src/main/java/org/truffleruby/core/CoreLibrary.java +++ b/src/main/java/org/truffleruby/core/CoreLibrary.java @@ -783,7 +783,7 @@ public void loadRubyCoreLibraryAndPostBoot() { public RubySource loadCoreFileSource(String path) throws IOException { if (path.startsWith(RubyLanguage.RESOURCE_SCHEME)) { - return new RubySource(ResourceLoader.loadResource(path, language.options.CORE_AS_INTERNAL), path); + return ResourceLoader.loadResource(path, language.options.CORE_AS_INTERNAL); } else { final FileLoader fileLoader = new FileLoader(context, language); return fileLoader.loadFile(path); diff --git a/src/main/java/org/truffleruby/language/loader/ResourceLoader.java b/src/main/java/org/truffleruby/language/loader/ResourceLoader.java index 75a3b6678061..3d292f391e2c 100644 --- a/src/main/java/org/truffleruby/language/loader/ResourceLoader.java +++ b/src/main/java/org/truffleruby/language/loader/ResourceLoader.java @@ -12,12 +12,12 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.io.InputStreamReader; -import java.nio.charset.StandardCharsets; import java.util.Locale; -import org.truffleruby.RubyContext; import org.truffleruby.RubyLanguage; +import org.truffleruby.core.encoding.Encodings; +import org.truffleruby.parser.MagicCommentParser; +import org.truffleruby.parser.RubySource; import org.truffleruby.shared.TruffleRuby; import com.oracle.truffle.api.source.Source; @@ -27,29 +27,35 @@ */ public abstract class ResourceLoader { - public static Source loadResource(String path, boolean internal) throws IOException { + public static RubySource loadResource(String path, boolean internal) throws IOException { assert path.startsWith(RubyLanguage.RESOURCE_SCHEME); if (!path.toLowerCase(Locale.ENGLISH).endsWith(".rb")) { throw new FileNotFoundException(path); } - final Class relativeClass = RubyContext.class; final String resourcePath = path.substring(RubyLanguage.RESOURCE_SCHEME.length()); - final InputStream stream = relativeClass.getResourceAsStream(resourcePath); - if (stream == null) { - throw new FileNotFoundException(resourcePath); + final byte[] sourceBytes; + try (InputStream stream = ResourceLoader.class.getResourceAsStream(resourcePath)) { + if (stream == null) { + throw new FileNotFoundException(resourcePath); + } + + sourceBytes = stream.readAllBytes(); } - final Source source; - // We guarantee that we only put UTF-8 source files into resources - try (final InputStreamReader reader = new InputStreamReader(stream, StandardCharsets.UTF_8)) { - source = Source.newBuilder(TruffleRuby.LANGUAGE_ID, reader, path).internal(internal).build(); - } + var sourceTString = MagicCommentParser.createSourceTStringBasedOnMagicEncodingComment(sourceBytes, + Encodings.UTF_8); + + Source source = Source + .newBuilder(TruffleRuby.LANGUAGE_ID, new ByteBasedCharSequence(sourceTString), path) + .mimeType(RubyLanguage.getMimeType(false)) + .internal(internal) + .build(); - return source; + return new RubySource(source, path, sourceTString); } } From eaf41582d8f16a04fbae82e6afab021d6a6bd130 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 6 Feb 2024 17:26:12 +0100 Subject: [PATCH 3/3] Disable ruby-test-mri-linux-aarch64 for now, it times out too often --- ci.jsonnet | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci.jsonnet b/ci.jsonnet index f7cc6c9d4be5..215a4b6b0978 100644 --- a/ci.jsonnet +++ b/ci.jsonnet @@ -528,7 +528,7 @@ local composition_environment = utils.add_inclusion_tracking(part_definitions, " "ruby-test-fast-linux-amd64": $.platform.linux + $.jdk.stable + $.env.jvm + gate + $.run.test_fast + { timelimit: "45:00" }, # To catch missing slow tags "ruby-test-mri-asserts": $.platform.linux + $.jdk.stable + $.env.jvm + gate + $.run.test_mri_fast + { timelimit: "01:15:00" }, "ruby-test-mri-linux-amd64": $.platform.linux + $.jdk.stable + $.env.native + gate + $.run.test_mri + { timelimit: "01:20:00" }, - "ruby-test-mri-linux-aarch64": $.platform.linux_aarch64 + $.jdk.stable + $.env.native + gate + $.run.test_mri + { timelimit: "01:30:00" }, + # "ruby-test-mri-linux-aarch64": $.platform.linux_aarch64 + $.jdk.stable + $.env.native + gate + $.run.test_mri + { timelimit: "01:30:00" }, # GR-51361 "ruby-test-mri-darwin-amd64": $.platform.darwin_amd64 + $.jdk.stable + $.env.native + gate + $.run.test_mri + { timelimit: "01:30:00" }, "ruby-test-mri-darwin-aarch64": $.platform.darwin_aarch64 + $.jdk.stable + $.env.native + gate + $.run.test_mri + { timelimit: "01:30:00" }, "ruby-test-integration-linux-amd64": $.platform.linux + $.jdk.stable + $.env.jvm + gate + $.run.test_integration,