To fix #10853, a new flag should be added if the UTF-8 validity was checked or not, otherwise invalid UTF-8 string will need to be check on each call.

This looks great to me. Thanks.

The table-driven state machine for UTF-8 validity checking is very interesting. I'm interested to know how it compares when benchmarked against the 'fallback' UTF-8 validity checking implementation in mbstring (which was borrowed from PCRE).

To fix #10853, a new flag should be added if the UTF-8 validity was checked or not, otherwise invalid UTF-8 string will need to be check on each call.

👍

It looks like availability of bits in the object header should not be a problem... type_info, where the GC flags are kept, is 32 bits wide, and it looks like there are currently only 9 flags for zend_string objects.

I wrote a test script https://gist.github.com/youkidearitai/566e348e2e23301063ef5a95579d4efd(I put to ext/mbstring/tests) that this PR works fine. Would you like add this .phpt file?

@alexdowad Did you have time benchmarking the two implementations? If either implementation is faster it might make sense to move that one to core and use it from mbstring. I'll also check whether we can see a performance hit during opcache persisting.

@alexdowad Did you have time benchmarking the two implementations?

Hi @iluuu1994. Sorry for the delay. I just put together a lil' benchmark program which runs both functions on a bunch of random strings and compares how long they take.

Using gcc with no optimization, the function from PCRE is somewhat faster. With gcc -O3... Bjoern's function is so ridiculously fast that it's hard to believe. I am just investigating.

Looks like compiler must have been optimizing calls to Bjoern's function out because the result was not used.

After making sure the result is used, the PCRE function is consistently faster.

Looks like @iluuu1994 would do better to use mb_fast_check_utf8_default from mbstring, or mb_fast_check_utf8_avx2 in cases where it can be used.

We already have a mechanism whereby Zend core calls into some mbstring function through function pointers which are set via zend_multibyte_set_functions... but I guess you won't want to use that for this purpose, because then you could only use the UTF-8 checking functions if mbstring extension is present.

I think you should go ahead and move both mb_fast_check_utf8_{default,avx2} into core, rename them (perhaps change mb_ to zend_), and expose them to callers in extension modules.

Maybe also rename _fast_check_ to just _check_.

Thank you @alexdowad for your investigation! Moving the functions into core sounds good to me. I'll do that and check if there are any performance regressions for startup/compilation.