WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
36080
Performance degradations when using large javascript files
https://bugs.webkit.org/show_bug.cgi?id=36080
Summary
Performance degradations when using large javascript files
David Leong
Reported
2010-03-12 17:26:44 PST
Created
attachment 50644
[details]
Test cases We have noticed that using large javascript files results in a degradation in performance. Our use case contains a HTML page which uses with a large (>1meg) javascript file. Performance of any javascript class/function in this file is much slower than having the code split into smaller ~100k file chunks. On mobile devices we have found that that up to 70% of the time when executing javascript is doing character conversion and byteOrderMark checking. After some investigation, we found that two problems: 1) WebCore::CachedScript::script() deletes decoded data with a timer callback of 0s. If the script file is large this causes the data to be repeatedly re-parsed many times when javascript core only wants small sections of the code. 2) JSC::Lexer::setCode() is looking for the byteOrderMark for every character that javascript core is interested in. For large script files this results in many overlapping sections that have been checked before for the byte mark. We've made a simple test page to reproduce the problem. The page is based on the V8 benchmark with lots of text inserted as comments. Here are some statistics from running v8 on my desktop to showcase the problem: Tests done on a Core i7 920, 3gigs of ram, webkit trunk, running QGVLauncher and repeated several times. First, I ran the V8 test with no code changes. Next, I added lots of text into the V8 test scripts Finally, I disabled the byteOrderMark checking in JSC::Lexer and disabled the script delete timer in WebCore::CachedScript to test the performance with the workaround. V8 baseline: V8 modified: V8 modified workaround: Score: 977 806 975 Richards: 3314 2812 3279 DeltaBlue: 831 608 841 Crypto: 2211 1891 2238 RayTrace: 1193 993 1200 EarleyBoyer: 571 480 586 RegExp: 210 178 197 I am reworking the code to do a single pass BOM checking inside of cachedScript. Also, I am thinking of adding a setting to change the timeout before CachedScript class will delete the data. Any feedback and comments would be apprecitated!
Attachments
Test cases
(369.83 KB, application/x-zip-compressed)
2010-03-12 17:26 PST
,
David Leong
no flags
Details
First cut at the changes
(3.97 KB, patch)
2010-03-12 17:56 PST
,
David Leong
no flags
Details
Formatted Diff
Diff
Sunspider results with very large script file.
(2.70 KB, application/octet-stream)
2010-03-22 13:36 PDT
,
David Leong
no flags
Details
Test cases for sunspider. Baseline and modified with lots of text in the source code.
(1.04 MB, application/octet-stream)
2010-03-22 13:37 PDT
,
David Leong
no flags
Details
First stab at a patch.
(7.69 KB, patch)
2010-03-24 15:32 PDT
,
David Leong
eric
: review-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
David Leong
Comment 1
2010-03-12 17:56:03 PST
Created
attachment 50646
[details]
First cut at the changes I need to fix my cygwin environment and get the create webkit patch scripts going.
Alexey Proskuryakov
Comment 2
2010-03-12 22:58:17 PST
I suggest tracking these two issues in separate bugs. When you have a patch for review, please also provide sunspider results - a reviewer will definitely want these. Although they'll probably be more interested in Mac results, Windows ones should be helpful as a first approximation.
David Leong
Comment 3
2010-03-16 09:47:26 PDT
Thanks Alexy, I will work on sunspider results as well as create a second bug for the script deletion timer.
David Leong
Comment 4
2010-03-16 09:56:48 PDT
Created
bug 36174
to address problem #1 for the short script deletion timer.
David Leong
Comment 5
2010-03-22 13:35:25 PDT
Created sunspider version of the test case. (logs attached as well) In sunspider, all of the javascript code is embedded in html files. In this case, the CachedScriptSourceProvider class is not involved but StringSourceProvider is used. Moving the byte order mark checking to the string source provider also improves the performance of large script files. Here are the results: Sunspider baseline, no webkit modifications: Total: 2107 3d: 171.4 Access: 64.4 Bitops: 27.2 controlflow: 3.6 crypto: 90.4 date: 451.2 math: 70.6 regexp: 23.2 string: 1205.0 Sunspider baseline, with webkit modifications: Total: 2023.6 3d: 156.0 Access: 59.8 Bitops: 27 controlflow: 3.4 crypto: 86 date: 437.2 math: 70 regexp: 21.6 string: 1162.6 Sunspider modified with large file, no webkit modifications: Total: 2580.8 3d: 299 Access: 114.2 Bitops: 50 controlflow: 14.4 crypto: 196.8 date: 496.2 math: 90 regexp: 20.8 string: 1299.4 Sunspider modified with large file, with webkit modifications: Total: 2059.6 3d: 162.6 Access: 61.2 Bitops: 27.6 controlflow: 3.2 crypto: 87.8 date: 442.8 math: 72.6 regexp: 21.8 string: 1180.0
David Leong
Comment 6
2010-03-22 13:36:29 PDT
Created
attachment 51342
[details]
Sunspider results with very large script file.
David Leong
Comment 7
2010-03-22 13:37:37 PDT
Created
attachment 51343
[details]
Test cases for sunspider. Baseline and modified with lots of text in the source code.
David Leong
Comment 8
2010-03-24 15:32:29 PDT
Created
attachment 51559
[details]
First stab at a patch. This patch makes JSC::SourceProvider::hasBOMs() virtual so sub classes can inherit from it. This method is overwritten for both the CachedScriptSourceProvider and the StringSourceProvider. For the CachedScriptSourceProvider, I delegated the BOM check to the CachedScript as the buffer maybe deleted by the decoded data callback timer. For the StringSourceProvider the BOM check is done when the class is constructed. Both classes store the results of the BOM check so that repeated calls by the JSC::Parser for source code will not repeat the BOM checks. I was thinking should we move the BOM checking to the base class ScriptSourceProvider? In that case how would we deal with the decoded data deletion by the CachedScript. Any feedback or ideas would be greatly appreciated.
WebKit Review Bot
Comment 9
2010-03-24 15:37:49 PDT
Attachment 51559
[details]
did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/loader/CachedScript.h:33: Code inside a namespace should not be indented. [whitespace/indent] [4] WebCore/loader/CachedScript.h:33: Missing space before { [whitespace/braces] [5] WebCore/bindings/js/StringSourceProvider.h:64: Missing spaces around += [whitespace/operators] [3] WebCore/loader/CachedScript.cpp:144: Tab found; better to use spaces [whitespace/tab] [1] WebCore/loader/CachedScript.cpp:145: Missing spaces around += [whitespace/operators] [3] Total errors found: 5 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Eric Seidel (no email)
Comment 10
2010-04-01 23:38:13 PDT
Comment on
attachment 51559
[details]
First stab at a patch. Please fix the style errors and be sure to include the bug URL in your ChangeLog entry.
David Leong
Comment 11
2010-08-06 14:29:34 PDT
From Oliver's comment in the other error:
https://bugs.webkit.org/show_bug.cgi?id=36174
"Can you re-measure following the recent parser + lexer + bom handling changes? That said the 0-delay timer did seem low to me when i was doing the bom handling stuff." Updated the results with the latest webkit baseline
r64659
. Looks like performance degradations are now much smaller <4% difference. We can probably close this bug. V8 Baseline 966 978 981 Richards 3115 3098 3154 DeltaBlue 808 829 823 Cryoto 2352 2369 2345 RayTrace 855 886 909 EarleyBoyer 592 594 593 Regexp 272 273 272 V8 Modified 958 953 949 Richards 3091 3140 2993 DeltaBlue 814 810 800 Cryoto 2323 2311 2320 RayTrace 885 886 899 EarleyBoyer 599 589 581 Regexp 250 245 252 SunSpider 1552.4 1633.8 1637.2 3d 151.6 138.4 144.0 access 61.4 65.4 69.4 bitops 27.8 27.6 27.2 controlflow 3.4 4.0 4.0 crypto 60.6 62.8 65.6 date 319.4 343.2 337.4 math 37.6 37.0 37.8 regexp 17.4 17.6 17.8 string 873.2 937.8 934.0 SunSpider-slow 1705.8 1728.4 1701.6 3d 165.0 161.6 146.6 access 66.6 71.2 67.0 bitops 27.8 29 27.2 controlflow 3.4 3.8 3.4 crypto 70.0 71.6 70.0 date 355.2 358.8 351.8 math 37.2 37.0 36.8 regexp 17.6 17.2 17.6 string 963.0 978.2 981.2
Oliver Hunt
Comment 12
2010-08-06 15:09:14 PDT
Closing per above comment. In all likelihood this has ceased to be a problem due to the update to modern es5 semantics (which get rid of the bom stripping semantics)
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug