Skip to content

Commit 65634d8

Browse files
Earlopainmatzbot
authored andcommitted
[ruby/prism] Optimize ruby visitor
`compact_child_nodes` allocates an array. We can skip that step by simply yielding the nodes. Benchmark for visiting the rails codebase: ```rb require "prism" require "benchmark/ips" files = Dir.glob("../rails/**/*.rb") results = files.map { Prism.parse_file(it) } visitor = Prism::Visitor.new Benchmark.ips do |x| x.config(warmup: 3, time: 10) x.report do results.each do visitor.visit(it.value) end end end RubyVM::YJIT.enable Benchmark.ips do |x| x.config(warmup: 3, time: 10) x.report do results.each do visitor.visit(it.value) end end end ``` Before: ``` ruby 3.4.8 (2025-12-17 revision ruby/prism@995b59f666) +PRISM [x86_64-linux] Warming up -------------------------------------- 1.000 i/100ms Calculating ------------------------------------- 2.691 (± 0.0%) i/s (371.55 ms/i) - 27.000 in 10.089422s ruby 3.4.8 (2025-12-17 revision ruby/prism@995b59f666) +YJIT +PRISM [x86_64-linux] Warming up -------------------------------------- 1.000 i/100ms Calculating ------------------------------------- 7.278 (±13.7%) i/s (137.39 ms/i) - 70.000 in 10.071568s ``` After: ``` ruby 3.4.8 (2025-12-17 revision ruby/prism@995b59f666) +PRISM [x86_64-linux] Warming up -------------------------------------- 1.000 i/100ms Calculating ------------------------------------- 3.429 (± 0.0%) i/s (291.65 ms/i) - 35.000 in 10.208580s ruby 3.4.8 (2025-12-17 revision ruby/prism@995b59f666) +YJIT +PRISM [x86_64-linux] Warming up -------------------------------------- 1.000 i/100ms Calculating ------------------------------------- 16.815 (± 0.0%) i/s (59.47 ms/i) - 169.000 in 10.054668s ``` ~21% faster on the interpreter, ~56% with YJIT ruby/prism@bf631750cf
1 parent 14fbcf0 commit 65634d8

File tree

4 files changed

+29
-6
lines changed

4 files changed

+29
-6
lines changed

lib/prism/translation/ruby_parser.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1422,7 +1422,7 @@ def visit_or_node(node)
14221422
# ```
14231423
def visit_parameters_node(node)
14241424
children =
1425-
node.compact_child_nodes.map do |element|
1425+
node.each_child_node.map do |element|
14261426
if element.is_a?(MultiTargetNode)
14271427
visit_destructured_parameter(element)
14281428
else

prism/templates/lib/prism/compiler.rb.erb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,14 @@ module Prism
2929

3030
# Visit the child nodes of the given node.
3131
def visit_child_nodes(node)
32-
node.compact_child_nodes.map { |node| node.accept(self) }
32+
node.each_child_node.map { |node| node.accept(self) }
3333
end
3434

3535
<%- nodes.each_with_index do |node, index| -%>
3636
<%= "\n" if index != 0 -%>
3737
# Compile a <%= node.name %> node
3838
def visit_<%= node.human %>(node)
39-
node.compact_child_nodes.map { |node| node.accept(self) }
39+
node.each_child_node.map { |node| node.accept(self) }
4040
end
4141
<%- end -%>
4242
end

prism/templates/lib/prism/node.rb.erb

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ module Prism
187187
while (node = queue.shift)
188188
result << node
189189

190-
node.compact_child_nodes.each do |child_node|
190+
node.each_child_node do |child_node|
191191
child_location = child_node.location
192192

193193
start_line = child_location.start_line
@@ -259,6 +259,13 @@ module Prism
259259

260260
alias deconstruct child_nodes
261261

262+
# With a block given, yields each child node. Without a block, returns
263+
# an enumerator that contains each child node. Excludes any `nil`s in
264+
# the place of optional nodes that were not present.
265+
def each_child_node
266+
raise NoMethodError, "undefined method `each_child_node' for #{inspect}"
267+
end
268+
262269
# Returns an array of child nodes, excluding any `nil`s in the place of
263270
# optional nodes that were not present.
264271
def compact_child_nodes
@@ -335,6 +342,22 @@ module Prism
335342
}.compact.join(", ") %>]
336343
end
337344

345+
# def each_child_node: () { (Prism::node) -> void } -> void | () -> Enumerator[Prism::node]
346+
def each_child_node
347+
return to_enum(:each_child_node) unless block_given?
348+
349+
<%- node.fields.each do |field| -%>
350+
<%- case field -%>
351+
<%- when Prism::Template::NodeField -%>
352+
yield <%= field.name %>
353+
<%- when Prism::Template::OptionalNodeField -%>
354+
yield <%= field.name %> if <%= field.name %>
355+
<%- when Prism::Template::NodeListField -%>
356+
<%= field.name %>.each {|node| yield node }
357+
<%- end -%>
358+
<%- end -%>
359+
end
360+
338361
# def compact_child_nodes: () -> Array[Node]
339362
def compact_child_nodes
340363
<%- if node.fields.any? { |field| field.is_a?(Prism::Template::OptionalNodeField) } -%>

prism/templates/lib/prism/visitor.rb.erb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ module Prism
2020
# Visits the child nodes of `node` by calling `accept` on each one.
2121
def visit_child_nodes(node)
2222
# @type self: _Visitor
23-
node.compact_child_nodes.each { |node| node.accept(self) }
23+
node.each_child_node { |node| node.accept(self) }
2424
end
2525
end
2626

@@ -48,7 +48,7 @@ module Prism
4848
<%= "\n" if index != 0 -%>
4949
# Visit a <%= node.name %> node
5050
def visit_<%= node.human %>(node)
51-
node.compact_child_nodes.each { |node| node.accept(self) }
51+
node.each_child_node { |node| node.accept(self) }
5252
end
5353
<%- end -%>
5454
end

0 commit comments

Comments
 (0)