Bug 182220 – Make MarkedBlock::Footer bigger

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

Patch forthcoming.

[Radar WebKit Bug Importer]

<rdar://problem/36953788>

[Filip Pizlo]

Created attachment 332487 [details]
the patch

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[JF Bastien]

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.

[Filip Pizlo]

(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.

[Filip Pizlo]

(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.

[Filip Pizlo]

(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.

[Filip Pizlo]

(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.

[Filip Pizlo]

(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.

[Filip Pizlo]

Landed in https://trac.webkit.org/changeset/227718/webkit