Bug 203544 - [FreeType] Too slow running encoding/legacy-mb-korean/euc-kr WPT tests
Summary: [FreeType] Too slow running encoding/legacy-mb-korean/euc-kr WPT tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-10-28 17:21 PDT by Carlos Alberto Lopez Perez
Modified: 2019-11-05 00:18 PST (History)
5 users (show)

See Also:


Attachments
Patch (10.37 KB, patch)
2019-10-31 01:54 PDT, Carlos Garcia Campos
clopez: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2019-10-28 17:21:04 PDT
The WPT tests encoding/legacy-mb-korean/euc-kr take too much in WebKitGTK. Like 5x more times than with other browsers.

An example of such test is: https://w3c-test.org/encoding/legacy-mb-korean/euc-kr/euckr-decode-ksc5601.html

On the WebKitGTK MiniBrowser it takes around 2 minutes to execute, but on other browsers (firefox, chrome) it executes in less than 15 seconds.

Note that the issue is only reproducible from a cold run. If the page is reloaded, then the second time it loads faster.


Related: https://github.com/web-platform-tests/wpt/issues/19961#issuecomment-547200149
Comment 1 Carlos Alberto Lopez Perez 2019-10-28 18:03:19 PDT
I have run perf over the WebProcess and it seems the biggest issue is by far the call to FcFontMatch inside FontCache::systemFallbackForCharacters()

Another test is just loading this page https://w3c-test.org/encoding/legacy-mb-korean/euc-kr/euckr_chars-ksc5601.html and checking that it draws the characters correctly and the time it takes to load.
Comment 2 Carlos Alberto Lopez Perez 2019-10-28 18:57:51 PDT
Looking at the blame log it looked like r229164 could be related. So I checked to build WebKitGTK on r229163 and r229164
and check the load times of https://w3c-test.org/encoding/legacy-mb-korean/euc-kr/euckr_chars-ksc5601.html
(from a cold start and loading the html from disk to avoid network latencies)


r229163  7 seconds
r229164 13 seconds
r251384 30 seconds


So, r229164 caused the first regression of almost a 100%, and this further regressed more after that.
I still don't know which revision(s) caused the second regression, probably its worth bisecting.
Comment 3 Carlos Alberto Lopez Perez 2019-10-28 21:15:54 PDT
(In reply to Carlos Alberto Lopez Perez from comment #2)
> I still don't know which revision(s) caused the second regression, probably
> its worth bisecting.

The one that caused this second regression (from 13 seconds to 30 seconds) it seems it was r230559

I can get the 13 seconds performance back by running minibrowser (on r251384) with the env var WEBKIT_FORCE_COMPLEX_TEXT=0
Comment 4 Carlos Garcia Campos 2019-10-31 01:54:51 PDT
Created attachment 382440 [details]
Patch
Comment 5 Carlos Alberto Lopez Perez 2019-10-31 08:46:44 PDT
Comment on attachment 382440 [details]
Patch

r=me
Amazing patch! Now the test takes just only 3 seconds. At least a 10x improvement! :)
Comment 6 Carlos Garcia Campos 2019-11-05 00:18:05 PST
Committed r252044: <https://trac.webkit.org/changeset/252044>