Introduce caching to circumvent issue with using UserAgentParser.parse#13
Closed
hexgnu wants to merge 1 commit intoua-parser:masterfrom
Closed
Introduce caching to circumvent issue with using UserAgentParser.parse#13hexgnu wants to merge 1 commit intoua-parser:masterfrom
hexgnu wants to merge 1 commit intoua-parser:masterfrom
Conversation
Contributor
|
You don't think the two mentions in the Readme is sufficient? and: |
Author
|
Yea I realized very quickly and moved over to loading a constant. I just personally felt that |
Contributor
|
Yeah, I understand. It's just implementing our own instance cache feels like a problem that shouldn't be solved in a gem that parses user agent strings. And I'm not sure of the side-effects of adding a cache. Perhaps we could just ditch the class method? |
Author
|
I am 100% for ditching the class method. |
Contributor
|
Cool, I've opened #14 to remove the class method on next major revision bump. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
So it seems to be a pretty recurring issue based on previous PR's that
UserAgentParser.parsecan cause performance issues. I found this out the hard way by loading 30 million user agent strings and not realizing that was how it was defined.This PR aims to help by introducing a cache that allows users to still rely on the same functionality but enjoy the much faster speed.
How to test
I threw a few cache specific tests in there but also made sure the tests themselves still worked.