| Summary: | Builtin source should be minified more | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||||||
| Component: | JavaScriptCore | Assignee: | Joseph Pecoraro <joepeck> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | bburg, commit-queue, joepeck, keith_miller, mark.lam, msaboff, saam, ysuzuki | ||||||||
| Priority: | P2 | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
Looks like most of this whitespace is added by: generate_embedded_code_string_section_for_function It includes at least the leading 4 spaces of each line and the trailing newline at the end. (In reply to comment #1) > Looks like most of this whitespace is added by: > generate_embedded_code_string_section_for_function I take that back, that is whitespace outside the source string. I'll handle some basic stuff, like stripping the empty comments and empty lines. I think it leads to even more readable code in the results anyways: BEFORE: > const int s_moduleLoaderObjectRequestFetchCodeLength = 669; > const char* s_moduleLoaderObjectRequestFetchCode = > "(function (key)\n" \ > "{\n" \ > " //\n" \ > "\n" \ > " \"use strict\";\n" \ > "\n" \ > " var entry = this.ensureRegistered(key);\n" \ > " if (entry.state > this.Link) {\n" \ > " var deferred = @newPromiseCapability(@InternalPromise);\n" \ > " deferred.@reject.@call(undefined, new @TypeError(\"Requested module is already ready to be executed.\"));\n" \ > " return deferred.@promise;\n" \ > " }\n" \ > "\n" \ > " if (entry.fetch)\n" \ > " return entry.fetch;\n" \ > "\n" \ > " var loader = this;\n" \ > "\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " var fetchPromise = this.fetch(key).then(function (payload) {\n" \ > " loader.setStateToMax(entry, loader.Translate);\n" \ > " return payload;\n" \ > " });\n" \ > " entry.fetch = fetchPromise;\n" \ > " return fetchPromise;\n" \ > "})\n" \ > ; AFTER: > const int s_moduleLoaderObjectRequestFetchCodeLength = 615; > const char* s_moduleLoaderObjectRequestFetchCode = > "(function (key)\n" \ > "{\n" \ > " \"use strict\";\n" \ > " var entry = this.ensureRegistered(key);\n" \ > " if (entry.state > this.Link) {\n" \ > " var deferred = @newPromiseCapability(@InternalPromise);\n" \ > " deferred.@reject.@call(undefined, new @TypeError(\"Requested module is already ready to be executed.\"));\n" \ > " return deferred.@promise;\n" \ > " }\n" \ > " if (entry.fetch)\n" \ > " return entry.fetch;\n" \ > " var loader = this;\n" \ > " var fetchPromise = this.fetch(key).then(function (payload) {\n" \ > " loader.setStateToMax(entry, loader.Translate);\n" \ > " return payload;\n" \ > " });\n" \ > " entry.fetch = fetchPromise;\n" \ > " return fetchPromise;\n" \ > "})\n" \ > ; I didn't go all out and minify everything though. I think this is a good balance. Created attachment 267337 [details]
[PATCH] Proposed Fix
This patch modifies the JS builtins code generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-builtins-generator-tests --reset-results`) Comment on attachment 267337 [details]
[PATCH] Proposed Fix
r=me.
And I also suggest to disable this when building JSC in the debug mode.
In the debug build, we can use @assert buildin intrinsic. And it reports a line number in a given function when the assertion fires.
So, to report meaningful line number, disabling this in the debug mode is nice.
Created attachment 267382 [details]
[PATCH] Proposed Fix
I wasn't sure how to do this reliably check Debug vs. Non-Debug across all platforms, so I default to not doing the minifying and minify only in builds where the CONFIGURATION environment variable does not start with "Debug". So Mac Release/Production builds would minify.
Created attachment 267383 [details]
[PATCH] Proposed Fix
Comment on attachment 267383 [details] [PATCH] Proposed Fix Clearing flags on attachment: 267383 Committed r194144: <http://trac.webkit.org/changeset/194144> All reviewed patches have been landed. Closing bug. |
* SUMMARY Builtin source should be minified. In many cases, the extracted builtin code can be minified more. Here are some existing examples: 1. Whitespace can be stripped. Primarily leading whitespace and many trailing newlines. This seems to account for sometimes 20% of the size of these builtins. 2. Lines that end up as only comments can be removed entirely. For example: > const JSC::ConstructAbility s_moduleLoaderObjectNewRegistryEntryCodeConstructAbility = JSC::ConstructAbility::CannotConstruct; > const int s_moduleLoaderObjectNewRegistryEntryCodeLength = 713; > const char* s_moduleLoaderObjectNewRegistryEntryCode = > "(function (key)\n" \ > "{\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > " //\n" \ > "\n" \ > " \"use strict\";\n" \ > "\n" \ > " return {\n" \ > " key: key,\n" \ > " state: this.Fetch,\n" \ > " metadata: undefined,\n" \ > " fetch: undefined,\n" \ > " translate: undefined,\n" \ > " instantiate: undefined,\n" \ > " resolveDependencies: undefined,\n" \ > " dependencies: [], //\n" \ > " dependenciesMap: undefined,\n" \ > " module: undefined, //\n" \ > " error: undefined,\n" \ > " };\n" \ > "})\n" \