[kddnewton]

This makes it so that you do not have to recurse when serializing and deserializing, making this safe for deep trees. There's a bit of extension to make lazy def bodies work — we put a marker in the says how much of the node to skip (i.e., the header) and then a length for the body. Other than that it should all be the same.

cc @enebo @eregon I'm definitely not super confident on the Java side, but it seems correct-ish. Let me know what you think. I'm hoping to get this out for 2.0 so we're fully safe for deep trees.

[eregon]

I took a quick look this evening, but it's rather hard to tell from the diff so I'll need to look at the generated code.
Most importantly we should benchmark this to make sure it's not slower than before (previous benchmarks for that here).

[kddnewton]

I wouldn't be surprised if it were a bit slower, but this is a correctness thing. Serializing crashes before we hit the parser-imposed depth limit, meaning we can't serialize every possible tree that is possible to parse.

If this is indeed slower, we could probably track a maximum depth and then switch on that to try to guess if we would overflow the stack, but that feels unreliable.

[eregon]

Very minor but the generated Java & Ruby code looks like:

    private Nodes.GlobalVariableWriteNode loadGlobalVariableWriteNode(NodeStack stack) {
        int startOffset = loadVarUInt();
        int length = loadVarUInt();
        var field_name = loadConstant();
        var field_value = stack.pop();
        return new Nodes.GlobalVariableWriteNode(startOffset, length, field_name, field_value);
    }

Would be more readable without that field_ prefix.

[eregon]

I benchmarked the total parsing time for railsbench boot:
before:

$ for i in 1 2 3 4 5 6 7 8 9; do GEM_HOME=/home/eregon/code/ruby-bench/benchmarks/railsbench/vendor/bundle/truffleruby/4.0.2.10 GEM_PATH= BUNDLER_VERSION=4.0.12 $SYSTEM_RUBY /home/eregon/code/trws/truffleruby/tool/jt.rb -u before-jvm -q ruby --experimental-options --metrics-profile-require=total bin/rails boot; done |& grep 'parsing time'
[ruby] INFO: total     parsing time: 470ms
[ruby] INFO: total     parsing time: 471ms
[ruby] INFO: total     parsing time: 464ms
[ruby] INFO: total     parsing time: 479ms
[ruby] INFO: total     parsing time: 459ms
[ruby] INFO: total     parsing time: 457ms
[ruby] INFO: total     parsing time: 483ms
[ruby] INFO: total     parsing time: 444ms
[ruby] INFO: total     parsing time: 477ms

for i in 1 2 3 4 5 6 7 8 9; do GEM_HOME=/home/eregon/code/ruby-bench/benchmarks/railsbench/vendor/bundle/truffleruby/4.0.2.10 GEM_PATH= BUNDLER_VERSION=4.0.12 $SYSTEM_RUBY /home/eregon/code/trws/truffleruby/tool/jt.rb -u before-jvm -q ruby --experimental-options --metrics-profile-require=total bin/rails boot; done |& grep 'parsing time'
[ruby] INFO: total     parsing time: 476ms
[ruby] INFO: total     parsing time: 460ms
[ruby] INFO: total     parsing time: 459ms
[ruby] INFO: total     parsing time: 485ms
[ruby] INFO: total     parsing time: 474ms
[ruby] INFO: total     parsing time: 472ms
[ruby] INFO: total     parsing time: 490ms
[ruby] INFO: total     parsing time: 460ms
[ruby] INFO: total     parsing time: 468ms

after:

$ for i in 1 2 3 4 5 6 7 8 9; do GEM_HOME=/home/eregon/code/ruby-bench/benchmarks/railsbench/vendor/bundle/truffleruby/4.0.2.10 GEM_PATH= BUNDLER_VERSION=4.0.12 $SYSTEM_RUBY /home/eregon/code/trws/truffleruby/tool/jt.rb -u after-jvm -q ruby --experimental-options --metrics-profile-require=total bin/rails boot; done |& grep 'parsing time'
[ruby] INFO: total     parsing time: 484ms
[ruby] INFO: total     parsing time: 512ms
[ruby] INFO: total     parsing time: 502ms
[ruby] INFO: total     parsing time: 484ms
[ruby] INFO: total     parsing time: 482ms
[ruby] INFO: total     parsing time: 485ms
[ruby] INFO: total     parsing time: 469ms
[ruby] INFO: total     parsing time: 491ms
[ruby] INFO: total     parsing time: 492ms

for i in 1 2 3 4 5 6 7 8 9; do GEM_HOME=/home/eregon/code/ruby-bench/benchmarks/railsbench/vendor/bundle/truffleruby/4.0.2.10 GEM_PATH= BUNDLER_VERSION=4.0.12 $SYSTEM_RUBY /home/eregon/code/trws/truffleruby/tool/jt.rb -u after-jvm -q ruby --experimental-options --metrics-profile-require=total bin/rails boot; done |& grep 'parsing time'
[ruby] INFO: total     parsing time: 481ms
[ruby] INFO: total     parsing time: 523ms
[ruby] INFO: total     parsing time: 513ms
[ruby] INFO: total     parsing time: 477ms
[ruby] INFO: total     parsing time: 507ms
[ruby] INFO: total     parsing time: 501ms
[ruby] INFO: total     parsing time: 487ms
[ruby] INFO: total     parsing time: 510ms
[ruby] INFO: total     parsing time: 500ms

$ xsel -b | cut -c 38-40 | stats
before:
median:  470.0
average: 467.1111111111111
MAD:     9.0 (1.91%)
min-max: [444.0 - 483.0]
Q1-Q3:   [459.0 - 477.0]

median:  472.0
average: 471.55555555555554
MAD:     12.0 (2.54%)
min-max: [459.0 - 490.0]
Q1-Q3:   [460.0 - 476.0]


after:
median:  485.0
average: 489.0
MAD:     6.0 (1.24%)
min-max: [469.0 - 512.0]
Q1-Q3:   [484.0 - 492.0]

median:  501.0
average: 499.8888888888889
MAD:     12.0 (2.40%)
min-max: [477.0 - 523.0]
Q1-Q3:   [487.0 - 510.0]

So it's something like between 3% (485.0/470.0) and 6% (501.0/472.0) slower, not ideal but close enough.

[eregon]

Thank you for fixing this!

[eregon]

Benchmarks on TruffleRuby Native for completeness:

before:
median:  281.0
average: 282.1111111111111
MAD:     20.0 (7.12%)
min-max: [256.0 - 308.0]
Q1-Q3:   [261.0 - 300.0]

median:  264.0
average: 275.0
MAD:     18.0 (6.82%)
min-max: [246.0 - 323.0]
Q1-Q3:   [255.0 - 289.0]

after:
median:  309.0
average: 309.1111111111111
MAD:     12.0 (3.88%)
min-max: [279.0 - 356.0]
Q1-Q3:   [297.0 - 316.0]

median:  294.0
average: 305.55555555555554
MAD:     16.0 (5.44%)
min-max: [278.0 - 346.0]
Q1-Q3:   [286.0 - 325.0]

Much faster than on JVM, but the slowdown is higher: 10-11%.

Might be worth looking into it (after this PR is merged), because parsing is actually quite a bit slower than translating on Native (on JVM translating dominates like parsing 528ms translating 754ms):

[ruby] INFO: total   searching time: 388ms
[ruby] INFO: total     parsing time: 382ms
[ruby] INFO: total translating time: 159ms

[ruby] INFO: total   searching time: 371ms
[ruby] INFO: total     parsing time: 333ms
[ruby] INFO: total translating time: 147ms

[ruby] INFO: total   searching time: 291ms
[ruby] INFO: total     parsing time: 268ms
[ruby] INFO: total translating time: 133ms