Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use prism for parsing renders when available #49438

Merged
merged 1 commit into from Oct 1, 2023

Conversation

kddnewton
Copy link
Contributor

Motivation / Background

For tracking template dependencies, rails currently uses the ripper library to parse Ruby code and find calls to render or render_to_string. A ripper subclass is difficult to maintain since its interface is not guaranteed, it's not very fast, and it's generally pretty opaque in its implementation.

Detail

This pull request uses the prism standard library, which is available as of Ruby 3.3. The code is much smaller and hopefully easier to understand. Additionally it uses named fields as opposed to array indices, so should be more maintainable going forward.

Additional information

benchmark.rb
# frozen_string_literal: true

require "bundler/inline"
gemfile do
  source "https://rubygems.org"
  gem "benchmark-ips"
  gem "prism"

  gem "activesupport", path: "../activesupport"
  gem "activemodel", path: "../activemodel"
  gem "actionpack", path: "../actionpack"

  gemspec
end

require "ripper"
require "active_support/core_ext/string/inflections"
require "action_view/render_parser"
require "action_view/render_parser/prism_render_parser"
require "action_view/render_parser/ripper_render_parser"

name = __FILE__
code = DATA.read

Benchmark.ips do |x|
  x.report("Prism") { ActionView::RenderParser::PrismRenderParser.new(name, code).render_calls }
  x.report("Ripper") { ActionView::RenderParser::RipperRenderParser.new(name, code).render_calls }
  x.compare!
end

__END__
 @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render partial: "unknown_render_call", unknown_render_option: "yes" ); @output_buffer.safe_append='
    '.freeze;
@output_buffer

 @output_buffer.append=( render 'messages/message123' );
@output_buffer

 @output_buffer.append=( render partial: 'ピカチュウ', object: @pokémon );
@output_buffer

 @output_buffer.append=  render layout: 'messages/layout' do ;
@output_buffer

 @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render              "header" ); @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render    partial:  "form" ); @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render              @message ); @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render ( @message.events ) ); @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render    :collection => @message.comments,
                    :partial =>    "comments/comment" ); @output_buffer.safe_append='
    '.freeze;
@output_buffer

 @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render "double/#{quote}" ); @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render 'single/#{quote}' ); @output_buffer.safe_append='
    '.freeze;
@output_buffer

 @output_buffer.safe_append='
'.freeze;
 @output_buffer.safe_append='    '.freeze;
@output_buffer

 @output_buffer.append=( render'messages/message' );
@output_buffer

 @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render SomeConstant.message(this: "that") ); @output_buffer.safe_append='
    '.freeze;
@output_buffer

 @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render partial: "messages/message", collection: books, spacer_template: "messages/message_spacer" ); @output_buffer.safe_append='
    '.freeze;
@output_buffer

 @output_buffer.append=( render(message.topic) );
@output_buffer

 @output_buffer.append=(
      render object: @all_posts,
             partial: 'posts' );
@output_buffer

 @output_buffer.append=( rendering 'it useless' );
@output_buffer

 @output_buffer.append=( surrender 'to reason' );
@output_buffer

 @output_buffer.append=( render @parent.child.grandchildren );
@output_buffer

 @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render $globals ); @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render @instance_variables ); @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render @@class_variables ); @output_buffer.safe_append='
    '.freeze;
@output_buffer

 @output_buffer.append=( render partial: 'messages/show', layout: 'messages/layout' );
@output_buffer

 @output_buffer.append=( render(message_type.messages) );
@output_buffer

 @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render('application/header', title: 'Title') ); @output_buffer.safe_append='
      <h2>Section title</h2>
      '.freeze; @output_buffer.append=( render@section ); @output_buffer.safe_append='
    '.freeze;
@output_buffer

 @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render "single/quote's" ); @output_buffer.safe_append='
      '.freeze; @output_buffer.append=( render 'double/quote"s' ); @output_buffer.safe_append='
    '.freeze;
@output_buffer

Running the above benchmark script (which is a concatenation of all of the tests in the repository), you get:

Warming up --------------------------------------
               Prism   173.000  i/100ms
              Ripper    97.000  i/100ms
Calculating -------------------------------------
               Prism      1.731k (± 1.5%) i/s -      8.823k in   5.098289s
              Ripper    942.045  (± 4.1%) i/s -      4.753k in   5.054548s

Comparison:
               Prism:     1731.0 i/s
              Ripper:      942.0 i/s - 1.84x  slower

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot bot added the actionview label Sep 30, 2023
@rafaelfranca rafaelfranca merged commit 001f7a4 into rails:main Oct 1, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants