Bug 121334 – XMLViewer_js and XMLViewer_css should be minified

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Joseph Pecoraro]

We should minimize XMLViewer.{js,css} before xxding them to reduce their size in the WebCore binary.

To get their sizes:
shell> symbols Release/WebCore.framework/WebCore | sed -E 's/^ *0x[0-9a-f]+ *\(( *)0x([^)]+)\)/\1\2/' | sort -r | grep XMLView

Before Minimizing: 13.58k + 2.14k
  3650 XMLViewer_js [NameNList, MangledNameNList, NList]
   890 XMLViewer_css [NameNList, MangledNameNList, NList]

After Minimizing: 10.17k + 0.66k
  28b0 XMLViewer_js [NameNList, MangledNameNList, NList]
   2a0 XMLViewer_css [NameNList, MangledNameNList, NList]

I'm waiting on attaching a patch until bug 121183 lands. It includes the necessary minifiers.

[Joseph Pecoraro]

Created attachment 211637 [details]
[PATCH] Proposed Fix

[Early Warning System Bot]

Comment on attachment 211637 [details]
[PATCH] Proposed Fix

Attachment 211637 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1819269

[Early Warning System Bot]

Comment on attachment 211637 [details]
[PATCH] Proposed Fix

Attachment 211637 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1875284

[Timothy Hatcher]

I thought we removed XMLViewer…

[Timothy Hatcher]

Comment on attachment 211637 [details]
[PATCH] Proposed Fix

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

> Source/WebCore/DerivedSources.make:769
> +	python "$(WebCore)/inspector/Scripts/cssmin.py" <"$(WebCore)/xml/XMLViewer.css" > ./XMLViewer.min.css

This suggests the scripts should move up to a top level WebCore/Scripts directory.

[Joseph Pecoraro]

(In reply to comment #5)
> (From update of attachment 211637 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=211637&action=review
> 
> > Source/WebCore/DerivedSources.make:769
> > +	python "$(WebCore)/inspector/Scripts/cssmin.py" <"$(WebCore)/xml/XMLViewer.css" > ./XMLViewer.min.css
> 
> This suggests the scripts should move up to a top level WebCore/Scripts directory.

They also already use inspector/xxd.pl.

If we move scripts, I want to do it all at once. The inspector scripts are all over.

[Joseph Pecoraro]

Created attachment 211662 [details]
[PATCH] For Bots 1

[Timothy Hatcher]

Comment on attachment 211637 [details]
[PATCH] Proposed Fix

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

>>> Source/WebCore/DerivedSources.make:769
>>> +	python "$(WebCore)/inspector/Scripts/cssmin.py" <"$(WebCore)/xml/XMLViewer.css" > ./XMLViewer.min.css
>> 
>> This suggests the scripts should move up to a top level WebCore/Scripts directory.
> 
> They also already use inspector/xxd.pl.
> 
> If we move scripts, I want to do it all at once. The inspector scripts are all over.

Fair enough.

[Joseph Pecoraro]

Committed <http://trac.webkit.org/changeset/155790>.

[Hugo Parente Lima]

Comment on attachment 211662 [details]
[PATCH] For Bots 1

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

> Source/WebCore/CMakeLists.txt:3059
> +    COMMAND ${PYTHON_EXECUTABLE} ${WEBCORE_DIR}/inspector/Scripts/cssmin.py <${WEBCORE_DIR}/xml/XMLViewer.js > ${DERIVED_SOURCES_WEBCORE_DIR}/XMLViewer.min.js

This should be jsmin.py instead of cssmin.py.

Also should have a space after "<" or cmake will expand this like "/usr/bin/python2 WEBKITDIR/Source/WebCore/inspector/Scripts/cssmin.py "<WEBKITDIR/Source/WebCore/xml/XMLViewer.css" > WEBKITDIR/WebKitBuild/Release/DerivedSources/WebCore/XMLViewer.min.css"

I'm going to upload a patch these cmake issues.

[Joseph Pecoraro]

(In reply to comment #10)
> (From update of attachment 211662 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=211662&action=review
> 
> > Source/WebCore/CMakeLists.txt:3059
> > +    COMMAND ${PYTHON_EXECUTABLE} ${WEBCORE_DIR}/inspector/Scripts/cssmin.py <${WEBCORE_DIR}/xml/XMLViewer.js > ${DERIVED_SOURCES_WEBCORE_DIR}/XMLViewer.min.js
> 
> This should be jsmin.py instead of cssmin.py.
> 
> Also should have a space after "<" or cmake will expand this like "/usr/bin/python2 WEBKITDIR/Source/WebCore/inspector/Scripts/cssmin.py "<WEBKITDIR/Source/WebCore/xml/XMLViewer.css" > WEBKITDIR/WebKitBuild/Release/DerivedSources/WebCore/XMLViewer.min.css"
> 
> I'm going to upload a patch these cmake issues.

Good catches. Please CC me so I can review.

[Alexey Proskuryakov]

Is this really worth it? Do we expect to make performance better with 5K improvements to binary size?

[Ryosuke Niwa]

(In reply to comment #12)
> Is this really worth it? Do we expect to make performance better with 5K improvements to binary size?

I don't think this change alone will improve the performance per se.  But when we have hundreds of small changes like this, the result accumulates and makes a difference.