Skip to content

Fix source.match performance without specifying term string#186

Merged
kou merged 2 commits intoruby:masterfrom
tompng:fix_source_match_performance
Aug 1, 2024
Merged

Fix source.match performance without specifying term string#186
kou merged 2 commits intoruby:masterfrom
tompng:fix_source_match_performance

Conversation

@tompng
Copy link
Copy Markdown
Member

@tompng tompng commented Jul 30, 2024

Performance problem of source.match(regexp) was recently fixed by specifying terminator string. However, I think maintaining appropriate terminator string for a regexp is hard.
I propose solving this performance issue by increasing bytes to read in each iteration.

Comment thread lib/rexml/source.rb Outdated
Comment on lines 250 to 252
# Note: When specifying a string for 'pattern', it must not include '>' except in the following formats:
# - ">"
# - "XXX>" (X is any string excluding '>')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove this outdated comment?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed 👍

Comment thread lib/rexml/source.rb
# - ">"
# - "XXX>" (X is any string excluding '>')
def match( pattern, cons=false, term: nil )
def match( pattern, cons=false )
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style. Very minor: the spaces around the argument list does not match the other methods in the file.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. We should use a consistent coding style in the same code base. But it's out-of-scope of this pull request. We should work on it separately.

@kou kou merged commit 6cac15d into ruby:master Aug 1, 2024
@kou
Copy link
Copy Markdown
Member

kou commented Aug 1, 2024

Thanks.

@tompng tompng deleted the fix_source_match_performance branch August 1, 2024 03:22
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.

3 participants