Shopify / seafoam

A tool for working with compiler graphs dumped by the GraalVM compiler
MIT License
126 stars 22 forks source link

`describe` is somewhat broken in Seafoam 0.16 #90

Open nirvdrum opened 5 months ago

nirvdrum commented 5 months ago

I noticed when running seafoam describe on some graphs I would get a cryptic error:

seafoam: comparison of Array with Array failed

Running with debug mode to get a real trace, I see:

> ruby -d -S seafoam /Users/nirvdrum/dev/workspaces/truffleruby-ws/truffleruby/TruffleHotSpotCompilation-
Exception `ArgumentError' at /Users/nirvdrum/.rubies/ruby-3.3.0/lib/ruby/gems/3.3.0/gems/seafoam-0.16/lib/seafoam/graal/graph_description.rb:25 - comparison of Array with Array failed
Exception `ArgumentError' at /Users/nirvdrum/.rubies/ruby-3.3.0/lib/ruby/gems/3.3.0/gems/seafoam-0.16/bin/seafoam:16 - comparison of Array with Array failed
/Users/nirvdrum/.rubies/ruby-3.3.0/lib/ruby/gems/3.3.0/gems/seafoam-0.16/lib/seafoam/graal/graph_description.rb:25:in `sort_by': comparison of Array with Array failed (ArgumentError)
    from /Users/nirvdrum/.rubies/ruby-3.3.0/lib/ruby/gems/3.3.0/gems/seafoam-0.16/lib/seafoam/graal/graph_description.rb:25:in `sorted_node_counts'
    from /Users/nirvdrum/.rubies/ruby-3.3.0/lib/ruby/gems/3.3.0/gems/seafoam-0.16/lib/seafoam/formatters/text.rb:11:in `format'
    from /Users/nirvdrum/.rubies/ruby-3.3.0/lib/ruby/gems/3.3.0/gems/seafoam-0.16/lib/seafoam/commands.rb:414:in `block in describe'
    from <internal:kernel>:187:in `loop'
    from /Users/nirvdrum/.rubies/ruby-3.3.0/lib/ruby/gems/3.3.0/gems/seafoam-0.16/lib/seafoam/commands.rb:372:in `describe'
    from /Users/nirvdrum/.rubies/ruby-3.3.0/lib/ruby/gems/3.3.0/gems/seafoam-0.16/lib/seafoam/commands.rb:54:in `seafoam'
    from /Users/nirvdrum/.rubies/ruby-3.3.0/lib/ruby/gems/3.3.0/gems/seafoam-0.16/bin/seafoam:11:in `<top (required)>'
    from /Users/nirvdrum/.rubies/ruby-3.3.0/bin/seafoam:25:in `load'
    from /Users/nirvdrum/.rubies/ruby-3.3.0/bin/seafoam:25:in `<main>'
nirvdrum commented 5 months ago

I've worked out what the problem is. When updating the test suite for GraalVM 24.0.0 I had seen some test failures related to a nil node_class value. Previously we were comparing values with ==, which would have been fine if one was nil. But, in order to support both the old and new Graal package names, we started comparing classes with end_with? to do a partial search. Having a nil value in that case would result in an exception. I should have looked more into it at the time, but without thinking too deeply about it I used "" as a fallback and that worked.

While that fixed the Truffle pass specs, it turns out describe used a different code path and was still seeing a nil value. That, in turn, resulted in exception when calling sort_by on an Array.

I'm pretty sure the problem is the synthetic nodes. While debugging I saw that the nodes failing, at the very least, were the Truffle argument nodes. I think the solution is to use the synthetic_class value in that case. There may be other situations where the node_class is not populated, but I haven't seen any of them.