Add node event dispatcher #1523
Conversation
This commit changes the node template to create a dispatcher class, which can be used to walk an AST an emit events to all registered listeners Co-authored-by: Kevin Newton <[email protected]>
| @@ -48,6 +48,31 @@ module YARP | |||
| }.compact.join(", ") %>] | |||
| end | |||
|
|
|||
| # def compact_child_nodes: () -> Array[Node] | |||
| def compact_child_nodes | |||
| <%- if node.fields.any? { |field| field.is_a?(YARP::OptionalNodeField) } -%> | |||
There was a problem hiding this comment.
Another possibility would be to use [node_field, *optional_node_field, *node_list_field] because there is no #to_a on Node and *nil is [].
Not sure if faster or slower though, it would make the logic here a bit simpler.
| case node.human | ||
| <%- nodes.each do |node| -%> | ||
| when :<%= node.human %> | ||
| listeners[:<%= node.human %>_enter]&.each { |listener| listener.<%= node.human %>_enter(node) } | ||
| queue = node.compact_child_nodes.concat(queue) | ||
| listeners[:<%= node.human %>_leave]&.each { |listener| listener.<%= node.human %>_leave(node) } | ||
| <%- end -%> | ||
| end |
There was a problem hiding this comment.
These big case/when/end statements (dispatch is 570 lines) are pretty bad for optimizing JITs like on TruffleRuby and JRuby, it's likely too much code in a single method to compile or optimize well, somewhat similar to whitequark/parser#871.
Also this relies very heavily on case expr; when :symbol; when :symbol2; ...; end to use a Hash internally, otherwise it would be very slow as it would test each Symbol in order. I think that's the case on CRuby but on no other Ruby implementation.
I think something that would be fast for all Rubies and behave better for JIT would be to use a Hash of node.human to Procs containing the body of each when (one per node type to avoid needing listerner.send which would be slow).
| <%- nodes.each do |node| -%> | ||
| when :<%= node.human %> | ||
| listeners[:<%= node.human %>_enter]&.each { |listener| listener.<%= node.human %>_enter(node) } | ||
| queue = node.compact_child_nodes.concat(queue) |
There was a problem hiding this comment.
Is this the correct order?
Maybe it should be queue.concat(node.compact_child_nodes)?
It looks like this adds child nodes before the current elements in the array (so on the "left side" i.e. near 0 index) and queue.shift above removes from the "left side" too.
So it looks like it behaves somewhat like a stack:
q = [1]
q.shift
q = [2, 3].concat(q)
p q # => [2, 3]
p q.shift # => 2
q = [4].concat(q)
p q # => [4, 3]
p q.shift # => 4, the last added element is the first out in this case
# Not fully LIFO though because if it was `q = [4, 5].concat(q)` above, then 4 is out before 5
This commit changes the node template to create a dispatcher class, which can be used to walk an AST an emit events to all registered listeners.
This is useful when performing several types of analysis on the same AST to avoid having to perform several rounds of visits. Two main examples are:
For both of them, this allows the complete analysis to be completed in a single round of visits, saving numerous method dispatches and improving performance.