whitequark / parser

A Ruby parser.
Other
1.59k stars 199 forks source link

Extract even more actions into methods #901

Closed iliabylich closed 9 months ago

iliabylich commented 1 year ago

Continuation of https://github.com/whitequark/parser/pull/899.

cc @headius @eregon

For me locally on MRI it doesn't really change anything in terms of performance. I've extracted even more actions into methods.

The problem is that Lexer#advance is still huge. You can clone this branch locally and check what gets compiled. Here's a tiny script to show which actions are repeated most of all:

# make sure to run "rake generate"

compiled = File.read('lib/parser/lexer-F1.rb').lines
src = File.read('lib/parser/lexer.rl').lines

compiled
  .select { |l| l.include?('"lib/parser/lexer.rl"') }
  .map { |l| l[/\d+/].to_i }
  .tally
  .to_a
  .sort_by(&:last)
  .reverse
  .each { |lineno, count| puts "#{count} (lexer.rl:#{lineno})"; puts src[lineno - 1..lineno + 5].join; puts '-' * 100 }

It prints top repeated actions, the number of times they get emitted, location in original lexer.rl and first 5 lines of the action. So, the first { ... ruby code ... } part that you'll see is the action itself.

I've extracted (as much as I can) all actions that are repeated 5 or more times. The size of Lexer#advance is still too big I suppose.

@headius @eregon May I ask you to take a look at this branch and check if it's enough? If it's not enough I just don't know what to do. And if so I'll probably revert https://github.com/whitequark/parser/pull/899 because this approach clearly doesn't work and only makes code less maintainable.

eregon commented 1 year ago

Thank you for this! Please see https://github.com/whitequark/parser/issues/871#issuecomment-1362199891