Closed frankmayer closed 5 years ago
Some of the regex stuff absolutely makes sense and should be merged.
However, running a benchmark script I wrote on my local copy of PHP 7.1 your version is consistently coming out just a little slower.
I'm honestly not sure what's causing the difference.
If you care to research my benchmark script is below.
It runs through all the user agents in my test suite 10,000 times - feel free to experiment with the script below yourself.
<?php
require 'Source/UserAgentParser.php';
$uas = json_decode(file_get_contents(__DIR__ . '/tests/user_agents.json'), true);
$time = microtime(true);
for($i = 0; $i <= 10000; $i++){
if($i % 1000 == 0) {
echo ($i / 1000) . ' ';
}
foreach($uas as $ua => $expected_result) {
$result = parse_user_agent($ua);
}
}
echo "\n";
echo microtime(true) - $time;
echo "\n";
Hmmm, yes. I get the same results. With the modifications, it's slightly slower...
I'll take a look
@frankmayer I'm jealous of your CPU and how much quicker it ran for you ;)
So, I reverted some commits and left only those, that did not do any harm and, in the case of the one Regex, performed a little bit better. The rest should be revisited as soon as PHP gets its JIT Compiler...
@donatj Yeah, the new CPU was quite an upgrade ;) I am very pleased with my Ryzen 1700. BTW, with PHP 7.2 the time of the benchmark was about 35s. That's quite a nice improvement (~12%) for a minor version upgrade.
@donat are you still interested in the rest of the optimizations or should I close this? (I am on a cleaning spree :smile: )
You can probably close it. It never proved reliably quicker on benchmarks and I'm in the middle of something of a rewrite anyway.
I do however really appreciate the contribution!
On Tue, Aug 7, 2018, 4:06 PM Frank Mayer notifications@github.com wrote:
@donat https://github.com/donat are you still interested in the rest of the optimizations or should I close this? (I am on a cleaning spree 😄 )
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/donatj/PhpUserAgent/pull/46#issuecomment-411202633, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIKc7-URfaG4wPAniZcz7KXIMEMiFUEks5uOgFhgaJpZM4Q4FHp .
ok, closing this. Looking forward to your rewrite. Thanks
Hi Jesse,
Just a small contribution. Some micro optimizations and stricter comparisons.
Thank you for your work. :+1: