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 faster matching #36

Merged
merged 4 commits into from
Sep 11, 2018
Merged

use faster matching #36

merged 4 commits into from
Sep 11, 2018

Conversation

filiptepper
Copy link
Contributor

Faster matching:

  • accessing Regexp using patterns[:regex] is slightly faster than patterns['regex'] (0.05 performance increase),
  • Ruby 2.4.0 introduced match? for faster regular expression matching, using match? to verify regular expression and then calling match to return matches is faster than using match for every iteration (0.32 performance increase).
Warming up --------------------------------------
               match   125.000  i/100ms
              match?   166.000  i/100ms
Calculating -------------------------------------
               match      1.266k (± 2.1%) i/s -      6.375k in   5.037348s
              match?      1.729k (± 2.1%) i/s -      8.798k in   5.090618s

Comparison:
              match?:     1729.1 i/s
               match:     1266.1 i/s - 1.37x  slower

@filiptepper
Copy link
Contributor Author

@wallin Would you mind having a look?

@wallin
Copy link
Contributor

wallin commented Sep 6, 2018

Thanks for finding this @filiptepper. Could you perhaps make the method definition dynamically based on the version instead of having a conditional inside the method?

@filiptepper
Copy link
Contributor Author

@wallin Something like 089083c will work?

@wallin
Copy link
Contributor

wallin commented Sep 11, 2018

Looks good @filiptepper. Should we also check for JRuby >9.2.0.0?

@filiptepper
Copy link
Contributor Author

@wallin I added it, since it changes MRI Ruby compatibility to 2.5.

@wallin wallin merged commit 46a9eb1 into ua-parser:master Sep 11, 2018
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