Optimize offsetGet

[kamil-tekiela]

We don't actually need to call getCharLength() if we inline the operation and use mb_substr with substr. This trick fetches 4 bytes from the current byte offset because UTF-8 characters are at most 4 bytes. mb_substr will then read the first Unicode character from the selected 4 bytes.

The previous technique was actually brilliant. The ASCII map optimization is the fastest way to calculate it, but it also is quite long. This technique would be perfect if PHP had a way to avoid ord() call altogether. I benchmarked 3 different techniques I could come up with. The ASCII map is the fastest. But actually, the worst thing for performance is a function call. So inlining the long implementation wouldn't be wise. That's why I decided to use the mb_substr() technique.

Real-life benchmarks:
Before:

After:

As you can see this is a microoptimization, so I think we shouldn't be so worried about which one is the fastest. The difference in my benchmark is only 4ms with 10k calls.

What do you think? Should I add a code comment explaining how this works?

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.02% ⚠️

Comparison is base (b737500) 96.46% compared to head (be2ca97) 96.45%.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #506      +/-   ##
============================================
- Coverage     96.46%   96.45%   -0.02%     
+ Complexity     2179     2170       -9     
============================================
  Files            66       66              
  Lines          5065     5045      -20     
============================================
- Hits           4886     4866      -20     
  Misses          179      179              

Files Changed	Coverage Δ
src/Tools/CustomJsonSerializer.php	0.00% <ø> (ø)
src/UtfString.php	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.

📢 Have feedback on the report? Share it here.

[MauricioFauth]

Should I add a code comment explaining how this works?

Definitely.

[williamdes]

Can you benchmark the function and set a baseline please ?

[kamil-tekiela]

Should it not remain the same? How do I run this benchmark?

[williamdes]

Well, since it's a more complete function and the function was removed maybe everything should be updated then
But having a benchmark data could help

[kamil-tekiela]

I added a new commit. Is this what you had in mind? These are the results I get:

    benchBuildUtfString.....................I19 ✔ Mo6.052ms (±3.59%)
    benchUtfStringRandomAccessWithUnicode...I19 ✔ Mo63.459μs (±10.50%)

[williamdes]

Yes, that's good to have checked at all times.
For the results I do not know, if the benchmark was added before we could compare but that's totally okay never mind :)

[williamdes]

What do you think? Should I add a code comment explaining how this works?

Could we keep the previous optimisation that should normally be the fastest one and keep yours to avoid calling ord ?

[kamil-tekiela]

What do you think? Should I add a code comment explaining how this works?

Could we keep the previous optimisation that should normally be the fastest one and keep yours to avoid calling ord ?

No, because it's either this or that. To use the ASCII map optimization one must use ord(). My solution is only slightly faster now because I replace two function calls ord() and getCharLength() with another two substr() and mb_substr() while avoiding any ifs. If there is a way to do this using only one function call or less then that would be the best solution.

But maybe I misunderstood what you mean. What exactly are you proposing? Do you see a way to merge these two solutions together?

[williamdes]

No, because it's either this or that. To use the ASCII map optimization one must use ord(). My solution is only slightly faster now because I replace two function calls ord() and getCharLength() with another two substr() and mb_substr() while avoiding any ifs. If there is a way to do this using only one function call or less then that would be the best solution.

But maybe I misunderstood what you mean. What exactly are you proposing?

Indeed, I misunderstood the new solution
But it feels like it requires much more functions, is there no way to avoid using some of them ?
Not sure if's me over optimizing the solution ^^

Do you see a way to merge these two solutions together?

Not really because of the use of delta and other things that seems to advance the string much more quickly

Maybe phpbench could show how this is quick ?

[kamil-tekiela]

But it feels like it requires much more functions, is there no way to avoid using some of them ?

Yeah it does. I wish there was a way to avoid it but I can't think of any. Good thing that strlen isn't a function call but a dedicated opcode.

[williamdes]

Looks great !