Bug 182220 - Make MarkedBlock::Footer bigger
Summary: Make MarkedBlock::Footer bigger
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: InRadar
Depends on:
Blocks: 181636
  Show dependency treegraph
 
Reported: 2018-01-27 20:38 PST by Filip Pizlo
Modified: 2018-01-28 11:08 PST (History)
10 users (show)

See Also:


Attachments
the patch (19.33 KB, patch)
2018-01-27 20:42 PST, Filip Pizlo
jfbastien: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews201 for win-future (11.51 MB, application/zip)
2018-01-27 23:20 PST, EWS Watchlist
no flags Details
Archive of layout-test-results from ews206 for win-future (11.55 MB, application/zip)
2018-01-28 01:53 PST, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2018-01-27 20:38:02 PST
Patch forthcoming.
Comment 1 Radar WebKit Bug Importer 2018-01-27 20:38:32 PST
<rdar://problem/36953788>
Comment 2 Filip Pizlo 2018-01-27 20:42:00 PST
Created attachment 332487 [details]
the patch
Comment 3 EWS Watchlist 2018-01-27 23:20:47 PST
Comment on attachment 332487 [details]
the patch

Attachment 332487 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/6237943

New failing tests:
js/dom/array-with-double-assign.html
js/dom/array-with-double-push.html
Comment 4 EWS Watchlist 2018-01-27 23:20:58 PST
Created attachment 332489 [details]
Archive of layout-test-results from ews201 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 5 EWS Watchlist 2018-01-28 01:53:30 PST
Comment on attachment 332487 [details]
the patch

Attachment 332487 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/6238666

New failing tests:
js/dom/array-with-double-assign.html
js/dom/array-with-double-push.html
Comment 6 EWS Watchlist 2018-01-28 01:53:41 PST
Created attachment 332490 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 7 JF Bastien 2018-01-28 08:56:12 PST
Comment on attachment 332487 [details]
the patch

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

r=me since this mostly moves stuff around.

> Source/JavaScriptCore/heap/MarkedBlockInlines.h:148
>          MarkedBlock::Handle* fencedThis = fenceBefore.consume(this);

So these are the only things that can change, and none below need to be consumed? I'm guessing you thought about it, but worth asking anyways.
Comment 8 Filip Pizlo 2018-01-28 09:31:12 PST
(In reply to JF Bastien from comment #7)
> Comment on attachment 332487 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=332487&action=review
> 
> r=me since this mostly moves stuff around.
> 
> > Source/JavaScriptCore/heap/MarkedBlockInlines.h:148
> >          MarkedBlock::Handle* fencedThis = fenceBefore.consume(this);
> 
> So these are the only things that can change, and none below need to be
> consumed? I'm guessing you thought about it, but worth asking anyways.

It's simple, really:

- We're taking a read lock on the block.
- We're threading all pointers to the block through the fence.
- The block lock doesn't have any state that it protects that isn't on the block/footer/handle, all of which get threaded through the fence.

Therefore, we're GTG.
Comment 9 Filip Pizlo 2018-01-28 09:32:27 PST
(In reply to Build Bot from comment #5)
> Comment on attachment 332487 [details]
> the patch
> 
> Attachment 332487 [details] did not pass win-ews (win):
> Output: http://webkit-queues.webkit.org/results/6238666
> 
> New failing tests:
> js/dom/array-with-double-assign.html
> js/dom/array-with-double-push.html

Based on this I'll test 32-bit.

If 32-bit Mac is OK then I will ignore this.
Comment 10 Filip Pizlo 2018-01-28 09:50:39 PST
(In reply to Filip Pizlo from comment #9)
> (In reply to Build Bot from comment #5)
> > Comment on attachment 332487 [details]
> > the patch
> > 
> > Attachment 332487 [details] did not pass win-ews (win):
> > Output: http://webkit-queues.webkit.org/results/6238666
> > 
> > New failing tests:
> > js/dom/array-with-double-assign.html
> > js/dom/array-with-double-push.html
> 
> Based on this I'll test 32-bit.
> 
> If 32-bit Mac is OK then I will ignore this.

32-bit JSC tests are fine.  I'll test 32-bit layout tests next.
Comment 11 Filip Pizlo 2018-01-28 10:28:45 PST
(In reply to Filip Pizlo from comment #10)
> (In reply to Filip Pizlo from comment #9)
> > (In reply to Build Bot from comment #5)
> > > Comment on attachment 332487 [details]
> > > the patch
> > > 
> > > Attachment 332487 [details] did not pass win-ews (win):
> > > Output: http://webkit-queues.webkit.org/results/6238666
> > > 
> > > New failing tests:
> > > js/dom/array-with-double-assign.html
> > > js/dom/array-with-double-push.html
> > 
> > Based on this I'll test 32-bit.
> > 
> > If 32-bit Mac is OK then I will ignore this.
> 
> 32-bit JSC tests are fine.  I'll test 32-bit layout tests next.

I can't build 32-bit. :-(  I'll try a different computer.
Comment 12 Filip Pizlo 2018-01-28 11:07:41 PST
(In reply to Filip Pizlo from comment #11)
> (In reply to Filip Pizlo from comment #10)
> > (In reply to Filip Pizlo from comment #9)
> > > (In reply to Build Bot from comment #5)
> > > > Comment on attachment 332487 [details]
> > > > the patch
> > > > 
> > > > Attachment 332487 [details] did not pass win-ews (win):
> > > > Output: http://webkit-queues.webkit.org/results/6238666
> > > > 
> > > > New failing tests:
> > > > js/dom/array-with-double-assign.html
> > > > js/dom/array-with-double-push.html
> > > 
> > > Based on this I'll test 32-bit.
> > > 
> > > If 32-bit Mac is OK then I will ignore this.
> > 
> > 32-bit JSC tests are fine.  I'll test 32-bit layout tests next.
> 
> I can't build 32-bit. :-(  I'll try a different computer.

I can't build 32-bit at all.  Ima land this.
Comment 13 Filip Pizlo 2018-01-28 11:08:32 PST
Landed in https://trac.webkit.org/changeset/227718/webkit