Switch to DeviceDetector 2.X #305
Conversation
sgiehl
commented
Jun 8, 2014
sgiehl
commented
- Switched to DeviceDetector 2.X
- Do detection only once (in tracker/settings). There is no need doing device type detection in DeviceDetection plugin as all those detections are already done before (Doing them twice slows down tracking)
- Introduce new DeviceDetectorCache to use an optimal caching for DeviceDetector (File Cache + Static Caching)
- Speed up detection for bulk requests (thx to @cbonte)
- Use special parsers where only those are needed (e.g. check for bots or mobile os)
Conflicts: core/Tracker/Visit.php
Conflicts: core/Tracker/Settings.php plugins/API/API.php
…or. This cache combines file and static caching to speed up detections as much as possible
By caching parsed DeviceDetectors, we reduce the number of regexes to process. It allows to process more requests per second when the same user agent is met.
The previous commit had at least 2 issues : one identified by sgiehl, who reported that the static variable was useless if we always create a new cache instance, and indeed, he's absolutely right ! Another one can be classified as a potential security issue, where a request can forge a user agent as a key to an object which is not meant to be accessed (CacheStatic instances share their data).
Device detector2 cache
…detector based on the given user agent. That provides the possibility to reuse DeviceDetector for bot and visitor data detection.
|
@sgiehl Tests are in progress. From the first results, It seems it's a little better but I can't import more than 800-810req/s (which is still quite good compared to the results without any cache). I'm making some more tests but I think it's because with the factory, we're calling the parse() method for everything and not only for bots. |
|
DeviceDetector skips parsing additional data when a bot is detected. So it |
|
Yes, I saw the tests after posting. |
Conflicts: plugins/DevicesDetection/DevicesDetection.php
| @@ -0,0 +1,65 @@ | |||
| <?php | |||
There was a problem hiding this comment.
Maybe files core/DeviceDetector* should be in plugins/DevicesDetection/ ?
There was a problem hiding this comment.
Thought about that, too. But the classes using those files are mostly located in Core.
There was a problem hiding this comment.
@tsteur is working on this in this branch: https://github.com/piwik/piwik/tree/report_and_dimension_refactoring
In this branch. all reports and custom columns will be put in their own plugins after this huge refactor.
After Thomas merges master in his branch, then the code regarding DevicesDetection should be all (or almost all) in the plugin itself.
There was a problem hiding this comment.
If it makes sense after the refactoring, it would be the best solution if he moves it there then
|
That was a great Pull request... well done Stefan, you're the man! Merging now... feel free to make edits as you see fit! |
