parsica-php / parsica

Parsica - PHP Parser Combinators - The easiest way to build robust parsers.
https://parsica-php.github.io/
MIT License
405 stars 18 forks source link

use static callback functions which improves performance by ~3% #29

Closed staabm closed 3 years ago

staabm commented 3 years ago

inspired by your blackfire session on twitch yesterday, I took a stab at improving performance on parsica.

grafik

https://blackfire.io/profiles/compare/554e2a9c-1726-43a1-b626-95aa2a230949/graph

using static function instead of function reduces overhead of php in callables, because the runtime does not need to introduce a $this object for each function.

turanct commented 3 years ago

Hi, again thanks for this contribution.

when checking this out in phpbench, I saw that it's sometimes a little bit faster than before the change, but sometimes a bit slower as well. On average it's a bit slower in all my test runs.

phpbench benchmark before the change

static-before

phpbench benchmark after the change

static-after

The speed improvement might be a bit marginal to have to write all closures as static, but I'm not sure. What's your opinion on the impact this has for the readability of the code? Should we advise users of the package to write their parsers using static parser functions as well? Your opinion here would be greatly appreciated.

staabm commented 3 years ago

Hey. Thx for your feedback.

In my experience other high performance projects use static callables to speedup hot paths.

Using those in the basic building blocks is a must have IMO. I dont see a downside regarding readability.

On the top levels of the parser it might not be that important but on the hot (especially recursive) paths its really helpful.

I guess we see different overall results because of differences in php versions and opcache.optimization_level

Btw, I would also expect that the php8 jit could lead to major perf differences

turanct commented 3 years ago

Okay that makes sense. I'll try testing on PHP8 and with a few varying opcache settings. Stay tuned.