Skip to content

Commit

Permalink
[ruby/prism] Autoload newlines and comment visitors
Browse files Browse the repository at this point in the history
Having the @newline instance variable in every node adds up, and
since it is so rarely used, we only want to add it when necessary.

Moving this into an autoloaded file and moving the instance variable
out of the default initializers reduces allocated memory because the
nodes are now smaller and some fit into the compact list. On my
machine, I'm seeing about an 8% drop.

ruby/prism@eea92c07d2
  • Loading branch information
kddnewton authored and matzbot committed May 13, 2024
1 parent 02c8e65 commit e634025
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 58 deletions.
2 changes: 0 additions & 2 deletions lib/prism.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ def self.load(source, serialized)
require_relative "prism/node"
require_relative "prism/node_ext"
require_relative "prism/parse_result"
require_relative "prism/parse_result/comments"
require_relative "prism/parse_result/newlines"

# This is a Ruby implementation of the prism parser. If we're running on CRuby
# and we haven't explicitly set the PRISM_FFI_BACKEND environment variable, then
Expand Down
17 changes: 17 additions & 0 deletions lib/prism/parse_result.rb
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,12 @@ def failure?

# This is a result specific to the `parse` and `parse_file` methods.
class ParseResult < Result
autoload :Comments, "prism/parse_result/comments"
autoload :Newlines, "prism/parse_result/newlines"

private_constant :Comments
private_constant :Newlines

# The syntax tree that was parsed from the source code.
attr_reader :value

Expand All @@ -587,6 +593,17 @@ def initialize(value, comments, magic_comments, data_loc, errors, warnings, sour
def deconstruct_keys(keys)
super.merge!(value: value)
end

# Attach the list of comments to their respective locations in the tree.
def attach_comments!
Comments.new(self).attach! # steep:ignore
end

# Walk the tree and mark nodes that are on a new line, loosely emulating
# the behavior of CRuby's `:line` tracepoint event.
def mark_newlines!
value.accept(Newlines.new(source.offsets.size)) # steep:ignore
end
end

# This is a result specific to the `lex` and `lex_file` methods.
Expand Down
7 changes: 0 additions & 7 deletions lib/prism/parse_result/comments.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,5 @@ def nearest_targets(node, comment)
[preceding, NodeTarget.new(node), following]
end
end

private_constant :Comments

# Attach the list of comments to their respective locations in the tree.
def attach_comments!
Comments.new(self).attach! # steep:ignore
end
end
end
111 changes: 100 additions & 11 deletions lib/prism/parse_result/newlines.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,34 @@ class ParseResult < Result
# Note that the logic in this file should be kept in sync with the Java
# MarkNewlinesVisitor, since that visitor is responsible for marking the
# newlines for JRuby/TruffleRuby.
#
# This file is autoloaded only when `mark_newlines!` is called, so the
# re-opening of the various nodes in this file will only be performed in
# that case. We do that to avoid storing the extra `@newline` instance
# variable on every node if we don't need it.
class Newlines < Visitor
# Create a new Newlines visitor with the given newline offsets.
def initialize(newline_marked)
@newline_marked = newline_marked
def initialize(lines)
@lines = Array.new(1 + lines, false)
end

# Permit block/lambda nodes to mark newlines within themselves.
def visit_block_node(node)
old_newline_marked = @newline_marked
@newline_marked = Array.new(old_newline_marked.size, false)
old_lines = @lines
@lines = Array.new(old_lines.size, false)

begin
super(node)
ensure
@newline_marked = old_newline_marked
@lines = old_lines
end
end

alias_method :visit_lambda_node, :visit_block_node

# Mark if/unless nodes as newlines.
def visit_if_node(node)
node.set_newline_flag(@newline_marked)
node.newline!(@lines)
super(node)
end

Expand All @@ -48,17 +53,101 @@ def visit_if_node(node)
# Permit statements lists to mark newlines within themselves.
def visit_statements_node(node)
node.body.each do |child|
child.set_newline_flag(@newline_marked)
child.newline!(@lines)
end
super(node)
end
end
end

class Node
def newline? # :nodoc:
@newline ? true : false
end

def newline!(lines) # :nodoc:
line = location.start_line
unless lines[line]
lines[line] = true
@newline = true
end
end
end

class BeginNode < Node
def newline!(lines) # :nodoc:
# Never mark BeginNode with a newline flag, mark children instead.
end
end

class ParenthesesNode < Node
def newline!(lines) # :nodoc:
# Never mark ParenthesesNode with a newline flag, mark children instead.
end
end

class IfNode < Node
def newline!(lines) # :nodoc:
predicate.newline!(lines)
end
end

class UnlessNode < Node
def newline!(lines) # :nodoc:
predicate.newline!(lines)
end
end

class UntilNode < Node
def newline!(lines) # :nodoc:
predicate.newline!(lines)
end
end

class WhileNode < Node
def newline!(lines) # :nodoc:
predicate.newline!(lines)
end
end

class RescueModifierNode < Node
def newline!(lines) # :nodoc:
expression.newline!(lines)
end
end

class InterpolatedMatchLastLineNode < Node
def newline!(lines) # :nodoc:
first = parts.first
first.newline!(lines) if first
end
end

class InterpolatedRegularExpressionNode < Node
def newline!(lines) # :nodoc:
first = parts.first
first.newline!(lines) if first
end
end

class InterpolatedStringNode < Node
def newline!(lines) # :nodoc:
first = parts.first
first.newline!(lines) if first
end
end

private_constant :Newlines
class InterpolatedSymbolNode < Node
def newline!(lines) # :nodoc:
first = parts.first
first.newline!(lines) if first
end
end

# Walk the tree and mark nodes that are on a new line.
def mark_newlines!
value.accept(Newlines.new(Array.new(1 + source.offsets.size, false))) # steep:ignore
class InterpolatedXStringNode < Node
def newline!(lines) # :nodoc:
first = parts.first
first.newline!(lines) if first
end
end
end
7 changes: 1 addition & 6 deletions prism/templates/lib/prism/inspect_visitor.rb.erb
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,8 @@ module Prism

# Compose a header for the given node.
def inspect_node(name, node)
result = +"@ #{name} ("

location = node.location
result << "location: (#{location.start_line},#{location.start_column})-(#{location.end_line},#{location.end_column})"
result << ", newline: true" if node.newline?

result << ")\n"
"@ #{name} (location: (#{location.start_line},#{location.start_column})-(#{location.end_line},#{location.end_column}))\n"
end

# Compose a string representing the given inner location field.
Expand Down
32 changes: 0 additions & 32 deletions prism/templates/lib/prism/node.rb.erb
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,6 @@ module Prism
location.is_a?(Location) ? location.end_offset : ((location >> 32) + (location & 0xFFFFFFFF))
end

def newline? # :nodoc:
@newline ? true : false
end

def set_newline_flag(newline_marked) # :nodoc:
line = location.start_line
unless newline_marked[line]
newline_marked[line] = true
@newline = true
end
end

# Returns all of the lines of the source code associated with this node.
def source_lines
location.source_lines
Expand Down Expand Up @@ -181,7 +169,6 @@ module Prism
# def initialize: (<%= (node.fields.map { |field| "#{field.rbs_class} #{field.name}" } + ["Location location"]).join(", ") %>) -> void
def initialize(source, <%= (node.fields.map(&:name) + ["location"]).join(", ") %>)
@source = source
@newline = false
@location = location
<%- node.fields.each do |field| -%>
<%- if Prism::Template::CHECK_FIELD_KIND && field.respond_to?(:check_field_kind) -%>
Expand All @@ -195,25 +182,6 @@ module Prism
def accept(visitor)
visitor.visit_<%= node.human %>(self)
end
<%- if node.newline == false -%>

def set_newline_flag(newline_marked) # :nodoc:
# Never mark <%= node.name %> with a newline flag, mark children instead
end
<%- elsif node.newline.is_a?(String) -%>

def set_newline_flag(newline_marked) # :nodoc:
<%- field = node.fields.find { |f| f.name == node.newline } or raise node.newline -%>
<%- case field -%>
<%- when Prism::Template::NodeField -%>
<%= field.name %>.set_newline_flag(newline_marked)
<%- when Prism::Template::NodeListField -%>
first = <%= field.name %>.first
first.set_newline_flag(newline_marked) if first
<%- else raise field.class.name -%>
<%- end -%>
end
<%- end -%>

# def child_nodes: () -> Array[nil | Node]
def child_nodes
Expand Down

0 comments on commit e634025

Please sign in to comment.