Shopify / seafoam

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

Add option for automation-friendly output #41

Closed nirvdrum closed 2 years ago

nirvdrum commented 3 years ago

I'm working on a tool that uses Seafoam internally, parsing the data returned by Seafoam on STDOUT. While that mostly works, I've run into a problem with the list command. It outputs two pieces of information, separated by some whitespace:

https://github.com/Shopify/seafoam/blob/a984e7226dc5fd4e49b2bc043b6033597a631454/lib/seafoam/commands.rb#L212

Parsing the "#{file}:#{index} column can be slightly tricky because the filename may have : in it. E.g., a compilation of Truffle::RegexpOperations.match will include the qualified method name in the filename. I've had success splitting on the last :, but I'm not confident that'll hold up in the long-run.

Parsing the "#{parser.graph_name(graph_header)}" column is proving to be much trickier. Seafoam uses a/delimite to join the components of thegraph_name`:

https://github.com/Shopify/seafoam/blob/a984e7226dc5fd4e49b2bc043b6033597a631454/lib/seafoam/bgv/bgv_parser.rb#L157-L167

Those components may have an / embedded in them, however, such as is the case with a JIT compiled regex. The phase name nested in a group may also have an /, such as "Call Tree/Before Inline". As a result, I can't split on / and reliably separate the phase name from the method name.

It would be helpful if there were a structured output format for the Seafoam commands. I'd suggest JSON since it's common, but I'd be happy with anything that will allow faithful representation of the values from the various commands. Parsing difficulties aside, having a machine-readable output format also means tooling won't break every time Seafoam changes the way it presents data to the user.

chrisseaton commented 3 years ago

Right - for any command that prints output maybe we should have a --json equivalent.

chrisseaton commented 2 years ago

This is our JSON output isn't it?

nirvdrum commented 2 years ago

Indeed. I'm sorry about that. It was fixed in #48.