search

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

StringStream: remember strlen to speedup isEOF() which gains ~1,5% performance #32

Closed staabm closed 3 years ago

staabm commented 3 years ago

grafik

turanct commented 3 years ago

Hi!

First of all, thanks so much for your contribution, it's greatly appreciated.

I just checked this out locally and ran the phpbench benchmarks. With your changes, I indeed see an improvement of ± 1.5% in performance. However, this made me wonder if we really needed mb_strlen() to just know if we are at EOF.

I replaced mb_strlen() with the normal strlen() in the original code (on main branch) and I saw a performance improvement of ±40% in the JSON parser. Doing the same thing as this PR does (calculating length in the constructor) using stlen() gives a similar performance gain, but it's a little less performant because we then do it too often, more than we need.

So all in all, thank you so much for this contribution. It made me look in the correct place. I'm going to close this PR, create a new PR with the change in it, add you as a Co-Author and let you review it before I merge, if you're fine with that.

turanct commented 3 years ago

Closing because this initiated #33 which fixes this even better 🚀