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

Support custom parser in Prism::Translation::Parser #3478

Merged

Conversation

koic
Copy link
Contributor

@koic koic commented Feb 25, 2025

Follow-up to Shopify/ruby-lsp#1849.

This is an extension of Prism::Translation::Parser to implement Shopify/ruby-lsp#1849. It is based on the comments in Shopify/ruby-lsp#1849 (review), but also adds a default argument for delegation to Parser::Base super class.

Using this API, rubocop/rubocop-ast#359 has been implemented in RuboCop AST.
As detailed in rubocop/rubocop-ast#359, this change is expected to improve performance by 1.3x for some source code.
Achieving a 1.3x speedup with such this simple modification is a significant improvement for Ruby LSP and its users.

cc @kddnewton @vinistock

koic added a commit to koic/rubocop-ast that referenced this pull request Feb 25, 2025
Follow-up to Shopify/ruby-lsp#1849.

This feature utilizes ruby/prism#3478 to implement Shopify/ruby-lsp#1849.

By using ruby/prism#3478, this feature enables performance improvements by reusing Prism's parsed results
instead of parsing the source code. Below is a sample code snippet, which is expected to improve performance by
1.3x in this case.

```ruby
#!/usr/local/bin/ruby

require 'benchmark/ips'
require 'prism'
require 'rubocop-ast'

@source = File.read(__FILE__)
@parse_lex_result = Prism.parse_lex(@source)

def build_processed_source(parse_lex_result)
  RuboCop::AST::ProcessedSource.new(
    @source,
    3.4,
    __FILE__,
    parser_engine: 'parser_prism',
    prism_result: parse_lex_result
  )
end

Benchmark.ips do |x|
  x.report('source')                { build_processed_source(nil) }
  x.report('Prism::ParseLexResult') { build_processed_source(@parse_lex_result) }

  x.compare!
end
```

```console
$ bundle exec ruby reusable_ast.rb
ruby 3.4.2 (2025-02-15 revision d2930f8e7a) +PRISM [x86_64-darwin23]
Warming up --------------------------------------
              source   116.000 i/100ms
Prism::ParseLexResult
                       151.000 i/100ms
Calculating -------------------------------------
              source      1.144k (± 8.4%) i/s -      5.684k in   5.018270s
Prism::ParseLexResult
                          1.460k (± 5.2%) i/s -      7.399k in   5.082102s

Comparison:
Prism::ParseLexResult:     1460.3 i/s
              source:     1144.4 i/s - 1.28x  slower
```

Achieving 1.3x speedup with such this simple modification is a significant improvement for Ruby LSP and its users.

## Compatibility

By using `parser_class.instance_method(:initialize).parameters.assoc(:key)`,
the implementation checks whether `Prism#initialize` supports the `:parser` keyword.
If it does not, the system falls back to the conventional parsing method.
This ensures backward compatibility with previous versions of Prism.

## Development Notes

Since this feature is specifically designed for Prism, the keyword name `prism_result` is meant for Prism.
While it may be possible to achieve similar functionality with the Parser gem, there are currently no concrete use cases.
Therefore, for clarity, we have chosen `prism_result` as the keyword.

Additionally, Prism has two types of results: `Prism::ParseResult` and `Prism::ParseLexResult`,
but the `prism_result` keyword argument is meant to receive `Prism::ParseLexResult`.
Since `prism_parse_lex_result` would be too long, the name `prism_result` was chosen to align with the superclass `Prism::Result`.
koic added a commit to koic/rubocop-ast that referenced this pull request Feb 25, 2025
Follow-up to Shopify/ruby-lsp#1849.

This feature utilizes ruby/prism#3478 to implement Shopify/ruby-lsp#1849.

By using ruby/prism#3478, this feature enables performance improvements by reusing Prism's parsed results
instead of parsing the source code. Below is a sample code snippet, which is expected to improve performance by
1.3x in this case.

```ruby
#!/usr/local/bin/ruby

require 'benchmark/ips'
require 'prism'
require 'rubocop-ast'

@source = File.read(__FILE__)
@parse_lex_result = Prism.parse_lex(@source)

def build_processed_source(parse_lex_result)
  RuboCop::AST::ProcessedSource.new(
    @source,
    3.4,
    __FILE__,
    parser_engine: 'parser_prism',
    prism_result: parse_lex_result
  )
end

Benchmark.ips do |x|
  x.report('source')                { build_processed_source(nil) }
  x.report('Prism::ParseLexResult') { build_processed_source(@parse_lex_result) }

  x.compare!
end
```

```console
$ bundle exec ruby reusable_ast.rb
ruby 3.4.2 (2025-02-15 revision d2930f8e7a) +PRISM [x86_64-darwin23]
Warming up --------------------------------------
              source   116.000 i/100ms
Prism::ParseLexResult
                       151.000 i/100ms
Calculating -------------------------------------
              source      1.144k (± 8.4%) i/s -      5.684k in   5.018270s
Prism::ParseLexResult
                          1.460k (± 5.2%) i/s -      7.399k in   5.082102s

Comparison:
Prism::ParseLexResult:     1460.3 i/s
              source:     1144.4 i/s - 1.28x  slower
```

Achieving 1.3x speedup with such this simple modification is a significant improvement for Ruby LSP and its users.

## Compatibility

By using `parser_class.instance_method(:initialize).parameters.assoc(:key)`,
the implementation checks whether `Prism#initialize` supports the `:parser` keyword.
If it does not, the system falls back to the conventional parsing method.
This ensures backward compatibility with previous versions of Prism.

## Development Notes

Since this feature is specifically designed for Prism, the keyword name `prism_result` is meant for Prism.
While it may be possible to achieve similar functionality with the Parser gem, there are currently no concrete use cases.
Therefore, for clarity, we have chosen `prism_result` as the keyword.

Additionally, Prism has two types of results: `Prism::ParseResult` and `Prism::ParseLexResult`,
but the `prism_result` keyword argument is meant to receive `Prism::ParseLexResult`.
Since `prism_parse_lex_result` would be too long, the name `prism_result` was chosen to align with the superclass `Prism::Result`.
@koic koic force-pushed the support_custom_parser_in_prism_translation_paser branch from 986bfbf to fe1dfc1 Compare February 25, 2025 04:16
koic added a commit to koic/rubocop that referenced this pull request Feb 25, 2025
Follow-up to Shopify/ruby-lsp#1849

This feature utilizes ruby/prism#3478 and rubocop/rubocop-ast#359
to implement Shopify/ruby-lsp#1849.

As described in rubocop/rubocop-ast#359,
it speeds up the processing around `Prism::Translation::Parser` via `ProcessedSource` by 1.3x when using the Ruby LSP add-on.
koic added a commit to koic/rubocop that referenced this pull request Feb 25, 2025
Follow-up to Shopify/ruby-lsp#1849

This feature utilizes ruby/prism#3478 and rubocop/rubocop-ast#359
to implement Shopify/ruby-lsp#1849.

As described in rubocop/rubocop-ast#359,
it speeds up the processing around `Prism::Translation::Parser` via `ProcessedSource` by 1.3x when using the Ruby LSP add-on.
koic added a commit to koic/rubocop-ast that referenced this pull request Feb 25, 2025
Follow-up to Shopify/ruby-lsp#1849.

This feature utilizes ruby/prism#3478 to implement Shopify/ruby-lsp#1849.

By using ruby/prism#3478, this feature enables performance improvements by reusing Prism's parsed results
instead of parsing the source code. Below is a sample code snippet, which is expected to improve performance by
1.3x in this case.

```ruby
#!/usr/local/bin/ruby

require 'benchmark/ips'
require 'prism'
require 'rubocop-ast'

@source = File.read(__FILE__)
@parse_lex_result = Prism.parse_lex(@source)

def build_processed_source(parse_lex_result)
  RuboCop::AST::ProcessedSource.new(
    @source,
    3.4,
    __FILE__,
    parser_engine: 'parser_prism',
    prism_result: parse_lex_result
  )
end

Benchmark.ips do |x|
  x.report('source')                { build_processed_source(nil) }
  x.report('Prism::ParseLexResult') { build_processed_source(@parse_lex_result) }

  x.compare!
end
```

```console
$ bundle exec ruby reusable_ast.rb
ruby 3.4.2 (2025-02-15 revision d2930f8e7a) +PRISM [x86_64-darwin23]
Warming up --------------------------------------
              source   116.000 i/100ms
Prism::ParseLexResult
                       151.000 i/100ms
Calculating -------------------------------------
              source      1.144k (± 8.4%) i/s -      5.684k in   5.018270s
Prism::ParseLexResult
                          1.460k (± 5.2%) i/s -      7.399k in   5.082102s

Comparison:
Prism::ParseLexResult:     1460.3 i/s
              source:     1144.4 i/s - 1.28x  slower
```

Achieving 1.3x speedup with such this simple modification is a significant improvement for Ruby LSP and its users.

## Compatibility

By using `parser_class.instance_method(:initialize).parameters.assoc(:key)`,
the implementation checks whether `Prism#initialize` supports the `:parser` keyword.
If it does not, the system falls back to the conventional parsing method.
This ensures backward compatibility with previous versions of Prism.

## Development Notes

Since this feature is specifically designed for Prism, the keyword name `prism_result` is meant for Prism.
While it may be possible to achieve similar functionality with the Parser gem, there are currently no concrete use cases.
Therefore, for clarity, we have chosen `prism_result` as the keyword.

Additionally, Prism has two types of results: `Prism::ParseResult` and `Prism::ParseLexResult`,
but the `prism_result` keyword argument is meant to receive `Prism::ParseLexResult`.
Since `prism_parse_lex_result` would be too long, the name `prism_result` was chosen to align with the superclass `Prism::Result`.
Follow-up to Shopify/ruby-lsp#1849.

This is an extension of `Prism::Translation::Parser` to implement Shopify/ruby-lsp#1849.
It is based on the comments in Shopify/ruby-lsp#1849 (review),
but also adds a default argument for delegation to `Parser::Base` super class.

Using this API, rubocop/rubocop-ast#359 has been implemented in RuboCop AST.
As detailed in rubocop/rubocop-ast#359, this change is expected to improve performance by 1.3x
for some source code.
Achieving a 1.3x speedup with such this simple modification is a significant improvement for Ruby LSP and its users.
@koic koic force-pushed the support_custom_parser_in_prism_translation_paser branch from fe1dfc1 to 9257252 Compare February 25, 2025 04:50
koic added a commit to koic/rubocop-ast that referenced this pull request Feb 25, 2025
Follow-up to Shopify/ruby-lsp#1849.

This feature utilizes ruby/prism#3478 to implement Shopify/ruby-lsp#1849.

By using ruby/prism#3478, this feature enables performance improvements by reusing Prism's parsed results
instead of parsing the source code. Below is a sample code snippet, which is expected to improve performance by
1.3x in this case.

```ruby
#!/usr/local/bin/ruby

require 'benchmark/ips'
require 'prism'
require 'rubocop-ast'

@source = File.read(__FILE__)
@parse_lex_result = Prism.parse_lex(@source)

def build_processed_source(parse_lex_result)
  RuboCop::AST::ProcessedSource.new(
    @source,
    3.4,
    __FILE__,
    parser_engine: 'parser_prism',
    prism_result: parse_lex_result
  )
end

Benchmark.ips do |x|
  x.report('source')                { build_processed_source(nil) }
  x.report('Prism::ParseLexResult') { build_processed_source(@parse_lex_result) }

  x.compare!
end
```

```console
$ bundle exec ruby reusable_ast.rb
ruby 3.4.2 (2025-02-15 revision d2930f8e7a) +PRISM [x86_64-darwin23]
Warming up --------------------------------------
              source   116.000 i/100ms
Prism::ParseLexResult
                       151.000 i/100ms
Calculating -------------------------------------
              source      1.144k (± 8.4%) i/s -      5.684k in   5.018270s
Prism::ParseLexResult
                          1.460k (± 5.2%) i/s -      7.399k in   5.082102s

Comparison:
Prism::ParseLexResult:     1460.3 i/s
              source:     1144.4 i/s - 1.28x  slower
```

Achieving 1.3x speedup with such this simple modification is a significant improvement for Ruby LSP and its users.

## Compatibility

By using `parser_class.instance_method(:initialize).parameters.assoc(:key)`,
the implementation checks whether `Prism#initialize` supports the `:parser` keyword.
If it does not, the system falls back to the conventional parsing method.
This ensures backward compatibility with previous versions of Prism.

## Development Notes

Since this feature is specifically designed for Prism, the keyword name `prism_result` is meant for Prism.
While it may be possible to achieve similar functionality with the Parser gem, there are currently no concrete use cases.
Therefore, for clarity, we have chosen `prism_result` as the keyword.

Additionally, Prism has two types of results: `Prism::ParseResult` and `Prism::ParseLexResult`,
but the `prism_result` keyword argument is meant to receive `Prism::ParseLexResult`.
Since `prism_parse_lex_result` would be too long, the name `prism_result` was chosen to align with the superclass `Prism::Result`.
koic added a commit to koic/rubocop that referenced this pull request Feb 25, 2025
Follow-up to Shopify/ruby-lsp#1849

This feature utilizes ruby/prism#3478 and rubocop/rubocop-ast#359
to implement Shopify/ruby-lsp#1849.

As described in rubocop/rubocop-ast#359,
it speeds up the processing around `Prism::Translation::Parser` via `ProcessedSource` by 1.3x when using the Ruby LSP add-on.
@kddnewton kddnewton merged commit c2e0cb1 into ruby:main Feb 25, 2025
59 checks passed
@koic koic deleted the support_custom_parser_in_prism_translation_paser branch February 25, 2025 15:49
Earlopain added a commit to Earlopain/ruby-prism that referenced this pull request Feb 25, 2025
Earlopain added a commit to Earlopain/ruby-prism that referenced this pull request Feb 25, 2025
Caused by ruby#3478 and ruby#3443

I also made the builder reference more explicit to clearly distinquish
between `::Parser` and `Prism::Translation::Parser`
@Earlopain Earlopain mentioned this pull request Feb 25, 2025
koic added a commit to koic/prism that referenced this pull request Feb 25, 2025
ruby#3443 and ruby#3478 conflicted,
resulting in different `initialize` method definitions.
This PR resolves the conflict and updates it to a unified `initialize` method.
matzbot pushed a commit to ruby/ruby that referenced this pull request Feb 25, 2025
Caused by ruby/prism#3478 and ruby/prism#3443

I also made the builder reference more explicit to clearly distinquish
between `::Parser` and `Prism::Translation::Parser`

ruby/prism@d52aaa75b6
koic added a commit to rubocop/rubocop-ast that referenced this pull request Mar 11, 2025
Follow-up to Shopify/ruby-lsp#1849.

This feature utilizes ruby/prism#3478 to implement Shopify/ruby-lsp#1849.

By using ruby/prism#3478, this feature enables performance improvements by reusing Prism's parsed results
instead of parsing the source code. Below is a sample code snippet, which is expected to improve performance by
1.3x in this case.

```ruby
#!/usr/local/bin/ruby

require 'benchmark/ips'
require 'prism'
require 'rubocop-ast'

@source = File.read(__FILE__)
@parse_lex_result = Prism.parse_lex(@source)

def build_processed_source(parse_lex_result)
  RuboCop::AST::ProcessedSource.new(
    @source,
    3.4,
    __FILE__,
    parser_engine: 'parser_prism',
    prism_result: parse_lex_result
  )
end

Benchmark.ips do |x|
  x.report('source')                { build_processed_source(nil) }
  x.report('Prism::ParseLexResult') { build_processed_source(@parse_lex_result) }

  x.compare!
end
```

```console
$ bundle exec ruby reusable_ast.rb
ruby 3.4.2 (2025-02-15 revision d2930f8e7a) +PRISM [x86_64-darwin23]
Warming up --------------------------------------
              source   116.000 i/100ms
Prism::ParseLexResult
                       151.000 i/100ms
Calculating -------------------------------------
              source      1.144k (± 8.4%) i/s -      5.684k in   5.018270s
Prism::ParseLexResult
                          1.460k (± 5.2%) i/s -      7.399k in   5.082102s

Comparison:
Prism::ParseLexResult:     1460.3 i/s
              source:     1144.4 i/s - 1.28x  slower
```

Achieving 1.3x speedup with such this simple modification is a significant improvement for Ruby LSP and its users.

## Compatibility

By using `parser_class.instance_method(:initialize).parameters.assoc(:key)`,
the implementation checks whether `Prism#initialize` supports the `:parser` keyword.
If it does not, the system falls back to the conventional parsing method.
This ensures backward compatibility with previous versions of Prism.

## Development Notes

Since this feature is specifically designed for Prism, the keyword name `prism_result` is meant for Prism.
While it may be possible to achieve similar functionality with the Parser gem, there are currently no concrete use cases.
Therefore, for clarity, we have chosen `prism_result` as the keyword.

Additionally, Prism has two types of results: `Prism::ParseResult` and `Prism::ParseLexResult`,
but the `prism_result` keyword argument is meant to receive `Prism::ParseLexResult`.
Since `prism_parse_lex_result` would be too long, the name `prism_result` was chosen to align with the superclass `Prism::Result`.
koic added a commit to koic/rubocop that referenced this pull request Mar 16, 2025
Follow-up to Shopify/ruby-lsp#1849

This feature utilizes ruby/prism#3478 and rubocop/rubocop-ast#359
to implement Shopify/ruby-lsp#1849.

As described in rubocop/rubocop-ast#359,
it speeds up the processing around `Prism::Translation::Parser` via `ProcessedSource` by 1.3x when using the Ruby LSP add-on.
kddnewton pushed a commit to kddnewton/ruby that referenced this pull request Mar 18, 2025
Caused by ruby/prism#3478 and ruby/prism#3443

I also made the builder reference more explicit to clearly distinquish
between `::Parser` and `Prism::Translation::Parser`

ruby/prism@d52aaa75b6
kddnewton pushed a commit to ruby/ruby that referenced this pull request Mar 18, 2025
Caused by ruby/prism#3478 and ruby/prism#3443

I also made the builder reference more explicit to clearly distinquish
between `::Parser` and `Prism::Translation::Parser`

ruby/prism@d52aaa75b6
koic added a commit to koic/rubocop that referenced this pull request Mar 18, 2025
Follow-up to Shopify/ruby-lsp#1849

This feature utilizes ruby/prism#3478 and rubocop/rubocop-ast#359
to implement Shopify/ruby-lsp#1849.

As described in rubocop/rubocop-ast#359,
it speeds up the processing around `Prism::Translation::Parser` via `ProcessedSource` by 1.3x when using the Ruby LSP add-on.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants