Bug 120143 – Support in memory compression of rarely used data

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Oliver Hunt]

Support in memory compression of rarely used data

[Oliver Hunt]

Created attachment 209315 [details]
Patch

[WebKit Commit Bot]

Attachment 209315 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig', u'Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xcodeproj/project.pbxproj', u'Source/WTF/wtf/CheckedArithmetic.h', u'Source/WTF/wtf/Compression.cpp', u'Source/WTF/wtf/Compression.h']" exit_code: 1
Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:329:  Missing space after ,  [whitespace/comma] [3]
Source/WTF/wtf/Compression.h:50:  This { should be at the end of the previous line  [whitespace/braces] [4]
Source/WTF/wtf/Compression.h:54:  Local variables should never be PassOwnPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WTF/wtf/Compression.cpp:28:  You should add a blank line after implementation file's own header.  [build/include_order] [4]
Source/WTF/wtf/Compression.cpp:52:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WTF/wtf/Compression.cpp:103:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WTF/wtf/Compression.cpp:119:  Local variables should never be PassOwnPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Total errors found: 7 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Oliver Hunt]

Comment on attachment 209315 [details]
Patch

Not complete, and i haven't got a good feel for the best compress interface.  Mainly using this to move the patch around :D

[Early Warning System Bot]

Comment on attachment 209315 [details]
Patch

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

[Early Warning System Bot]

Comment on attachment 209315 [details]
Patch

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

[EFL EWS Bot]

Comment on attachment 209315 [details]
Patch

Attachment 209315 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1528270

[EFL EWS Bot]

Comment on attachment 209315 [details]
Patch

Attachment 209315 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1526341

[kov's GTK+ EWS bot]

Comment on attachment 209315 [details]
Patch

Attachment 209315 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1543040

[Build Bot]

Comment on attachment 209315 [details]
Patch

Attachment 209315 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1543050

New failing tests:
fast/workers/termination-with-port-messages.html

[Build Bot]

Created attachment 209323 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.4

[kov's GTK+ EWS bot]

Comment on attachment 209315 [details]
Patch

Attachment 209315 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1530041

[Oliver Hunt]

Created attachment 209374 [details]
More refactoring

[Oliver Hunt]

Created attachment 209420 [details]
Patch

[WebKit Commit Bot]

Attachment 209420 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig', u'Source/JavaScriptCore/bytecode/ExpressionRangeInfo.h', u'Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp', u'Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xcodeproj/project.pbxproj', u'Source/WTF/wtf/CheckedArithmetic.h', u'Source/WTF/wtf/Compression.cpp', u'Source/WTF/wtf/Compression.h']" exit_code: 1
Source/WTF/wtf/Compression.h:58:  This { should be at the end of the previous line  [whitespace/braces] [4]
Source/WTF/wtf/Compression.h:62:  Local variables should never be PassOwnPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/JavaScriptCore/bytecode/ExpressionRangeInfo.h:112:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WTF/wtf/Compression.cpp:28:  You should add a blank line after implementation file's own header.  [build/include_order] [4]
Source/WTF/wtf/Compression.cpp:107:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 5 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Comment on attachment 209420 [details]
Patch

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

[Early Warning System Bot]

Comment on attachment 209420 [details]
Patch

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

[EFL EWS Bot]

Comment on attachment 209420 [details]
Patch

Attachment 209420 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1528427

[EFL EWS Bot]

Comment on attachment 209420 [details]
Patch

Attachment 209420 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1555099

[Oliver Hunt]

Created attachment 209425 [details]
Patch

[Early Warning System Bot]

Comment on attachment 209425 [details]
Patch

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

[Early Warning System Bot]

Comment on attachment 209425 [details]
Patch

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

[EFL EWS Bot]

Comment on attachment 209425 [details]
Patch

Attachment 209425 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1546143

[Oliver Hunt]

Created attachment 209427 [details]
Patch

[EFL EWS Bot]

Comment on attachment 209427 [details]
Patch

Attachment 209427 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1542178

[Early Warning System Bot]

Comment on attachment 209427 [details]
Patch

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

[Early Warning System Bot]

Comment on attachment 209427 [details]
Patch

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

[EFL EWS Bot]

Comment on attachment 209427 [details]
Patch

Attachment 209427 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1557004

[Oliver Hunt]

Created attachment 209431 [details]
Patch

[Early Warning System Bot]

Comment on attachment 209431 [details]
Patch

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

[Early Warning System Bot]

Comment on attachment 209431 [details]
Patch

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

[EFL EWS Bot]

Comment on attachment 209431 [details]
Patch

Attachment 209431 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1556128

[EFL EWS Bot]

Comment on attachment 209431 [details]
Patch

Attachment 209431 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1542206

[Oliver Hunt]

Created attachment 209437 [details]
Patch

[EFL EWS Bot]

Comment on attachment 209437 [details]
Patch

Attachment 209437 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1527498

[EFL EWS Bot]

Comment on attachment 209437 [details]
Patch

Attachment 209437 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1501330

[Anders Carlsson]

Comment on attachment 209437 [details]
Patch

Do you anticipate using this in WebCore or WebKit as well or is it just JSC? If not, I don't see why this has to go into WTF.

[Oliver Hunt]

(In reply to comment #36)
> (From update of attachment 209437 [details])
> Do you anticipate using this in WebCore or WebKit as well or is it just JSC? If not, I don't see why this has to go into WTF.

It's not strictly tied to JSC in anyway so i don't see why it belongs in JSC.

Putting it in WTF means it's easier to use everywhere without randomly including <JavaScriptCore/...> and having JSC:: namespaces everywhere.

[Oliver Hunt]

Created attachment 209472 [details]
Patch

[Chris Dumez]

Comment on attachment 209472 [details]
Patch

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

> Source/WTF/wtf/PlatformEfl.cmake:12
> +    z

We'll see if this builds but you likely want: ${ZLIB_LIBRARIES}

[Gavin Barraclough]

Comment on attachment 209472 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:8
> +        Include zlib in LD_FLAGS and make UnlinkedCodeBlock make use of CompressibleVector 

Please quote justification.

> Source/WTF/wtf/Compression.cpp:74
> +    memset(compressedData, 0, currentCapacity);

This kinda sucks.  If it's only here to appease bad compilers, might be worth commenting on that.

[Gavin Barraclough]

Oh, and please check this doesn't break any builds (efl I'm looking at you).

[Oliver Hunt]

Committed r154498: <http://trac.webkit.org/changeset/154498>

[Csaba Osztrogonác]

(In reply to comment #42)
> Committed r154498: <http://trac.webkit.org/changeset/15449

(In reply to comment #42)
> Committed r154498: <http://trac.webkit.org/changeset/154498>

FYi: It broke all jsc and layout test everywhere except mac.

[Csaba Osztrogonác]

(In reply to comment #43)
> (In reply to comment #42)
> > Committed r154498: <http://trac.webkit.org/changeset/15449
 
> FYi: It broke all jsc and layout test everywhere except mac.

new bug for this regression: https://bugs.webkit.org/show_bug.cgi?id=120246