| 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: | P2 | ||||||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||
| Bug Depends on: | 120246 | ||||||||||||||||||||||
| Bug Blocks: | |||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||
|
Description
Oliver Hunt
2013-08-21 17:30:32 PDT
Created attachment 209315 [details]
Patch
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. 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
Comment on attachment 209315 [details] Patch Attachment 209315 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1515986 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 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 Comment on attachment 209315 [details] Patch Attachment 209315 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1526341 Comment on attachment 209315 [details] Patch Attachment 209315 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/1543040 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 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
Comment on attachment 209315 [details] Patch Attachment 209315 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/1530041 Created attachment 209374 [details]
More refactoring
Created attachment 209420 [details]
Patch
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. Comment on attachment 209420 [details] Patch Attachment 209420 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1525791 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 Comment on attachment 209420 [details] Patch Attachment 209420 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1528427 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 Created attachment 209425 [details]
Patch
Comment on attachment 209425 [details] Patch Attachment 209425 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1527450 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 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 Created attachment 209427 [details]
Patch
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 Comment on attachment 209427 [details] Patch Attachment 209427 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1555118 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 Comment on attachment 209427 [details] Patch Attachment 209427 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1557004 Created attachment 209431 [details]
Patch
Comment on attachment 209431 [details] Patch Attachment 209431 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1526553 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 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 Comment on attachment 209431 [details] Patch Attachment 209431 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1542206 Created attachment 209437 [details]
Patch
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 Comment on attachment 209437 [details] Patch Attachment 209437 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1501330 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.
(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. Created attachment 209472 [details]
Patch
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} 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. Oh, and please check this doesn't break any builds (efl I'm looking at you). Committed r154498: <http://trac.webkit.org/changeset/154498> (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. (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 |