Bug #81378 [Asn]: mb_detect_encoding() performance regression in PHP 8.1
Edit report at https://bugs.php.net/bug.php?id=81378&edit=1
ID: 81378
Updated by: alexdowad@php.net
Reported by: gfpuba+php at gmail dot com
Summary: mb_detect_encoding() performance regression in PHP
8.1
Status: Assigned
Type: Bug
Package: Scripting Engine problem
Operating System: Win10 Apache/2.4.48
PHP Version: 8.1.0beta3
Assigned To: alexdowad
Block user comment: N
Private report: N
New Comment:
Checking for fixed ranges of codepoints rather than doing Unicode property lookups via hash table
makes mb_detect_encoding a bit less than 4x faster. Redirecting to mb_check_encoding under the hood
gives another 5% gain. (Wonder if that's even worth doing? It might be if the
mb_detect_encoding algorithm becomes more complex in the future to achieve more accurate
detection...)
Previous Comments:
------------------------------------------------------------------------
[2021-08-25 08:41:09] nikic@php.net
> Dear nikic, thanks for sending this report my way. I'm not sure if you received the e-mail
> I sent you recently about mbstring, since there was no reply. Anyways, to reiterate what I said in
> the e-mail, I am just trying to get test coverage for mbstring (as reported by gcov) close to 100%,
> before doing any more cleanup or performance work.
Sent a reply!
> After test coverage is better, I will be happy to work on this. Your point about
> mb_detect_encoding() with a single candidate encoding is good, and suggests we could internally
> detect that case and delegate to mb_check_encoding(). Do you think that would be worthwhile?
Sounds reasonable to me. The strict=true case at least is mb_check_encoding() while strict=false ...
I think that should basically be an unconditional "return true", but I think currently
that checks whether the first character of the string is valid in the encoding.
------------------------------------------------------------------------
[2021-08-24 19:55:54] alexdowad@php.net
Dear nikic, thanks for sending this report my way. I'm not sure if you received the e-mail I
sent you recently about mbstring, since there was no reply. Anyways, to reiterate what I said in the
e-mail, I am just trying to get test coverage for mbstring (as reported by gcov) close to 100%,
before doing any more cleanup or performance work.
After test coverage is better, I will be happy to work on this. Your point about
mb_detect_encoding() with a single candidate encoding is good, and suggests we could internally
detect that case and delegate to mb_check_encoding(). Do you think that would be worthwhile?
------------------------------------------------------------------------
[2021-08-24 19:30:53] nikic@php.net
I've applied a couple of obvious optimizations in:
* https://github.com/php/php-src/commit/3be94217f4c353e718db6d823dbd74a29522fce3
* https://github.com/php/php-src/commit/f458b16041b6c9101d2f846027aba4a4b08e5a50
* https://github.com/php/php-src/commit/425c2e3ba1e3096f26adfef70f8afac9ae835ba9
There's more that can be done here, though I'm not sure how final the
"algorithm" here is.
Maybe Alex wants to take a look as well...
------------------------------------------------------------------------
[2021-08-24 13:35:32] nikic@php.net
Encoding detection now scores encodings based on character properties, and the property lookups are
very expensive and make up the majority of the execution time now.
We don't actually need them for the case of a single encoding, but I assume that was just a
minimal reduction, because calling mb_detect_encoding() with a single encoding doesn't make
sense (mb_check_encoding() should be used instead).
------------------------------------------------------------------------
[2021-08-23 20:10:41] gfpuba+php at gmail dot com
Description:
------------
The following code takes 10 times longer to execute on PHP/8.1.0beta3 compare to PHP 8.0.8
Test script:
---------------
$a = str_repeat('abcdef', 10000);
$b = mb_detect_encoding($a, 'UTF-8', true);
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=81378&edit=1
Thread (7 messages)