From e6340258f88064cabba5150cfef1f8898f6aa9d8 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Mon, 13 May 2024 11:26:39 -0400 Subject: [PATCH] [ruby/prism] Autoload newlines and comment visitors 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. https://github.com/ruby/prism/commit/eea92c07d2 --- lib/prism.rb | 2 - lib/prism/parse_result.rb | 17 +++ lib/prism/parse_result/comments.rb | 7 -- lib/prism/parse_result/newlines.rb | 111 ++++++++++++++++-- .../lib/prism/inspect_visitor.rb.erb | 7 +- prism/templates/lib/prism/node.rb.erb | 32 ----- 6 files changed, 118 insertions(+), 58 deletions(-) diff --git a/lib/prism.rb b/lib/prism.rb index 19774538e7..2bb7f79bf6 100644 --- a/lib/prism.rb +++ b/lib/prism.rb @@ -71,8 +71,6 @@ require_relative "prism/polyfill/byteindex" 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 diff --git a/lib/prism/parse_result.rb b/lib/prism/parse_result.rb index 63cc72a966..798fde09e5 100644 --- a/lib/prism/parse_result.rb +++ b/lib/prism/parse_result.rb @@ -574,6 +574,12 @@ module Prism # 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 @@ -587,6 +593,17 @@ module Prism 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. diff --git a/lib/prism/parse_result/comments.rb b/lib/prism/parse_result/comments.rb index 3fa0603d74..22c4148b2c 100644 --- a/lib/prism/parse_result/comments.rb +++ b/lib/prism/parse_result/comments.rb @@ -183,12 +183,5 @@ module Prism [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 diff --git a/lib/prism/parse_result/newlines.rb b/lib/prism/parse_result/newlines.rb index 4a8151cc09..cc1343dfda 100644 --- a/lib/prism/parse_result/newlines.rb +++ b/lib/prism/parse_result/newlines.rb @@ -17,21 +17,26 @@ module Prism # 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 @@ -39,7 +44,7 @@ module Prism # Mark if/unless nodes as newlines. def visit_if_node(node) - node.set_newline_flag(@newline_marked) + node.newline!(@lines) super(node) end @@ -48,17 +53,101 @@ module Prism # 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 - private_constant :Newlines + class Node + def newline? # :nodoc: + @newline ? true : false + 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 + 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 + + class InterpolatedSymbolNode < Node + def newline!(lines) # :nodoc: + first = parts.first + first.newline!(lines) if first + end + end + + class InterpolatedXStringNode < Node + def newline!(lines) # :nodoc: + first = parts.first + first.newline!(lines) if first end end end diff --git a/prism/templates/lib/prism/inspect_visitor.rb.erb b/prism/templates/lib/prism/inspect_visitor.rb.erb index 8e7902f0f1..9328da636b 100644 --- a/prism/templates/lib/prism/inspect_visitor.rb.erb +++ b/prism/templates/lib/prism/inspect_visitor.rb.erb @@ -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. diff --git a/prism/templates/lib/prism/node.rb.erb b/prism/templates/lib/prism/node.rb.erb index 50e622a8d1..f0ce226def 100644 --- a/prism/templates/lib/prism/node.rb.erb +++ b/prism/templates/lib/prism/node.rb.erb @@ -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 @@ -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) -%> @@ -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