Slight Octane regression from concurrent GC's eager object zero-fill

• Component: JavaScriptCore
• Status: RESOLVED WONTFIX

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 294946 [details]
one possible approach

This fixes the eager zero-fill performance problems by doing the zero-fill conditionally.

But I'm also going to attempt a fix where the GC manages the zero-filling.

[Filip Pizlo]

(In reply to comment #1)
> Created attachment 294946 [details]
> one possible approach
> 
> This fixes the eager zero-fill performance problems by doing the zero-fill
> conditionally.
> 
> But I'm also going to attempt a fix where the GC manages the zero-filling.

Thinking about it more, doing the zero-filling in GC doesn't work. It fails to protect us from the case where the collector races with a transition on an object that was allocated without eager zero-fill.

Also, the GC zero-fill patch was not really faster.

[Filip Pizlo]

Created attachment 294967 [details]
the patch

[Geoffrey Garen]

Comment on attachment 294967 [details]
the patch

r=me

[Filip Pizlo]

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

[Filip Pizlo]

I'm beginning to think that no version of this optimization is sound: it's possible that an object allocated before GC started, and so without zero-fill, could be part of a memory ordering race while the GC is running.

I think that a better approach would be to remove object initializations and then put fences on PutStructure. To make that efficient on ARM, we would have to do redundant PutStructure elimination in DFG IR.

[Filip Pizlo]

(In reply to comment #6)
> I'm beginning to think that no version of this optimization is sound: it's
> possible that an object allocated before GC started, and so without
> zero-fill, could be part of a memory ordering race while the GC is running.
> 
> I think that a better approach would be to remove object initializations and
> then put fences on PutStructure. To make that efficient on ARM, we would
> have to do redundant PutStructure elimination in DFG IR.

Rolled out in r208822.

[Chris Dumez]

(In reply to comment #5)
> Landed in https://trac.webkit.org/changeset/208811

FYI, one of our Mac bots has caught up with r208811 and there was no progression on Octane.

[Chris Dumez]

(In reply to comment #8)
> (In reply to comment #5)
> > Landed in https://trac.webkit.org/changeset/208811
> 
> FYI, one of our Mac bots has caught up with r208811 and there was no
> progression on Octane.

FYI, A/B testing shows there was no Speedometer progression from r208811 on ARM either.

[Filip Pizlo]

I don't think that we know that there was actually any regression due to this specifically, based on subsequent testing.