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

Improve performance #43

Merged
merged 2 commits into from
Jun 3, 2021
Merged

Improve performance #43

merged 2 commits into from
Jun 3, 2021

Conversation

mame
Copy link
Contributor

@mame mame commented May 17, 2021

This pull request contains two essential changes:

  • Use reg.match(str, pos) instead of reg.match(str[pos..-1])
  • Use \G instead of \A

This changes will improve the performance of the generated parser.

This will reduce a few seconds of Ruby's "make install" process
because kpeg is used to generate the markdown parser of RDoc.

This PR includes the change of #42.

mame added 2 commits May 17, 2021 23:32
The commit a742e81 changed only
format_parser.kpeg, but it looks like format_parser.rb was not
re-generated.
... because the former is faster. Note that we need to use `\\G` instead
of `\\A` for regexps.

This trivial performance improvement will reduce a few seconds of Ruby's
"make install" process because kpeg is used to generate the markdown
parser of RDoc.
@evanphx evanphx merged commit 534551b into evanphx:master Jun 3, 2021
aycabta added a commit to aycabta/rdoc that referenced this pull request Aug 9, 2021
I compared the results between the latest release of kpeg gem (1.1.0)
and HEAD of kpeg gem with `time` command, and the execution time is
reduced from 38.35s sec to 35.06 sec.

ref. evanphx/kpeg#43
@aycabta aycabta mentioned this pull request Aug 9, 2021
matzbot pushed a commit to ruby/ruby that referenced this pull request Aug 9, 2021
I compared the results between the latest release of kpeg gem (1.1.0)
and HEAD of kpeg gem with `time` command, and the execution time is
reduced from 38.35s sec to 35.06 sec.

ref. evanphx/kpeg#43

ruby/rdoc@682bcb48ce
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