Here's what I'm finding:
- The data structures that are used to track LargeAllocation seem like they may have some thread-safety issues. It's clear that the LargeAllocation table is getting corrupted, but it's not clear why. It *seems* like that code should not need locking since the collector only parses that table when the world is stopped and the mutator only shrinks the table when the collector is not running. The mutator may grow the table while the collector is running, but that shouldn't be a problem.
- We need some kind of thread safety story in MarkedAllocator and BlockSet. I think that we need a read-write lock there, implemented using two normal locks: one lock for the mutator to read, one lock for the collector to read, and to write you need to grab both locks.
(In reply to comment #3)
> Here's what I'm finding:
>
> - The data structures that are used to track LargeAllocation seem like they
> may have some thread-safety issues. It's clear that the LargeAllocation
> table is getting corrupted, but it's not clear why. It *seems* like that
> code should not need locking since the collector only parses that table when
> the world is stopped and the mutator only shrinks the table when the
> collector is not running. The mutator may grow the table while the
> collector is running, but that shouldn't be a problem.
Well that was easy. The LargeAllocation table had a second set of pointers directly into the vector that the GC would set up at the start of a cycle. In fact, it needs to set those pointers up at the start of each STW increment.
>
> - We need some kind of thread safety story in MarkedAllocator and BlockSet.
> I think that we need a read-write lock there, implemented using two normal
> locks: one lock for the mutator to read, one lock for the collector to read,
> and to write you need to grab both locks.
I'm going to work on this next, though it seems that with the above fix, the concurrent GC is a heck of a lot more stable.
The problem with MarkedAllocator is that it has those darned bitvectors, and they can resize.
Here are the possible things we could do:
- Let the mutator stop the collector when resizing the bitvectors.
- Use segmented bitvectors to allow lock-free resizing.
- Put read-write locks around the bitvectors.
- Try to implement a concurrent copying system for bits.
- Rely on the fact that marking only messes with those bits inside aboutToMarkSlow and noteMarkedSlow. The former already holds the block lock and the latter easily could. We could resize the bitvectors by locking all of the blocks in the allocator!
I'm going to slow down a bit and run regression tests on the version with the LargeAllocation fix and then I'll run benchmarks. I want to make sure that the fixes so far have not regressed anything.
Then, I think I'll try the read-write lock approach first. This means one extra lock acquisition in aboutToMarkSlow and one extra lock acquisition in noteMarkedSlow. On the flipside, they lose the need to CAS atomically. I'll go with that if it's fast enough - otherwise I think I'll try the lock-all-blocks approach to resizing. This is clearly the lowest impact and the bitvectors do not resize in steady state so this ought to be fine.
Looks like the next bug is that CodeBlockSet is failing to delete code blocks at the end of the collection in which they died. This is causing the next batch of nasty crashes.
Created attachment 295342[details]
better tests
This adds a new dedicated collect-continuously mode. It runs with no-cjit. This also makes the dfg-eager modes collect continuously, as the ftl-eager ones did.
Test failure rate is up to 91 because of the new modes.
Created attachment 295452[details]
even more fixes
collectContinuously indeed found two pre-existing bugs: one in IsFunction constant folding and another in SamplingProfiler.
(In reply to comment #13)
> Created attachment 295463[details]
> getting so stable
>
> Fixed race conditions in some UnconditionalFinalizers.
49 failures.
Attachment 295579[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:353: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2176: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 5 in 22 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295586[details]
Archive of layout-test-results from ews117 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Attachment 295604[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:353: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2176: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 5 in 26 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295615[details]
Archive of layout-test-results from ews115 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5
Attachment 295667[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:353: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2176: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 6 in 28 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 295669[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:353: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2176: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 6 in 33 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295688[details]
Archive of layout-test-results from ews117 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
(In reply to comment #6)
> The problem with MarkedAllocator is that it has those darned bitvectors, and
> they can resize.
>
> Here are the possible things we could do:
>
> - Let the mutator stop the collector when resizing the bitvectors.
>
> - Use segmented bitvectors to allow lock-free resizing.
>
> - Put read-write locks around the bitvectors.
>
> - Try to implement a concurrent copying system for bits.
>
> - Rely on the fact that marking only messes with those bits inside
> aboutToMarkSlow and noteMarkedSlow. The former already holds the block lock
> and the latter easily could. We could resize the bitvectors by locking all
> of the blocks in the allocator!
I've circled back to this now that I have everything else worked out.
I realized that having a stopTheCollector mechanism is actually super easy, and probably generally useful. I'm playing with that solution now.
I have a test that seems to reliably crash because of the bitvectors getting messed up, probably because of this bug. Hopefully doing this will fix that program.
Attachment 295704[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:353: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2197: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 6 in 38 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 295706[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:353: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2197: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 6 in 38 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295711[details]
Archive of layout-test-results from ews116 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Attachment 295746[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:353: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2197: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 6 in 41 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295808[details]
Archive of layout-test-results from ews116 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Attachment 295812[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:354: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2210: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 8 in 48 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 295898[details]
audited all visitChildren in JSC
View in context: https://bugs.webkit.org/attachment.cgi?id=295898&action=review
r=me
> Source/JavaScriptCore/bytecode/CodeBlock.cpp:2957
> + codeBlock->jettison(Profiler::JettisonDueToOldAge);
I think you should update the comment around "this != replacement()". It is now possible for that condition to be true simply because we were replacement() at time of check but we were no longer replacement() at time of jettison -- that's a perfectly valid outcome, and an important condition to get right.
Attachment 296039[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:354: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2210: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 9 in 89 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 296042[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:354: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2210: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 9 in 89 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296045[details]
Archive of layout-test-results from ews114 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Attachment 296054[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:354: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2210: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 9 in 89 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296059[details]
Archive of layout-test-results from ews114 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 296060[details]
Archive of layout-test-results from ews101 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Attachment 296072[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:354: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2210: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 9 in 89 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296080[details]
Archive of layout-test-results from ews117 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
(In reply to comment #70)
> Created attachment 296106[details]
> new protocol is starting to work
This isn't right - it breaks the compiler's ability to concurrently read objects. It breaks its ability to even do basic things like read object structures.
One solution is to change how we nuke the structure ID. We could simply poison some bit in the structureID, which would still result in a useful structureID.
Another solution is to have some other field - like a bit somewhere - get used as the thing that indicates badness. That seems harder.
Attachment 296114[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:354: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2250: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.cpp:219: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSCInlines.h:55: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 11 in 107 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 296115[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:354: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2250: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.cpp:219: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSCInlines.h:55: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 11 in 108 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 296116[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:354: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2250: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.cpp:219: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSCInlines.h:55: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 11 in 108 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 296119[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:354: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2250: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.cpp:219: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 10 in 106 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296125[details]
Archive of layout-test-results from ews112 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5
Attachment 296127[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:354: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2250: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.cpp:219: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 10 in 106 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 296072[details]
last patch to have confirmed good perf
This patch has a bad race in JSObject::visitChildren. To fix it, I implemented things that could be a slow-down. I'm setting this aside for comparison if needed..
Attachment 296149[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:356: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2256: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.cpp:219: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 10 in 106 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 296210[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:356: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2256: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.cpp:219: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 10 in 106 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 296294[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:356: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2256: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.cpp:219: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 10 in 106 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 296294[details]
the patch
View in context: https://bugs.webkit.org/attachment.cgi?id=296294&action=review
r=me
> Source/JavaScriptCore/ChangeLog:191
> + * runtime/Options.h: Change the default max GC parallelism to 8. I don't know why it was still 7.
Now that we have good locks, I'm pretty sure we can bump to Inf without penalty.
> Source/JavaScriptCore/runtime/JSObject.cpp:166
> + // structure before and after reading the butterfly. For simplicity, lets first consider the case
let's
(In reply to comment #100)
> EWS mac-debug seems to be crashing reliably beneath sputnik/Conformance.
Yeah, I wished it crashed like that locally!
I've now had two runs in a row that didn't crash.
(In reply to comment #101)
> (In reply to comment #100)
> > EWS mac-debug seems to be crashing reliably beneath sputnik/Conformance.
>
> Yeah, I wished it crashed like that locally!
>
> I've now had two runs in a row that didn't crash.
I mean that didn't crash in GC.
I get the same non-GC assertions on every run.
Attachment 296379[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:356: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2261: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.cpp:231: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 10 in 114 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 296467[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/heap/CodeBlockSet.cpp:84: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/heap/MarkedAllocator.h:175: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/ForwardingHeaders/heap/DeleteAllCodeEffort.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:356: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/heap/Heap.cpp:2261: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.cpp:231: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/Structure.cpp:483: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:113: The parameter name "it" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SparseArrayValueMap.h:114: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/LockDuringMarking.h:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
Total errors found: 10 in 115 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #113)
> We don't have a lot of data points yet but this looks like a 2% regression
> on Dromaeo.
Possible 3% regression on JSBench as well.
2016-11-19 11:12 PST, Filip Pizlo
2016-11-22 09:27 PST, Filip Pizlo
2016-11-22 12:58 PST, Filip Pizlo
2016-11-22 13:45 PST, Filip Pizlo
2016-11-23 17:35 PST, Filip Pizlo
2016-11-26 20:52 PST, Filip Pizlo
2016-11-27 13:57 PST, Filip Pizlo
2016-11-27 17:43 PST, Filip Pizlo
2016-11-28 17:58 PST, Filip Pizlo
2016-11-28 21:37 PST, Filip Pizlo
2016-11-28 23:40 PST, Build Bot
2016-11-29 10:24 PST, Filip Pizlo
2016-11-29 11:49 PST, Build Bot
2016-11-29 16:11 PST, Filip Pizlo
2016-11-29 16:32 PST, Filip Pizlo
2016-11-29 18:01 PST, Build Bot
2016-11-29 22:43 PST, Filip Pizlo
2016-11-29 23:07 PST, Filip Pizlo
2016-11-30 03:07 PST, Build Bot
2016-11-30 13:14 PST, Filip Pizlo
2016-11-30 18:36 PST, Build Bot
2016-11-30 20:06 PST, Filip Pizlo
2016-12-01 14:38 PST, Filip Pizlo
2016-12-02 13:20 PST, Filip Pizlo
2016-12-02 15:39 PST, Filip Pizlo
2016-12-02 21:12 PST, Filip Pizlo
2016-12-02 22:41 PST, Filip Pizlo
2016-12-03 06:50 PST, Build Bot
2016-12-03 10:31 PST, Filip Pizlo
2016-12-03 13:04 PST, Build Bot
2016-12-03 13:25 PST, Build Bot
2016-12-03 14:47 PST, Filip Pizlo
2016-12-04 03:43 PST, Build Bot
2016-12-04 12:00 PST, Filip Pizlo
2016-12-04 14:18 PST, Filip Pizlo
2016-12-04 15:46 PST, Filip Pizlo
2016-12-04 17:00 PST, Filip Pizlo
2016-12-04 17:28 PST, Filip Pizlo
2016-12-04 17:43 PST, Filip Pizlo
2016-12-04 18:00 PST, Filip Pizlo
2016-12-04 20:28 PST, Filip Pizlo
2016-12-04 21:32 PST, Build Bot
2016-12-04 21:34 PST, Filip Pizlo
2016-12-04 21:37 PST, Filip Pizlo
2016-12-05 09:33 PST, Filip Pizlo
2016-12-05 15:56 PST, Filip Pizlo
2016-12-05 16:06 PST, Filip Pizlo
2016-12-05 16:15 PST, Filip Pizlo
2016-12-05 16:21 PST, Filip Pizlo
2016-12-06 09:18 PST, Filip Pizlo
2016-12-06 19:33 PST, Filip Pizlo
2016-12-06 22:04 PST, Filip Pizlo
2016-12-06 22:21 PST, Filip Pizlo
2016-12-07 19:54 PST, Filip Pizlo