Conflicting requirements between Ruby and Java builds

[headius]

I'm very close to having the JVM WASM-based library hooked up to the Ruby code, but I've run into some conflicting requirements.

The current Java Loader class requires that the serialized content have no location fields:

java.lang.Error: Deserialization error: Loader.java requires no location fields in the serialized output (expected 1 but was 0 at position 9)
	at org.ruby_lang.prism.Loader.expect(Loader.java:773)
	at org.ruby_lang.prism.Loader.load(Loader.java:107)
	at org.ruby_lang.prism.Loader.load(Loader.java:26)
	at org.ruby_lang.prism.wasm.Prism.serializeParse(Prism.java:151)
	at org.jruby.parser.prism.WASMTest.test1Aot(WASMTest.java:51)

But the deserializer in the Ruby code requires that content have those location fields:

$ jruby -Ilib -e 'require "prism"; p Prism.parse("def foo; 1; end")'
RuntimeError: Invalid serialization (location fields must be included but are not)
  load_header at /Users/headius/work/prism/lib/prism/serialize.rb:289
   load_parse at /Users/headius/work/prism/lib/prism/serialize.rb:39
        parse at /Users/headius/work/prism/lib/prism/wasm.rb:60
       <main> at -e:1

I would rather not ship two different WASM builds to satisfy both. Why does the Loader not handle location fields?

[headius]

This appears to have been started by #807 to reduce the amount of serialized content JRuby and TruffleRuby that don't need location on every node.

No such support was ever added to the serialize.rb, though, and I guess it still requires location to be on nodes.

Making serialize.rb compatible with the location-omitted build would probably be the best option here.

[hsbt]

@headius Is it for https://github.com/ruby/prism? This repository is psych.

[hsbt]

I can transfer this issue to ruby/prism. Let me know if you want to do that.

[headius]

Oh I'm very sorry... Too many tabs and I picked the wrong repository! I will move this and #3963.

[headius]

A naïve attempt to fake out the Location logic did not appear to work (complained about an invalid warning level, so I assume it's still off):

diff --git a/templates/lib/prism/serialize.rb.erb b/templates/lib/prism/serialize.rb.erb
index 63ef07cb6..6e5630b6b 100644
--- a/templates/lib/prism/serialize.rb.erb
+++ b/templates/lib/prism/serialize.rb.erb
@@ -275,7 +275,7 @@ module Prism
       def load_header
         raise "Invalid serialization" if io.read(5) != "PRISM"
         raise "Invalid serialization" if io.read(3).unpack("C3") != [MAJOR_VERSION, MINOR_VERSION, PATCH_VERSION]
-        raise "Invalid serialization (location fields must be included but are not)" if io.getbyte != 0
+        @load_locations = io.getbyte == 0
       end
 
       def load_encoding
@@ -405,11 +405,9 @@ module Prism
         tokens = []
 
         while (type = TOKEN_TYPES.fetch(load_varuint))
-          start = load_varuint
-          length = load_varuint
           lex_state = load_varuint
 
-          location = Location.new(@source, start, length)
+          location = load_location_object(false)
           token = Token.new(@source, type, location.slice, location)
 
           tokens << [token, lex_state]
@@ -481,7 +479,9 @@ module Prism
       end
 
       def load_location_object(freeze)
-        location = Location.new(source, load_varuint, load_varuint)
+        location = @load_locations ?
+                     Location.new(source, load_varuint, load_varuint) :
+                     Location.new(source, 0, 0)
         location.freeze if freeze
         location
       end

[headius]

On second thought... in order for the WASM build to be fully functional within the gem, it's going to need the non-semantic data like locations and comments, correct?

So I may be stuck requiring a separate build for the gem that includes that data.

[headius]

It's not currently possible to build the WASM with location info while running JRuby (due to a hardcoded check ) but after hacking that out I have a working Prism.parse using the Chicory WASM backend!

$ jruby -Ilib -e 'require "prism"; p Prism.parse("1")'  
#<Prism::ParseResult:0x43755e2f @value=@ ProgramNode (location: (1,0)-(1,1))
├── flags: ∅
├── locals: []
└── statements:
    @ StatementsNode (location: (1,0)-(1,1))
    ├── flags: ∅
    └── body: (length: 1)
        └── @ IntegerNode (location: (1,0)-(1,1))
            ├── flags: newline, static_literal, decimal
            └── value: 1
, @source=#<Prism::ASCIISource:0x778c2e7c @offsets=[0], @source="1", @start_line=1>, @comments=[], @data_loc=nil, @warnings=[#<Prism::ParseWarning @type=:void_statement @message="possibly useless use of a literal in void context" @location=#<Prism::Location @start_offset=0 @length=1 start_line=1> @level=:verbose>], @magic_comments=[], @errors=[]>

[eregon]

On second thought... in order for the WASM build to be fully functional within the gem, it's going to need the non-semantic data like locations and comments, correct?

Yes. It's by design a build/template-time thing to avoid any overhead.
IIRC I described that in one of my PRs, probably linked to #807.
TruffleRuby ships two versions of Prism for that and other reasons (notably the internal one used for parsing in Java needs to not expose native symbols and the other needs to for FFI & the Ruby API).

due to a hardcoded check

This points to a blank line.

[eregon]

Also when installing another version of the Prism gem you'd have two separate WASM builds of Prism anyway, so might as well be consistent for the "Prism default gem" and have its own WASM build separate from the internal Java/JRuby usage.

[headius]

This points to a blank line.

Looks like it's the previous line then. My local copy must have been off by one. The check forces templates generated by JRuby to always omit line information, even if requested by env var.

Prism default gem

This wasm version will also ship as the default backend for the regular prism gem. Users can opt into the native version with a separate gem.

I will likely need to push two separate Maven artifacts for this to avoid having twice the AOT WASM code in one. It's annoying but not a problem.

[headius]

This wasm version will also ship as the default backend for the regular prism gem. Users can opt into the native version with a separate gem.

For JRuby, that is.

[eregon]

Looks like it's the previous line then. My local copy must have been off by one. The check forces templates generated by JRuby to always omit line information, even if requested by env var.

The previous line is:

prism/templates/template.rb

Line 16 in 80b7402

INCLUDE_NODE_ID = !SERIALIZE_ONLY_SEMANTICS_FIELDS || JAVA_BACKEND == "jruby"

Is the problem about node_id? I'd think JRuby always wants that.

I suppose

prism/templates/template.rb

Line 10 in 80b7402

SERIALIZE_ONLY_SEMANTICS_FIELDS = ENV.fetch("PRISM_SERIALIZE_ONLY_SEMANTICS_FIELDS", false)

is the relevant line but that's not hardcoded, it depends on an env var.

Anyway, doesn't matter much, could you create a PR for the change you have there?

[enebo]

This was about saving size and not also parsing/skipping tracking all non-semantic fields like comments. node_id is an unfortunate addition needed for Rails but if it was ever possible to divine it (which I trust @tenderlove spent quite a bit of time trying for MRI) then we should also not use that for the same reasons.

I think there is an argument if we cannot really tell the difference then perhaps we could just use one form. If we can then deployin two artifacts seems like less an issue than slowing down parsing.

[eregon]

if we cannot really tell the difference

We can, it's almost twice as small with only semantic fields:
https://github.com/ruby/prism/actions/runs/22742474605/job/65958817017

All fields:

Total sizes for top 100 gems:
total source size:      90207647
total serialized size: 113025865
total serialized/total source: 1.253

Stats of ratio serialized/source per file:
average: 1.350
median:  1.343
1st quartile: 0.980
3rd quartile: 1.681
min - max: 0.129 - 4.922

Semantic fields only:

Total sizes for top 100 gems:
total source size:      90207647
total serialized size:  66147370
total serialized/total source: 0.733

Stats of ratio serialized/source per file:
average: 0.800
median:  0.779
1st quartile: 0.568
3rd quartile: 1.007
min - max: 0.076 - 3.675

Let's close this issue, it's "conflicting" on purpose/by design and a requirement for the smaller serialized size & avoiding many costly runtime checks.