Merge pull request #3998 from eregon/no-source-bytes-for-java

Do not pass source bytes to Loader.java at all and mark newlines for lazy DefNodes

Author: eregon

java-wasm/src/main/java/org/jruby/parser/prism/wasm/Prism.java

@@ -83,7 +83,7 @@ public byte[] serialize(byte[] packedOptions, byte[] sourceBytes, int sourceLeng
     public ParseResult serializeParse(byte[] packedOptions, String source) {
         var sourceBytes = source.getBytes(StandardCharsets.ISO_8859_1);
         byte[] result = serialize(packedOptions, sourceBytes, sourceBytes.length);
-        return Loader.load(result, sourceBytes);
+        return Loader.load(result);
     }
 
     @Override

java/org/ruby_lang/prism/MarkNewlinesVisitor.java

@@ -6,9 +6,9 @@ final class MarkNewlinesVisitor extends AbstractNodeVisitor<Void> {
     private final Nodes.Source source;
     private boolean[] newlineMarked;
 
-    MarkNewlinesVisitor(Nodes.Source source, boolean[] newlineMarked) {
+    MarkNewlinesVisitor(Nodes.Source source) {
         this.source = source;
-        this.newlineMarked = newlineMarked;
+        this.newlineMarked = new boolean[1 + source.getLineCount()];
     }
 
     @Override

rakelib/serialization.rake

@@ -25,8 +25,7 @@ task "test:java_loader:internal" => :compile do
     puts
     puts path
     serialized = Prism.dump_file(path)
-    source_bytes = File.binread(path).unpack('c*')
-    parse_result = org.ruby_lang.prism.Loader.load(serialized.unpack('c*'), source_bytes)
+    parse_result = org.ruby_lang.prism.Loader.load(serialized.unpack('c*'))
     puts parse_result.value
   end
 end

templates/java/org/ruby_lang/prism/Loader.java.erb

@@ -1,8 +1,6 @@
 <%- string_type = Prism::Template::JAVA_STRING_TYPE -%>
 package org.ruby_lang.prism;
 
-import org.ruby_lang.prism.Nodes;
-
 import java.lang.Short;
 import java.math.BigInteger;
 import java.nio.ByteBuffer;
@@ -15,8 +13,8 @@ import java.util.Locale;
 // @formatter:off
 public class Loader {
 
-    public static ParseResult load(byte[] serialized, byte[] sourceBytes) {
-        return new Loader(serialized).load(sourceBytes);
+    public static ParseResult load(byte[] serialized) {
+        return new Loader(serialized).load();
     }
 
     // Overridable methods
@@ -76,15 +74,14 @@ public class Loader {
     private Charset encodingCharset;
     <%- end -%>
     private ConstantPool constantPool;
+    private Nodes.Source source = null;
 
     protected Loader(byte[] serialized) {
         this.buffer = ByteBuffer.wrap(serialized).order(ByteOrder.nativeOrder());
     }
 
-    // We pass sourceBytes here and not in the constructor to avoid keeping
-    // the sourceBytes in memory unnecessarily with lazy DefNode's which hold on the Loader.
-    protected ParseResult load(byte[] sourceBytes) {
-        Nodes.Source source = new Nodes.Source(sourceBytes);
+    protected ParseResult load() {
+        this.source = new Nodes.Source();
 
         expect((byte) 'P', "incorrect prism header");
         expect((byte) 'R', "incorrect prism header");
@@ -129,8 +126,7 @@ public class Loader {
                 throw new Error("Expected to consume all bytes while deserializing but there were " + left + " bytes left");
             }
 
-            boolean[] newlineMarked = new boolean[1 + source.getLineCount()];
-            MarkNewlinesVisitor visitor = new MarkNewlinesVisitor(source, newlineMarked);
+            MarkNewlinesVisitor visitor = new MarkNewlinesVisitor(source);
             node.accept(visitor);
         } else {
             node = null;
@@ -409,11 +405,17 @@ public class Loader {
     }
 
     Nodes.DefNode createDefNodeFromSavedPosition(<%= base_params_sig -%>, int bufferPosition) {
+        Nodes.DefNode node;
         // This method mutates the buffer position and may be called from different threads so we must synchronize
         synchronized (this) {
             buffer.position(bufferPosition);
-            return createDefNode(<%= base_params.join(", ") -%>);
+            node = createDefNode(<%= base_params.join(", ") -%>);
         }
+
+        MarkNewlinesVisitor visitor = new MarkNewlinesVisitor(source);
+        node.accept(visitor);
+
+        return node;
     }
     <%- array_types.uniq.each do |type| -%>
 

templates/java/org/ruby_lang/prism/Nodes.java.erb

@@ -47,12 +47,10 @@ public abstract class Nodes {
     }
 
     public static final class Source {
-        public final byte[] bytes;
         private int startLine = 1;
         private int[] lineOffsets = null;
 
-        Source(byte[] bytes) {
-          this.bytes = bytes;
+        Source() {
         }
 
         void setStartLine(int startLine) {
@@ -70,7 +68,6 @@ public abstract class Nodes {
 
         // 0-based
         public int findLine(int byteOffset) {
-            if (byteOffset >= bytes.length) byteOffset = bytes.length - 1;
             assert byteOffset >= 0 : byteOffset;
             int index = Arrays.binarySearch(lineOffsets, byteOffset);
             int line;