Bug 193544 - [JSC] Reduce size of SourceProvider
Summary: [JSC] Reduce size of SourceProvider
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks: 193606
  Show dependency treegraph
 
Reported: 2019-01-17 11:53 PST by Yusuke Suzuki
Modified: 2019-01-20 21:51 PST (History)
9 users (show)

See Also:


Attachments
Patch (18.05 KB, patch)
2019-01-18 02:20 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-highsierra (308.10 KB, application/zip)
2019-01-18 03:12 PST, EWS Watchlist
no flags Details
Archive of layout-test-results from ews104 for mac-highsierra-wk2 (427.03 KB, application/zip)
2019-01-18 03:18 PST, EWS Watchlist
no flags Details
Archive of layout-test-results from ews116 for mac-highsierra (455.25 KB, application/zip)
2019-01-18 03:26 PST, EWS Watchlist
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (130.77 MB, application/zip)
2019-01-18 04:21 PST, EWS Watchlist
no flags Details
Patch (18.81 KB, patch)
2019-01-18 20:50 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews112 for mac-highsierra (473.73 KB, application/zip)
2019-01-18 21:57 PST, EWS Watchlist
no flags Details
Patch (21.59 KB, patch)
2019-01-19 00:34 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (22.05 KB, patch)
2019-01-19 00:54 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (22.07 KB, patch)
2019-01-19 01:03 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (72.00 KB, patch)
2019-01-20 13:43 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (70.59 KB, patch)
2019-01-20 13:47 PST, Yusuke Suzuki
sbarati: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2019-01-17 11:53:55 PST
SourceProvider is commonly used to represent code. Since JSC has bunch of string code internally (builtin JS), it easily bloat the number of SourceProvider (raw JSC shell has 208 StringSourceProvider).
We can easily reduce the size of SourceProvider by reordering. Furthermore, we can separately create BuiltinSourceProvider which does not have any source map information.
Comment 1 Yusuke Suzuki 2019-01-18 02:20:15 PST
Created attachment 359464 [details]
Patch
Comment 2 EWS Watchlist 2019-01-18 02:23:00 PST
Attachment 359464 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:71:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:75:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:76:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:77:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:78:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:64:  [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'generate_embedded_code_data_section_for_function' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_header.py:58:  multiple statements on one line (semicolon)  [pep8/E702] [5]
Total errors found: 7 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 EWS Watchlist 2019-01-18 03:12:55 PST
Comment on attachment 359464 [details]
Patch

Attachment 359464 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10794151

Number of test failures exceeded the failure limit.
Comment 4 EWS Watchlist 2019-01-18 03:12:56 PST
Created attachment 359467 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 5 EWS Watchlist 2019-01-18 03:18:06 PST
Comment on attachment 359464 [details]
Patch

Attachment 359464 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10794154

Number of test failures exceeded the failure limit.
Comment 6 EWS Watchlist 2019-01-18 03:18:08 PST
Created attachment 359468 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 7 EWS Watchlist 2019-01-18 03:26:40 PST
Comment on attachment 359464 [details]
Patch

Attachment 359464 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10794133

Number of test failures exceeded the failure limit.
Comment 8 EWS Watchlist 2019-01-18 03:26:42 PST
Created attachment 359469 [details]
Archive of layout-test-results from ews116 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 9 EWS Watchlist 2019-01-18 04:21:17 PST
Comment on attachment 359464 [details]
Patch

Attachment 359464 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10794252

Number of test failures exceeded the failure limit.
Comment 10 EWS Watchlist 2019-01-18 04:21:24 PST
Created attachment 359471 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 11 Yusuke Suzuki 2019-01-18 20:50:05 PST
Created attachment 359584 [details]
Patch
Comment 12 EWS Watchlist 2019-01-18 20:53:12 PST
Attachment 359584 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:71:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:75:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:76:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:77:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:78:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:64:  [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'generate_embedded_code_data_section_for_function' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_header.py:58:  multiple statements on one line (semicolon)  [pep8/E702] [5]
Total errors found: 7 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 EWS Watchlist 2019-01-18 21:57:49 PST
Comment on attachment 359584 [details]
Patch

Attachment 359584 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10806335

Number of test failures exceeded the failure limit.
Comment 14 EWS Watchlist 2019-01-18 21:57:50 PST
Created attachment 359587 [details]
Archive of layout-test-results from ews112 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 15 Yusuke Suzuki 2019-01-19 00:34:53 PST
Created attachment 359595 [details]
Patch
Comment 16 EWS Watchlist 2019-01-19 00:39:30 PST
Attachment 359595 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:71:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:75:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:76:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:77:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:78:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:64:  [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'generate_embedded_code_data_section_for_function' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_header.py:58:  multiple statements on one line (semicolon)  [pep8/E702] [5]
Total errors found: 7 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 Yusuke Suzuki 2019-01-19 00:54:24 PST
Created attachment 359597 [details]
Patch
Comment 18 EWS Watchlist 2019-01-19 00:56:41 PST
Attachment 359597 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:75:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:76:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:80:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:81:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:82:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:83:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:64:  [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'generate_embedded_code_data_section_for_function' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_header.py:58:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_header.py:59:  multiple statements on one line (semicolon)  [pep8/E702] [5]
Total errors found: 9 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 19 Yusuke Suzuki 2019-01-19 01:03:55 PST
Created attachment 359599 [details]
Patch
Comment 20 EWS Watchlist 2019-01-19 01:07:12 PST
Attachment 359599 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:75:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:76:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:80:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:81:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:82:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:83:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:64:  [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'generate_embedded_code_data_section_for_function' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_header.py:58:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_header.py:59:  multiple statements on one line (semicolon)  [pep8/E702] [5]
Total errors found: 9 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 Saam Barati 2019-01-20 12:23:15 PST
Comment on attachment 359599 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359599&action=review

> Source/JavaScriptCore/ChangeLog:17
> +        Unfortunately, MSVC does not accept super long C string literal. So instead, we construct combined string in a form of C array.

:(

> Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:83
> +            lines = []
> +            lines.append("const JSC::ConstructAbility s_%(codeName)sConstructAbility = JSC::ConstructAbility::%(canConstruct)s;" % data);
> +            lines.append("const int s_%(codeName)sLength = %(embeddedSourceLength)d;" % data);
> +            lines.append("static const JSC::Intrinsic s_%(codeName)sIntrinsic = JSC::%(intrinsic)s;" % data);
> +            lines.append("const char* s_%(codeName)s { s_%(namespace)sCombinedCode + %(embeddedSource)d };" % data);

Why can't you just use the "generate_embedded_code_string_section_for_function" function here?

> Source/JavaScriptCore/builtins/BuiltinExecutables.cpp:39
> +    , m_combinedSourceProvider(StringSourceProvider::create(StringImpl::createFromLiteral(s_JSCCombinedCode, s_JSCCombinedCodeLength), { }, URL()))

what happens for WebCore?

> Source/JavaScriptCore/parser/Parser.cpp:-132
> -    , m_syntaxAlreadyValidated(source.provider()->isValid())

This was unused? It feels like this should be a different patch.
Comment 22 Yusuke Suzuki 2019-01-20 13:20:03 PST
Comment on attachment 359599 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359599&action=review

>> Source/JavaScriptCore/ChangeLog:17
>> +        Unfortunately, MSVC does not accept super long C string literal. So instead, we construct combined string in a form of C array.
> 
> :(

Yeah, that was the saddest part...

>> Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:83
>> +            lines.append("const char* s_%(codeName)s { s_%(namespace)sCombinedCode + %(embeddedSource)d };" % data);
> 
> Why can't you just use the "generate_embedded_code_string_section_for_function" function here?

We generate the code based on the modified data, it's a bit tricky.
But it removes the above string templates. OK, changed.

>> Source/JavaScriptCore/builtins/BuiltinExecutables.cpp:39
>> +    , m_combinedSourceProvider(StringSourceProvider::create(StringImpl::createFromLiteral(s_JSCCombinedCode, s_JSCCombinedCodeLength), { }, URL()))
> 
> what happens for WebCore?

WebCore does not use combined version. It uses separate version, and this patch does not affect on that.

>> Source/JavaScriptCore/parser/Parser.cpp:-132
>> -    , m_syntaxAlreadyValidated(source.provider()->isValid())
> 
> This was unused? It feels like this should be a different patch.

Yes, nobody sets `true` for SourceProvider::m_validated. So, this is complete dead code.
This is done in this patch because we want to drop `bool m_validated` flag in SourceProvider to reduce the size of it.
Comment 23 Yusuke Suzuki 2019-01-20 13:43:44 PST
Created attachment 359648 [details]
Patch
Comment 24 EWS Watchlist 2019-01-20 13:46:38 PST
Attachment 359648 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_implementation.py:70:  [BuiltinsSeparateImplementationGenerator.generate_output] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'generate_embedded_code_string_section_for_data' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_implementation.py:70:  [BuiltinsSeparateImplementationGenerator.generate_output] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'generate_embedded_code_data_for_function' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:74:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:75:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:64:  [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'generate_embedded_code_data_for_function' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:78:  [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'generate_embedded_code_string_section_for_data' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generator.py:143:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generator.py:144:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generator.py:145:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generator.py:146:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_header.py:58:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_header.py:59:  multiple statements on one line (semicolon)  [pep8/E702] [5]
Total errors found: 12 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 25 Yusuke Suzuki 2019-01-20 13:47:14 PST
Created attachment 359649 [details]
Patch
Comment 26 EWS Watchlist 2019-01-20 13:50:45 PST
Attachment 359649 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_implementation.py:70:  [BuiltinsSeparateImplementationGenerator.generate_output] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'generate_embedded_code_string_section_for_data' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_implementation.py:70:  [BuiltinsSeparateImplementationGenerator.generate_output] Instance of 'BuiltinsSeparateImplementationGenerator' has no 'generate_embedded_code_data_for_function' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:74:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:75:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:64:  [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'generate_embedded_code_data_for_function' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_implementation.py:78:  [BuiltinsCombinedImplementationGenerator.generate_output] Instance of 'BuiltinsCombinedImplementationGenerator' has no 'generate_embedded_code_string_section_for_data' member  [pylint/E1101] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generator.py:143:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generator.py:144:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generator.py:145:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generator.py:146:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_header.py:58:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_combined_header.py:59:  multiple statements on one line (semicolon)  [pep8/E702] [5]
Total errors found: 12 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 27 Yusuke Suzuki 2019-01-20 14:57:44 PST
Updated the patch. Fixed the issues pointed in the review.
But this patch still keeps dropping m_validated flag & related Parser.cpp changes because this change is intended to drop unused m_validated flag in SourceProvider and dropping unused part is not so complicated.
Comment 28 Saam Barati 2019-01-20 15:20:34 PST
Comment on attachment 359649 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359649&action=review

> Source/JavaScriptCore/Scripts/tests/builtins/expected/JavaScriptCore-Builtin.Promise-Combined.js-result:122
> +const char s_JSCCombinedCode[] = { 40, 102, 117, 110, 99, 116, 105, 111, 110, 32, 40, 112, 114, 111, 109, 105, 115, 101, 44, 32, 118, 97, 108, 117, 101, 41, 10, 123, 10, 32, 32, 32, 32, 34, 117, 115, 101, 32, 115, 116, 114, 105, 99, 116, 34, 59, 10, 10, 32, 32, 32, 32, 118, 97, 114, 32, 114, 101, 97, 99, 116, 105, 111, 110, 115, 32, 61, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 70, 117, 108, 102, 105, 108, 108, 82, 101, 97, 99, 116, 105, 111, 110, 115, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 82, 101, 115, 117, 108, 116, 32, 61, 32, 118, 97, 108, 117, 101, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 70, 117, 108, 102, 105, 108, 108, 82, 101, 97, 99, 116, 105, 111, 110, 115, 32, 61, 32, 117, 110, 100, 101, 102, 105, 110, 101, 100, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 82, 101, 106, 101, 99, 116, 82, 101, 97, 99, 116, 105, 111, 110, 115, 32, 61, 32, 117, 110, 100, 101, 102, 105, 110, 101, 100, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 83, 116, 97, 116, 101, 32, 61, 32, 64, 112, 114, 111, 109, 105, 115, 101, 70, 117, 108, 102, 105, 108, 108, 101, 100, 59, 10, 10, 32, 32, 32, 32, 64, 73, 110, 115, 112, 101, 99, 116, 111, 114, 73, 110, 115, 116, 114, 117, 109, 101, 110, 116, 97, 116, 105, 111, 110, 46, 112, 114, 111, 109, 105, 115, 101, 70, 117, 108, 102, 105, 108, 108, 101, 100, 40, 112, 114, 111, 109, 105, 115, 101, 44, 32, 118, 97, 108, 117, 101, 44, 32, 114, 101, 97, 99, 116, 105, 111, 110, 115, 41, 59, 10, 10, 32, 32, 32, 32, 64, 116, 114, 105, 103, 103, 101, 114, 80, 114, 111, 109, 105, 115, 101, 82, 101, 97, 99, 116, 105, 111, 110, 115, 40, 114, 101, 97, 99, 116, 105, 111, 110, 115, 44, 32, 118, 97, 108, 117, 101, 41, 59, 10, 125, 41, 10, 40, 102, 117, 110, 99, 116, 105, 111, 110, 32, 40, 112, 114, 111, 109, 105, 115, 101, 44, 32, 114, 101, 97, 115, 111, 110, 41, 10, 123, 10, 32, 32, 32, 32, 34, 117, 115, 101, 32, 115, 116, 114, 105, 99, 116, 34, 59, 10, 10, 32, 32, 32, 32, 118, 97, 114, 32, 114, 101, 97, 99, 116, 105, 111, 110, 115, 32, 61, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 82, 101, 106, 101, 99, 116, 82, 101, 97, 99, 116, 105, 111, 110, 115, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 82, 101, 115, 117, 108, 116, 32, 61, 32, 114, 101, 97, 115, 111, 110, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 70, 117, 108, 102, 105, 108, 108, 82, 101, 97, 99, 116, 105, 111, 110, 115, 32, 61, 32, 117, 110, 100, 101, 102, 105, 110, 101, 100, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 82, 101, 106, 101, 99, 116, 82, 101, 97, 99, 116, 105, 111, 110, 115, 32, 61, 32, 117, 110, 100, 101, 102, 105, 110, 101, 100, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 83, 116, 97, 116, 101, 32, 61, 32, 64, 112, 114, 111, 109, 105, 115, 101, 82, 101, 106, 101, 99, 116, 101, 100, 59, 10, 10, 32, 32, 32, 32, 64, 73, 110, 115, 112, 101, 99, 116, 111, 114, 73, 110, 115, 116, 114, 117, 109, 101, 110, 116, 97, 116, 105, 111, 110, 46, 112, 114, 111, 109, 105, 115, 101, 82, 101, 106, 101, 99, 116, 101, 100, 40, 112, 114, 111, 109, 105, 115, 101, 44, 32, 114, 101, 97, 115, 111, 110, 44, 32, 114, 101, 97, 99, 116, 105, 111, 110, 115, 41, 59, 10, 10, 32, 32, 32, 32, 64, 116, 114, 105, 103, 103, 101, 114, 80, 114, 111, 109, 105, 115, 101, 82, 101, 97, 99, 116, 105, 111, 110, 115, 40, 114, 101, 97, 99, 116, 105, 111, 110, 115, 44, 32, 114, 101, 97, 115, 111, 110, 41, 59, 10, 125, 41, 10 };

What's MSVC's character limit?
Comment 29 Yusuke Suzuki 2019-01-20 15:25:24 PST
Comment on attachment 359649 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359649&action=review

>> Source/JavaScriptCore/Scripts/tests/builtins/expected/JavaScriptCore-Builtin.Promise-Combined.js-result:122
>> +const char s_JSCCombinedCode[] = { 40, 102, 117, 110, 99, 116, 105, 111, 110, 32, 40, 112, 114, 111, 109, 105, 115, 101, 44, 32, 118, 97, 108, 117, 101, 41, 10, 123, 10, 32, 32, 32, 32, 34, 117, 115, 101, 32, 115, 116, 114, 105, 99, 116, 34, 59, 10, 10, 32, 32, 32, 32, 118, 97, 114, 32, 114, 101, 97, 99, 116, 105, 111, 110, 115, 32, 61, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 70, 117, 108, 102, 105, 108, 108, 82, 101, 97, 99, 116, 105, 111, 110, 115, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 82, 101, 115, 117, 108, 116, 32, 61, 32, 118, 97, 108, 117, 101, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 70, 117, 108, 102, 105, 108, 108, 82, 101, 97, 99, 116, 105, 111, 110, 115, 32, 61, 32, 117, 110, 100, 101, 102, 105, 110, 101, 100, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 82, 101, 106, 101, 99, 116, 82, 101, 97, 99, 116, 105, 111, 110, 115, 32, 61, 32, 117, 110, 100, 101, 102, 105, 110, 101, 100, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 83, 116, 97, 116, 101, 32, 61, 32, 64, 112, 114, 111, 109, 105, 115, 101, 70, 117, 108, 102, 105, 108, 108, 101, 100, 59, 10, 10, 32, 32, 32, 32, 64, 73, 110, 115, 112, 101, 99, 116, 111, 114, 73, 110, 115, 116, 114, 117, 109, 101, 110, 116, 97, 116, 105, 111, 110, 46, 112, 114, 111, 109, 105, 115, 101, 70, 117, 108, 102, 105, 108, 108, 101, 100, 40, 112, 114, 111, 109, 105, 115, 101, 44, 32, 118, 97, 108, 117, 101, 44, 32, 114, 101, 97, 99, 116, 105, 111, 110, 115, 41, 59, 10, 10, 32, 32, 32, 32, 64, 116, 114, 105, 103, 103, 101, 114, 80, 114, 111, 109, 105, 115, 101, 82, 101, 97, 99, 116, 105, 111, 110, 115, 40, 114, 101, 97, 99, 116, 105, 111, 110, 115, 44, 32, 118, 97, 108, 117, 101, 41, 59, 10, 125, 41, 10, 40, 102, 117, 110, 99, 116, 105, 111, 110, 32, 40, 112, 114, 111, 109, 105, 115, 101, 44, 32, 114, 101, 97, 115, 111, 110, 41, 10, 123, 10, 32, 32, 32, 32, 34, 117, 115, 101, 32, 115, 116, 114, 105, 99, 116, 34, 59, 10, 10, 32, 32, 32, 32, 118, 97, 114, 32, 114, 101, 97, 99, 116, 105, 111, 110, 115, 32, 61, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 82, 101, 106, 101, 99, 116, 82, 101, 97, 99, 116, 105, 111, 110, 115, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 82, 101, 115, 117, 108, 116, 32, 61, 32, 114, 101, 97, 115, 111, 110, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 70, 117, 108, 102, 105, 108, 108, 82, 101, 97, 99, 116, 105, 111, 110, 115, 32, 61, 32, 117, 110, 100, 101, 102, 105, 110, 101, 100, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 82, 101, 106, 101, 99, 116, 82, 101, 97, 99, 116, 105, 111, 110, 115, 32, 61, 32, 117, 110, 100, 101, 102, 105, 110, 101, 100, 59, 10, 32, 32, 32, 32, 112, 114, 111, 109, 105, 115, 101, 46, 64, 112, 114, 111, 109, 105, 115, 101, 83, 116, 97, 116, 101, 32, 61, 32, 64, 112, 114, 111, 109, 105, 115, 101, 82, 101, 106, 101, 99, 116, 101, 100, 59, 10, 10, 32, 32, 32, 32, 64, 73, 110, 115, 112, 101, 99, 116, 111, 114, 73, 110, 115, 116, 114, 117, 109, 101, 110, 116, 97, 116, 105, 111, 110, 46, 112, 114, 111, 109, 105, 115, 101, 82, 101, 106, 101, 99, 116, 101, 100, 40, 112, 114, 111, 109, 105, 115, 101, 44, 32, 114, 101, 97, 115, 111, 110, 44, 32, 114, 101, 97, 99, 116, 105, 111, 110, 115, 41, 59, 10, 10, 32, 32, 32, 32, 64, 116, 114, 105, 103, 103, 101, 114, 80, 114, 111, 109, 105, 115, 101, 82, 101, 97, 99, 116, 105, 111, 110, 115, 40, 114, 101, 97, 99, 116, 105, 111, 110, 115, 44, 32, 114, 101, 97, 115, 111, 110, 41, 59, 10, 125, 41, 10 };
> 
> What's MSVC's character limit?

The error message is "fatal error C1091: compiler limit: string exceeds 65535 bytes in length". So we cannot create the string literal it is longer than 0xffff. And our JSCBuiltins.cpp for builtin JS code generates string literal longer than that if it is concatenated.
Comment 30 Yusuke Suzuki 2019-01-20 21:49:42 PST
Committed r240228: <https://trac.webkit.org/changeset/240228>
Comment 31 Radar WebKit Bug Importer 2019-01-20 21:51:05 PST
<rdar://problem/47420645>