Bug 120143

Summary:

Support in memory compression of rarely used data

Product:

WebKit

Reporter:

Oliver Hunt <oliver>

Component:

New Bugs

Assignee:

Oliver Hunt <oliver>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, benjamin, buildbot, cmarcelo, commit-queue, eflews.bot, gtk-ews, gyuyoung.kim, ossy, philn, rakuco, rego+ews, rniwa, webkit-ews, xan.lopez, zan

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

120246

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2	none
More refactoring	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	barraclough: review+

Oliver Hunt

Reported 2013-08-21 17:30:32 PDT

Support in memory compression of rarely used data

Attachments
Patch (15.87 KB, patch) 2013-08-21 17:31 PDT, Oliver Hunt	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2 (10.71 MB, application/zip) 2013-08-21 21:48 PDT, Build Bot	no flags	Details
More refactoring (16.53 KB, patch) 2013-08-22 09:39 PDT, Oliver Hunt	no flags	Details Formatted Diff Diff
Patch (20.52 KB, patch) 2013-08-22 18:25 PDT, Oliver Hunt	no flags	Details Formatted Diff Diff
Patch (24.24 KB, patch) 2013-08-22 19:43 PDT, Oliver Hunt	no flags	Details Formatted Diff Diff
Patch (24.22 KB, patch) 2013-08-22 20:05 PDT, Oliver Hunt	no flags	Details Formatted Diff Diff
Patch (24.29 KB, patch) 2013-08-22 22:53 PDT, Oliver Hunt	no flags	Details Formatted Diff Diff
Patch (24.62 KB, patch) 2013-08-22 23:43 PDT, Oliver Hunt	no flags	Details Formatted Diff Diff
Patch (25.01 KB, patch) 2013-08-23 10:23 PDT, Oliver Hunt	barraclough: review+	Details Formatted Diff Diff
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.

Oliver Hunt

Comment 1 2013-08-21 17:31:01 PDT

Created attachment 209315 [details] Patch

WebKit Commit Bot

Comment 2 2013-08-21 17:37:00 PDT

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 3 2013-08-21 17:42:13 PDT

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 4 2013-08-21 17:43:54 PDT

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 5 2013-08-21 17:47:58 PDT

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 6 2013-08-21 20:01:41 PDT

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 7 2013-08-21 20:12:18 PDT

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 8 2013-08-21 21:01:33 PDT

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 9 2013-08-21 21:48:25 PDT

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

Comment 10 2013-08-21 21:48:28 PDT

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 11 2013-08-21 22:06:59 PDT

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

Comment 12 2013-08-22 09:39:32 PDT

Created attachment 209374 [details] More refactoring

Oliver Hunt

Comment 13 2013-08-22 18:25:45 PDT

Created attachment 209420 [details] Patch

WebKit Commit Bot

Comment 14 2013-08-22 18:27:39 PDT

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 15 2013-08-22 18:35:43 PDT

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 16 2013-08-22 18:38:05 PDT

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 17 2013-08-22 18:56:03 PDT

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 18 2013-08-22 19:08:02 PDT

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

Comment 19 2013-08-22 19:43:12 PDT

Created attachment 209425 [details] Patch

Early Warning System Bot

Comment 20 2013-08-22 19:51:35 PDT

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 21 2013-08-22 19:54:07 PDT

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 22 2013-08-22 19:54:39 PDT

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

Comment 23 2013-08-22 20:05:39 PDT

Created attachment 209427 [details] Patch

EFL EWS Bot

Comment 24 2013-08-22 20:14:00 PDT

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 25 2013-08-22 20:15:03 PDT

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 26 2013-08-22 20:19:12 PDT

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 27 2013-08-22 20:45:49 PDT

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

Comment 28 2013-08-22 22:53:37 PDT

Created attachment 209431 [details] Patch

Early Warning System Bot

Comment 29 2013-08-22 23:03:24 PDT

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 30 2013-08-22 23:05:12 PDT

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 31 2013-08-22 23:24:48 PDT

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 32 2013-08-22 23:29:29 PDT

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

Comment 33 2013-08-22 23:43:10 PDT

Created attachment 209437 [details] Patch

EFL EWS Bot

Comment 34 2013-08-22 23:54:31 PDT

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 35 2013-08-23 01:46:45 PDT

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 36 2013-08-23 05:25:18 PDT

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

Comment 37 2013-08-23 08:44:50 PDT

(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

Comment 38 2013-08-23 10:23:01 PDT

Created attachment 209472 [details] Patch

Chris Dumez

Comment 39 2013-08-23 10:35:57 PDT

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 40 2013-08-23 10:43:36 PDT

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

Comment 41 2013-08-23 10:44:33 PDT

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

Oliver Hunt

Comment 42 2013-08-23 10:52:12 PDT

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

Csaba Osztrogonác

Comment 43 2013-08-23 13:58:33 PDT

(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

Comment 44 2013-08-24 11:12:48 PDT

(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

Note You need to log in before you can comment on or make changes to this bug.