Bug 164990 - Concurrent GC should be stable enough to land enabled on X86_64
Summary: Concurrent GC should be stable enough to land enabled on X86_64
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 149432
  Show dependency treegraph
 
Reported: 2016-11-19 11:06 PST by Filip Pizlo
Modified: 2016-12-12 09:38 PST (History)
10 users (show)

See Also:


Attachments
getting started (9.60 KB, patch)
2016-11-19 11:12 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
fixed LargeAllocation (16.37 KB, patch)
2016-11-22 09:27 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
fixed CodeBlockSet and InferredValue (19.93 KB, patch)
2016-11-22 12:58 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
better tests (22.00 KB, patch)
2016-11-22 13:45 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
more fixes (23.12 KB, patch)
2016-11-23 17:35 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
even more fixes (24.23 KB, patch)
2016-11-26 20:52 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
getting so stable (33.29 KB, patch)
2016-11-27 13:57 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
fixed another CodeBlock GC bug (34.24 KB, patch)
2016-11-27 17:43 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
added a nice visual benchmark for GCs (206.74 KB, patch)
2016-11-28 17:58 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
passes JSC tests! (214.94 KB, patch)
2016-11-28 21:37 PST, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews117 for mac-yosemite (2.23 MB, application/zip)
2016-11-28 23:40 PST, Build Bot
no flags Details
more fixes (233.93 KB, patch)
2016-11-29 10:24 PST, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews115 for mac-yosemite (2.22 MB, application/zip)
2016-11-29 11:49 PST, Build Bot
no flags Details
fix race in addNewPropertyTransition (239.26 KB, patch)
2016-11-29 16:11 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
with ChangeLog (250.50 KB, patch)
2016-11-29 16:32 PST, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews117 for mac-yosemite (2.43 MB, application/zip)
2016-11-29 18:01 PST, Build Bot
no flags Details
possible fix for MarkedBlock crashes (269.25 KB, patch)
2016-11-29 22:43 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
better fix (269.42 KB, patch)
2016-11-29 23:07 PST, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews116 for mac-yosemite (2.07 MB, application/zip)
2016-11-30 03:07 PST, Build Bot
no flags Details
the patch (281.82 KB, patch)
2016-11-30 13:14 PST, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews116 for mac-yosemite (1.88 MB, application/zip)
2016-11-30 18:36 PST, Build Bot
no flags Details
fix one more bug (289.27 KB, patch)
2016-11-30 20:06 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
audited all visitChildren in JSC (327.36 KB, patch)
2016-12-01 14:38 PST, Filip Pizlo
ggaren: review+
Details | Formatted Diff | Diff
the patch (336.71 KB, patch)
2016-12-02 13:20 PST, Filip Pizlo
ggaren: review+
Details | Formatted Diff | Diff
more locking (347.88 KB, patch)
2016-12-02 15:39 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (338.27 KB, patch)
2016-12-02 21:12 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (338.27 KB, patch)
2016-12-02 22:41 PST, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews114 for mac-yosemite (1.66 MB, application/zip)
2016-12-03 06:50 PST, Build Bot
no flags Details
the patch (338.42 KB, patch)
2016-12-03 10:31 PST, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews114 for mac-yosemite (1.79 MB, application/zip)
2016-12-03 13:04 PST, Build Bot
no flags Details
Archive of layout-test-results from ews101 for mac-yosemite (1.22 MB, application/zip)
2016-12-03 13:25 PST, Build Bot
no flags Details
last patch to have confirmed good perf (339.02 KB, patch)
2016-12-03 14:47 PST, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews117 for mac-yosemite (1.88 MB, application/zip)
2016-12-04 03:43 PST, Build Bot
no flags Details
possible new butterfly concurrency protocol (403.55 KB, patch)
2016-12-04 12:00 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
new protocol is starting to work (410.83 KB, patch)
2016-12-04 14:18 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
and now more correct than ever (419.57 KB, patch)
2016-12-04 15:46 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
passing more tests (422.40 KB, patch)
2016-12-04 17:00 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
maybe? (422.40 KB, patch)
2016-12-04 17:28 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
more fixes (422.88 KB, patch)
2016-12-04 17:43 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
debug runs splay again (422.88 KB, patch)
2016-12-04 18:00 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
more fixes (422.08 KB, patch)
2016-12-04 20:28 PST, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews112 for mac-yosemite (349.90 KB, application/zip)
2016-12-04 21:32 PST, Build Bot
no flags Details
more (429.97 KB, patch)
2016-12-04 21:34 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
it's getting better (429.98 KB, patch)
2016-12-04 21:37 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
more fixes (431.51 KB, patch)
2016-12-05 09:33 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
with updated changelogs (432.54 KB, patch)
2016-12-05 15:56 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (432.54 KB, patch)
2016-12-05 16:06 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (432.54 KB, patch)
2016-12-05 16:15 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (432.49 KB, patch)
2016-12-05 16:21 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (432.75 KB, patch)
2016-12-06 09:18 PST, Filip Pizlo
ggaren: review+
Details | Formatted Diff | Diff
possible patch for landing (450.56 KB, patch)
2016-12-06 19:33 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
with fixes to stopAllocating (451.07 KB, patch)
2016-12-06 22:04 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
rebased patch (450.10 KB, patch)
2016-12-06 22:21 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
more fixes (457.03 KB, patch)
2016-12-07 19:54 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2016-11-19 11:06:28 PST
I think that I'm close enough that I should do this last part as a single patch.
Comment 1 Filip Pizlo 2016-11-19 11:12:17 PST
Created attachment 295265 [details]
getting started
Comment 2 Filip Pizlo 2016-11-19 11:40:38 PST
192 failures!
Comment 3 Filip Pizlo 2016-11-22 09:05:57 PST
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.
Comment 4 Filip Pizlo 2016-11-22 09:21:55 PST
(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.
Comment 5 Filip Pizlo 2016-11-22 09:27:02 PST
Created attachment 295337 [details]
fixed LargeAllocation
Comment 6 Filip Pizlo 2016-11-22 09:32:42 PST
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!
Comment 7 Filip Pizlo 2016-11-22 09:35:18 PST
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.
Comment 8 Filip Pizlo 2016-11-22 10:27:46 PST
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.
Comment 9 Filip Pizlo 2016-11-22 12:58:11 PST
Created attachment 295340 [details]
fixed CodeBlockSet and InferredValue

Down to 60 failures in run-javascriptcor-tests --release.
Comment 10 Filip Pizlo 2016-11-22 13:45:17 PST
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.
Comment 11 Filip Pizlo 2016-11-23 17:35:33 PST
Created attachment 295387 [details]
more fixes

Down to 50 failures.
Comment 12 Filip Pizlo 2016-11-26 20:52:26 PST
Created attachment 295452 [details]
even more fixes

collectContinuously indeed found two pre-existing bugs: one in IsFunction constant folding and another in SamplingProfiler.
Comment 13 Filip Pizlo 2016-11-27 13:57:28 PST
Created attachment 295463 [details]
getting so stable

Fixed race conditions in some UnconditionalFinalizers.
Comment 14 Filip Pizlo 2016-11-27 14:22:08 PST
(In reply to comment #13)
> Created attachment 295463 [details]
> getting so stable
> 
> Fixed race conditions in some UnconditionalFinalizers.

49 failures.
Comment 15 Filip Pizlo 2016-11-27 17:43:32 PST
Created attachment 295477 [details]
fixed another CodeBlock GC bug

I'll retest soon...
Comment 16 Filip Pizlo 2016-11-27 20:20:36 PST
(In reply to comment #15)
> Created attachment 295477 [details]
> fixed another CodeBlock GC bug
> 
> I'll retest soon...

Only 4 test failures.
Comment 17 Filip Pizlo 2016-11-27 21:11:37 PST
Looks like turning on concurrent GC is a perf win.


Benchmark report for SunSpider, LongSpider, Octane, Kraken, and AsmBench on murderface (MacBookPro11,5).

VMs tested:
"TipOfTree" at /Volumes/Data/secondary/OpenSource/WebKitBuild/Release/jsc (r208896)
"Things" at /Volumes/Data/tertiary/OpenSource/WebKitBuild/Release/jsc (r208896)

Collected 6 samples per benchmark/VM, with 6 VM invocations per benchmark. Emitted a call to gc() between sample
measurements. Used 1 benchmark iteration per VM invocation for warm-up. Used the jsc-specific preciseTime() function to
get microsecond-level timing. Reporting benchmark execution times with 95% confidence intervals in milliseconds.

                                                TipOfTree                   Things                                      
SunSpider:
   3d-cube                                    4.5568+-0.2104     ?      4.6104+-0.1811        ? might be 1.0118x slower
   3d-morph                                   4.8957+-0.3341            4.6603+-0.0744          might be 1.0505x faster
   3d-raytrace                                4.4856+-0.1515     ?      4.6117+-0.2347        ? might be 1.0281x slower
   access-binary-trees                        1.9371+-0.0715     ?      1.9637+-0.0310        ? might be 1.0137x slower
   access-fannkuch                            4.7087+-0.1779     ?      4.7810+-0.3312        ? might be 1.0153x slower
   access-nbody                               2.4576+-0.1744            2.4170+-0.0542          might be 1.0168x faster
   access-nsieve                              2.8630+-0.0690            2.8418+-0.0362        
   bitops-3bit-bits-in-byte                   1.1185+-0.0548     ?      1.1661+-0.0999        ? might be 1.0426x slower
   bitops-bits-in-byte                        2.7521+-0.1011            2.7282+-0.0234        
   bitops-bitwise-and                         1.9471+-0.0609     ?      1.9580+-0.0467        ?
   bitops-nsieve-bits                         3.0599+-0.1556     ?      3.0605+-0.0924        ?
   controlflow-recursive                      2.2330+-0.0382     ?      2.2377+-0.0499        ?
   crypto-aes                                 3.9156+-0.0593     ?      3.9187+-0.0827        ?
   crypto-md5                                 2.3680+-0.0535     ?      2.4383+-0.0324        ? might be 1.0297x slower
   crypto-sha1                                2.5566+-0.3177            2.5212+-0.1994          might be 1.0140x faster
   date-format-tofte                          6.8097+-0.1118     ?      6.8496+-0.2203        ?
   date-format-xparb                          4.6393+-0.1944            4.6160+-0.0954        
   math-cordic                                2.7399+-0.1414            2.6518+-0.0339          might be 1.0333x faster
   math-partial-sums                          3.9711+-0.1888     ?      3.9960+-0.2891        ?
   math-spectral-norm                         1.9341+-0.0580     ?      1.9559+-0.0399        ? might be 1.0113x slower
   regexp-dna                                 6.0775+-0.1474     ?      6.3765+-0.5609        ? might be 1.0492x slower
   string-base64                              4.4662+-0.0658     ?      4.5625+-0.2903        ? might be 1.0215x slower
   string-fasta                               5.4991+-0.3321            5.3461+-0.2175          might be 1.0286x faster
   string-tagcloud                            8.1984+-0.1135     ?      8.2316+-0.1361        ?
   string-unpack-code                        18.4231+-0.7940     ?     19.3407+-0.8716        ? might be 1.0498x slower
   string-validate-input                      4.1622+-0.0456     ?      4.2281+-0.1240        ? might be 1.0158x slower

   <arithmetic>                               4.3375+-0.0358     ?      4.3873+-0.0448        ? might be 1.0115x slower

                                                TipOfTree                   Things                                      
LongSpider:
   3d-cube                                  811.3600+-6.7267          796.9972+-13.3162         might be 1.0180x faster
   3d-morph                                 565.0785+-9.0159     ?    567.5167+-6.9989        ?
   3d-raytrace                              448.8218+-2.9032     ?    451.7279+-4.3239        ?
   access-binary-trees                      841.5369+-3.2932          827.4131+-11.2758         might be 1.0171x faster
   access-fannkuch                          236.3149+-12.3910         235.7251+-13.0097       
   access-nbody                             520.3270+-9.1828          520.1399+-4.0402        
   access-nsieve                            283.3927+-3.8265     ?    287.1245+-7.4576        ? might be 1.0132x slower
   bitops-3bit-bits-in-byte                  31.8977+-1.0332           31.7256+-0.9814        
   bitops-bits-in-byte                       82.4250+-6.1793           81.0194+-2.4124          might be 1.0173x faster
   bitops-nsieve-bits                       365.6275+-5.2766     ?    369.2638+-10.5014       ?
   controlflow-recursive                    400.2899+-2.2029     ?    403.0983+-5.7452        ?
   crypto-aes                               543.1446+-11.3802    ?    544.1858+-8.1015        ?
   crypto-md5                               508.7051+-9.0968          503.5114+-4.6654          might be 1.0103x faster
   crypto-sha1                              592.3989+-2.2150          587.7568+-3.9554        
   date-format-tofte                        334.7242+-9.0115     ?    337.0022+-5.9575        ?
   date-format-xparb                        603.6682+-2.2729          602.9265+-1.8511        
   hash-map                                 137.2998+-4.3694     ?    141.3467+-4.2756        ? might be 1.0295x slower
   math-cordic                              399.7740+-3.8280     ?    400.0544+-4.1806        ?
   math-partial-sums                        282.7010+-2.2918          281.0641+-2.4510        
   math-spectral-norm                       518.3232+-5.1667          517.3634+-5.1537        
   string-base64                            469.0892+-7.4604     ^    393.6552+-33.9118       ^ definitely 1.1916x faster
   string-fasta                             337.4228+-8.5378          329.4245+-3.2522          might be 1.0243x faster
   string-tagcloud                          168.1153+-1.0714          165.9889+-4.3208          might be 1.0128x faster

   <geometric>                              339.6449+-1.6040     ^    336.3873+-1.5068        ^ definitely 1.0097x faster

                                                TipOfTree                   Things                                      
Octane:
   encrypt                                   0.14930+-0.00272    ?     0.14989+-0.00260       ?
   decrypt                                   2.53662+-0.01191    ?     2.56198+-0.06151       ?
   deltablue                        x2       0.12235+-0.00179          0.12033+-0.00143         might be 1.0168x faster
   earley                                    0.24699+-0.00099          0.24624+-0.00177       
   boyer                                     4.23707+-0.13908          4.15846+-0.02935         might be 1.0189x faster
   navier-stokes                    x2       4.58244+-0.02375    ?     4.60941+-0.01289       ?
   raytrace                         x2       0.70329+-0.00553          0.70071+-0.00483       
   richards                         x2       0.07868+-0.00067    ?     0.07899+-0.00141       ?
   splay                            x2       0.31799+-0.00502    ^     0.30072+-0.00529       ^ definitely 1.0574x faster
   regexp                           x2      16.49159+-0.94131         16.26655+-0.40570         might be 1.0138x faster
   pdfjs                            x2      40.79251+-0.70817         39.71938+-0.44233         might be 1.0270x faster
   mandreel                         x2      39.83001+-0.36496    ?    40.23303+-0.13319       ? might be 1.0101x slower
   gbemu                            x2      28.67573+-0.54130         28.51590+-0.34242       
   closure                                   0.46759+-0.00650          0.46220+-0.00168         might be 1.0117x faster
   jquery                                    6.24006+-0.02474    !     6.33973+-0.03538       ! definitely 1.0160x slower
   box2d                            x2       8.71716+-0.07835    ?     8.76956+-0.12658       ?
   zlib                             x2     338.25633+-10.90844       329.24019+-14.45725        might be 1.0274x faster
   typescript                       x2     614.11556+-13.53791       607.00456+-7.46490         might be 1.0117x faster

   <geometric>                               4.76284+-0.01919    ^     4.71988+-0.00753       ^ definitely 1.0091x faster

                                                TipOfTree                   Things                                      
Kraken:
   ai-astar                                   88.457+-1.445      ?      90.947+-2.930         ? might be 1.0281x slower
   audio-beat-detection                       35.708+-0.583             35.533+-0.252         
   audio-dft                                  97.777+-2.015             95.347+-2.645           might be 1.0255x faster
   audio-fft                                  27.363+-0.082      ?      27.432+-0.229         ?
   audio-oscillator                           43.233+-0.640      ?      44.191+-1.636         ? might be 1.0222x slower
   imaging-darkroom                           55.380+-0.301      ?      55.780+-0.237         ?
   imaging-desaturate                         40.670+-0.277      ?      41.172+-0.418         ? might be 1.0124x slower
   imaging-gaussian-blur                      59.087+-2.515             57.705+-4.267           might be 1.0240x faster
   json-parse-financial                       33.768+-0.585             33.763+-1.631         
   json-stringify-tinderbox                   21.422+-1.782      ?      21.639+-1.422         ? might be 1.0101x slower
   stanford-crypto-aes                        35.149+-0.543      ?      35.203+-0.646         ?
   stanford-crypto-ccm                        34.934+-3.005             32.311+-2.280           might be 1.0812x faster
   stanford-crypto-pbkdf2                     89.609+-0.539      ?      91.184+-4.659         ? might be 1.0176x slower
   stanford-crypto-sha256-iterative           29.954+-0.447             29.663+-0.700         

   <arithmetic>                               49.465+-0.447             49.419+-0.906           might be 1.0009x faster

                                                TipOfTree                   Things                                      
AsmBench:
   bigfib.cpp                               408.0430+-4.7006          402.9780+-1.1966          might be 1.0126x faster
   cray.c                                   356.6054+-3.9908          356.5528+-3.6843        
   dry.c                                    391.7522+-3.4333     ?    394.7277+-9.2834        ?
   FloatMM.c                                609.5984+-3.4004     ?    621.8640+-27.8287       ? might be 1.0201x slower
   gcc-loops.cpp                           3356.0475+-10.7577    ?   3361.3108+-18.6431       ?
   n-body.c                                 746.7086+-5.1632     ?    750.3443+-1.2910        ?
   Quicksort.c                              372.1285+-6.1118          369.2996+-11.1960       
   stepanov_container.cpp                  3126.0235+-30.6852        3124.4739+-33.6255       
   Towers.c                                 220.9481+-4.5037     ?    222.1495+-4.2271        ?

   <geometric>                              655.5962+-2.5172     ?    656.8712+-2.0530        ? might be 1.0019x slower

                                                TipOfTree                   Things                                      
Geomean of preferred means:
   <scaled-result>                           46.9253+-0.0703           46.8657+-0.1812          might be 1.0013x faster
Comment 18 Filip Pizlo 2016-11-28 17:58:45 PST
Created attachment 295561 [details]
added a nice visual benchmark for GCs
Comment 19 Filip Pizlo 2016-11-28 21:37:29 PST
Created attachment 295579 [details]
passes JSC tests!
Comment 20 WebKit Commit Bot 2016-11-28 22:17:46 PST
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.
Comment 21 Build Bot 2016-11-28 23:40:08 PST
Comment on attachment 295579 [details]
passes JSC tests!

Attachment 295579 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2587109

New failing tests:
js/slow-stress/Int32Array-alloc-large-long-lived.html
js/slow-stress/ArrayBuffer-Int8Array-alloc-large-long-lived-fragmented.html
js/slow-stress/ArrayBuffer-Int8Array-alloc-huge-long-lived.html
Comment 22 Build Bot 2016-11-28 23:40:12 PST
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
Comment 23 Filip Pizlo 2016-11-29 10:24:17 PST
Created attachment 295604 [details]
more fixes
Comment 24 WebKit Commit Bot 2016-11-29 10:26:13 PST
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.
Comment 25 Build Bot 2016-11-29 11:49:04 PST
Comment on attachment 295604 [details]
more fixes

Attachment 295604 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2589818

New failing tests:
js/slow-stress/Int32Array-alloc-large-long-lived.html
fast/events/form-onchange.html
Comment 26 Build Bot 2016-11-29 11:49:09 PST
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
Comment 27 Filip Pizlo 2016-11-29 16:11:30 PST
Created attachment 295667 [details]
fix race in addNewPropertyTransition
Comment 28 WebKit Commit Bot 2016-11-29 16:13:36 PST
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.
Comment 29 Filip Pizlo 2016-11-29 16:32:42 PST
Created attachment 295669 [details]
with ChangeLog
Comment 30 WebKit Commit Bot 2016-11-29 16:35:28 PST
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.
Comment 31 Build Bot 2016-11-29 18:00:59 PST
Comment on attachment 295669 [details]
with ChangeLog

Attachment 295669 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2591821

New failing tests:
js/slow-stress/Int32Array-alloc-large-long-lived.html
js/slow-stress/ArrayBuffer-Int8Array-alloc-large-long-lived-fragmented.html
js/slow-stress/nested-function-parsing-random.html
Comment 32 Build Bot 2016-11-29 18:01:04 PST
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
Comment 33 Filip Pizlo 2016-11-29 21:38:39 PST
(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.
Comment 34 Filip Pizlo 2016-11-29 22:43:11 PST
Created attachment 295704 [details]
possible fix for MarkedBlock crashes

Still testing this.  Not sure if this is really right.
Comment 35 WebKit Commit Bot 2016-11-29 22:44:23 PST
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.
Comment 36 Filip Pizlo 2016-11-29 23:07:02 PST
Created attachment 295706 [details]
better fix
Comment 37 WebKit Commit Bot 2016-11-29 23:09:38 PST
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.
Comment 38 Build Bot 2016-11-30 03:07:19 PST
Comment on attachment 295706 [details]
better fix

Attachment 295706 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2594171

New failing tests:
inspector/console/clearMessages.html
Comment 39 Build Bot 2016-11-30 03:07:25 PST
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
Comment 40 Filip Pizlo 2016-11-30 13:14:26 PST
Created attachment 295746 [details]
the patch
Comment 41 Filip Pizlo 2016-11-30 13:14:51 PST
(In reply to comment #40)
> Created attachment 295746 [details]
> the patch

I don't know if it's stable enough yet, but it's rapidly getting there.
Comment 42 WebKit Commit Bot 2016-11-30 13:16:12 PST
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.
Comment 43 Build Bot 2016-11-30 18:36:07 PST
Comment on attachment 295746 [details]
the patch

Attachment 295746 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2598341

New failing tests:
inspector/console/clearMessages.html
Comment 44 Build Bot 2016-11-30 18:36:12 PST
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
Comment 45 Filip Pizlo 2016-11-30 20:06:08 PST
Created attachment 295812 [details]
fix one more bug
Comment 46 WebKit Commit Bot 2016-11-30 20:07:56 PST
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 47 Filip Pizlo 2016-12-01 14:38:48 PST
Created attachment 295898 [details]
audited all visitChildren in JSC
Comment 48 Geoffrey Garen 2016-12-02 12:41:30 PST
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.
Comment 49 Filip Pizlo 2016-12-02 13:20:14 PST
Created attachment 295987 [details]
the patch
Comment 50 Geoffrey Garen 2016-12-02 15:05:24 PST
Comment on attachment 295987 [details]
the patch

r=me
Comment 51 Filip Pizlo 2016-12-02 15:39:03 PST
Created attachment 296012 [details]
more locking

Fixed a couple races in WebCore.
Comment 52 Filip Pizlo 2016-12-02 21:12:17 PST
Created attachment 296039 [details]
the patch

Lots of fixes in WebCore
Comment 53 WebKit Commit Bot 2016-12-02 21:14:33 PST
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.
Comment 54 Filip Pizlo 2016-12-02 22:41:16 PST
Created attachment 296042 [details]
the patch

More fixes.
Comment 55 WebKit Commit Bot 2016-12-02 22:43:55 PST
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.
Comment 56 Build Bot 2016-12-03 06:49:58 PST
Comment on attachment 296042 [details]
the patch

Attachment 296042 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2613104

New failing tests:
media/modern-media-controls/media-controller/media-controller-resize.html
Comment 57 Build Bot 2016-12-03 06:50:04 PST
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
Comment 58 Filip Pizlo 2016-12-03 10:31:07 PST
Created attachment 296054 [details]
the patch
Comment 59 WebKit Commit Bot 2016-12-03 10:34:07 PST
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.
Comment 60 Build Bot 2016-12-03 13:04:04 PST
Comment on attachment 296054 [details]
the patch

Attachment 296054 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2615085

New failing tests:
media/modern-media-controls/media-controller/media-controller-resize.html
Comment 61 Build Bot 2016-12-03 13:04:09 PST
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
Comment 62 Build Bot 2016-12-03 13:25:11 PST
Comment on attachment 296054 [details]
the patch

Attachment 296054 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2615194

New failing tests:
media/modern-media-controls/media-controller/media-controller-resize.html
Comment 63 Build Bot 2016-12-03 13:25:16 PST
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
Comment 64 Filip Pizlo 2016-12-03 14:47:22 PST
(In reply to comment #63)
> 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

Seems unlikely that this is GC:

Thread 21 Crashed:: com.apple.coremedia.player.async
0   libsystem_kernel.dylib        	0x00007fff91e03286 __pthread_kill + 10
1   libsystem_c.dylib             	0x00007fff8b5239a3 abort + 129
2   libsystem_malloc.dylib        	0x00007fff9644dfe2 szone_error + 625
3   libsystem_malloc.dylib        	0x00007fff96441d76 tiny_malloc_from_free_list + 1358
4   libsystem_malloc.dylib        	0x00007fff964407e4 szone_malloc_should_clear + 317
5   libsystem_malloc.dylib        	0x00007fff96440667 malloc_zone_malloc + 71
6   libsystem_malloc.dylib        	0x00007fff9643f187 malloc + 42
7   com.apple.CoreMedia           	0x00007fff8e4d4deb FigSimpleMutexCreateWithAttr + 20
8   com.apple.CoreMedia           	0x00007fff8e4d4f67 FigReentrantMutexCreate + 69
9   com.apple.CoreMedia           	0x00007fff8e4d4ff8 FigSemaphoreCreate + 46
10  com.apple.CoreMedia           	0x00007fff8e518989 FigReadWriteLockCreate + 71
11  com.apple.CoreMedia           	0x00007fff8e4bd2a6 figTimebaseCreate + 125
12  com.apple.CoreMedia           	0x00007fff8e4be14b CMTimebaseCreateWithMasterClock + 54
13  com.apple.MediaToolbox        	0x00007fff9796a283 FigPlayabilityMonitorCreate + 335
14  com.apple.MediaToolbox        	0x00007fff979f02b6 0x7fff97929000 + 815798
15  com.apple.MediaToolbox        	0x00007fff979ec305 0x7fff97929000 + 799493
16  com.apple.MediaToolbox        	0x00007fff979e1825 0x7fff97929000 + 755749
17  com.apple.MediaToolbox        	0x00007fff979e4790 0x7fff97929000 + 767888
18  com.apple.MediaToolbox        	0x00007fff97a078aa 0x7fff97929000 + 911530
19  com.apple.MediaToolbox        	0x00007fff979e0a59 0x7fff97929000 + 752217
20  com.apple.CoreMedia           	0x00007fff8e4d5ef5 figThreadMain + 417
21  libsystem_pthread.dylib       	0x00007fff9720205a _pthread_body + 131
22  libsystem_pthread.dylib       	0x00007fff97201fd7 _pthread_start + 176
23  libsystem_pthread.dylib       	0x00007fff971ff3ed thread_start + 13
Comment 65 Filip Pizlo 2016-12-03 14:47:44 PST
Created attachment 296072 [details]
last patch to have confirmed good perf

Fixed another crash.
Comment 66 WebKit Commit Bot 2016-12-03 14:51:03 PST
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.
Comment 67 Build Bot 2016-12-04 03:42:58 PST
Comment on attachment 296072 [details]
last patch to have confirmed good perf

Attachment 296072 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2620190

New failing tests:
inspector/unit-tests/url-utilities.html
Comment 68 Build Bot 2016-12-04 03:43:04 PST
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
Comment 69 Filip Pizlo 2016-12-04 12:00:03 PST
Created attachment 296094 [details]
possible new butterfly concurrency protocol

Dunno if it even compiles.
Comment 70 Filip Pizlo 2016-12-04 14:18:50 PST
Created attachment 296106 [details]
new protocol is starting to work
Comment 71 Filip Pizlo 2016-12-04 14:36:34 PST
(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.
Comment 72 Filip Pizlo 2016-12-04 15:46:21 PST
Created attachment 296110 [details]
and now more correct than ever
Comment 73 Filip Pizlo 2016-12-04 17:00:19 PST
Created attachment 296112 [details]
passing more tests
Comment 74 Filip Pizlo 2016-12-04 17:28:43 PST
Created attachment 296114 [details]
maybe?
Comment 75 WebKit Commit Bot 2016-12-04 17:32:18 PST
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.
Comment 76 Filip Pizlo 2016-12-04 17:43:13 PST
Created attachment 296115 [details]
more fixes
Comment 77 WebKit Commit Bot 2016-12-04 17:46:05 PST
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.
Comment 78 Filip Pizlo 2016-12-04 18:00:48 PST
Created attachment 296116 [details]
debug runs splay again
Comment 79 WebKit Commit Bot 2016-12-04 18:03:53 PST
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.
Comment 80 Filip Pizlo 2016-12-04 20:28:00 PST
Created attachment 296119 [details]
more fixes
Comment 81 WebKit Commit Bot 2016-12-04 20:31:08 PST
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.
Comment 82 Build Bot 2016-12-04 21:32:06 PST
Comment on attachment 296119 [details]
more fixes

Attachment 296119 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2624395

Number of test failures exceeded the failure limit.
Comment 83 Build Bot 2016-12-04 21:32:12 PST
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
Comment 84 Filip Pizlo 2016-12-04 21:34:48 PST
Created attachment 296126 [details]
more
Comment 85 Filip Pizlo 2016-12-04 21:37:23 PST
Created attachment 296127 [details]
it's getting better

My previous attempt at a build fix actually introduced a bug. :-(

This should fix it.
Comment 86 WebKit Commit Bot 2016-12-04 21:40:01 PST
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 87 Filip Pizlo 2016-12-05 09:32:42 PST
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..
Comment 88 Filip Pizlo 2016-12-05 09:33:17 PST
Created attachment 296149 [details]
more fixes
Comment 89 WebKit Commit Bot 2016-12-05 09:35:50 PST
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.
Comment 90 Filip Pizlo 2016-12-05 15:51:13 PST
Comment on attachment 296072 [details]
last patch to have confirmed good perf

OK - I now have detailed perf data on the new, more correct, version.
Comment 91 Filip Pizlo 2016-12-05 15:56:56 PST
Created attachment 296206 [details]
with updated changelogs
Comment 92 Filip Pizlo 2016-12-05 16:06:09 PST
Created attachment 296208 [details]
the patch

Rebased.
Comment 93 Filip Pizlo 2016-12-05 16:15:08 PST
Created attachment 296209 [details]
the patch
Comment 94 Filip Pizlo 2016-12-05 16:21:25 PST
Created attachment 296210 [details]
the patch

For real this time.
Comment 95 WebKit Commit Bot 2016-12-05 16:24:38 PST
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.
Comment 96 Filip Pizlo 2016-12-05 17:37:06 PST
My local debug RWT run showed no GC crashes.

The Mac bot is red because of a EWS glitch, compile fail in WebCore code I didn't touch.
Comment 97 Filip Pizlo 2016-12-06 09:18:30 PST
Created attachment 296294 [details]
the patch
Comment 98 WebKit Commit Bot 2016-12-06 09:22:15 PST
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 99 Geoffrey Garen 2016-12-06 15:18:58 PST
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
Comment 100 Geoffrey Garen 2016-12-06 15:20:19 PST
EWS mac-debug seems to be crashing reliably beneath sputnik/Conformance.
Comment 101 Filip Pizlo 2016-12-06 18:34:10 PST
(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.
Comment 102 Filip Pizlo 2016-12-06 18:34:47 PST
(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.
Comment 103 Filip Pizlo 2016-12-06 19:33:46 PST
Created attachment 296365 [details]
possible patch for landing
Comment 104 Filip Pizlo 2016-12-06 22:04:12 PST
Created attachment 296376 [details]
with fixes to stopAllocating
Comment 105 Filip Pizlo 2016-12-06 22:21:06 PST
Created attachment 296379 [details]
rebased patch
Comment 106 WebKit Commit Bot 2016-12-06 22:23:54 PST
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.
Comment 107 Filip Pizlo 2016-12-07 19:54:01 PST
Created attachment 296467 [details]
more fixes

I found and fixed another race condition.
Comment 108 WebKit Commit Bot 2016-12-07 19:57:47 PST
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.
Comment 109 Filip Pizlo 2016-12-08 14:15:44 PST
Landed in http://trac.webkit.org/changeset/209570
Comment 110 Csaba Osztrogonác 2016-12-09 01:51:39 PST
(In reply to comment #109)
> Landed in http://trac.webkit.org/changeset/209570

ARM buildfix landed in https://trac.webkit.org/changeset/209600
Comment 111 Csaba Osztrogonác 2016-12-09 02:53:21 PST
Comment on attachment 296467 [details]
more fixes

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

> Source/WebCore/bindings/js/JSDOMGlobalObject.cpp:127
>  #endif
> +    }

It is misplaced, it should be before the #endif.
Comment 112 Csaba Osztrogonác 2016-12-09 02:54:03 PST
(In reply to comment #111)
> Comment on attachment 296467 [details]
> more fixes
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=296467&action=review
> 
> > Source/WebCore/bindings/js/JSDOMGlobalObject.cpp:127
> >  #endif
> > +    }
> 
> It is misplaced, it should be before the #endif.

Fix landed in https://trac.webkit.org/changeset/209602
Comment 113 Chris Dumez 2016-12-09 10:57:08 PST
We don't have a lot of data points yet but this looks like a 2% regression on Dromaeo.
Comment 114 Chris Dumez 2016-12-09 10:58:12 PST
(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.
Comment 115 Filip Pizlo 2016-12-09 11:11:48 PST
(In reply to comment #113)
> We don't have a lot of data points yet but this looks like a 2% regression
> on Dromaeo.

Related: https://bugs.webkit.org/show_bug.cgi?id=165662
Comment 116 Chris Dumez 2016-12-12 09:38:46 PST
Note that when this re-landed in r209692, it was again a regression on several benchmarks it seems:
- 1 to 2% on Dromaeo JSLib
- 6% on JSBench