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

Decouple UA analyzer and parser to allow caching #109

Merged
merged 1 commit into from
Aug 24, 2016

Conversation

romainneutron
Copy link
Collaborator

While using this browser_adaptive feature, I realized it consumes quite a lot of CPU.
This PR decouple analyzer and parser so that cache can be used

->booleanNode('enabled')
->defaultFalse()
->end()
->scalarNode('service')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

might be parser instead of service

@romainneutron
Copy link
Collaborator Author

Don't forget to update README

@romainneutron romainneutron force-pushed the cached-ua-parser branch 2 times, most recently from 7ee18f2 to 9acd532 Compare July 13, 2016 09:34

$name = $this->parser->getUaFamily($userAgent);

if (null !== $item) {
Copy link

Choose a reason for hiding this comment

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

This will store the result only when there was an exception, but not when there was no hit.

@romainneutron romainneutron force-pushed the cached-ua-parser branch 2 times, most recently from 00ff590 to 2ff8c36 Compare August 23, 2016 11:20
@romainneutron
Copy link
Collaborator Author

Just pushed a final version with README updated. Comments are welcome

@romainneutron romainneutron merged commit 2eda4c5 into nelmio:master Aug 24, 2016
@romainneutron romainneutron deleted the cached-ua-parser branch August 24, 2016 11:04
private $lifetime;
private $prefix;

public function __construct(CacheItemPoolInterface $cache, UAFamilyParser $parser, $lifetime = 0, $prefix = 'nelmio-ua-parser-')
Copy link

Choose a reason for hiding this comment

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

Should this not type-hint the interface instead?

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.

None yet

2 participants