Attachment 341847 Details for Bug 186237 – the patch

[patch] the patch

blah.patch (text/plain), 3.53 KB, created by Filip Pizlo on 2018-06-02 10:34:05 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2018-06-02 10:34:05 PDT

Size: 3.53 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 232439)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,29 @@
>+2018-06-02  Filip Pizlo  <fpizlo@apple.com>
>+
>+        FunctionRareData::m_objectAllocationProfileWatchpoint is racy
>+        https://bugs.webkit.org/show_bug.cgi?id=186237
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We initialize it blind and let it go into auto-watch mode once the DFG adds a watchpoint, but
>+        that means that we never notice that it fired if it fires between when the DFG decides to
>+        watch it and when it actually adds the watchpoint.
>+        
>+        Most watchpoints are initialized watched for this purpose. This one had a somewhat good
>+        reason for being initialized blind: that's how we knew to ignore changes to the prototype
>+        before the first allocation. However, that functionality also arose out of the fact that the
>+        rare data is created lazily and usually won't exist until the first allocation.
>+        
>+        The fix here is to make the watchpoint go into watched mode as soon as we initialize the
>+        object allocation profile.
>+        
>+        It's hard to repro this race, however it started causing spurious test failures for me after
>+        bug 164904.
>+
>+        * runtime/FunctionRareData.cpp:
>+        (JSC::FunctionRareData::FunctionRareData):
>+        (JSC::FunctionRareData::initializeObjectAllocationProfile):
>+
> 2018-06-02  Caio Lima  <ticaiolima@gmail.com>
> 
>         [ESNext][BigInt] Implement support for addition operations
>Index: Source/JavaScriptCore/runtime/FunctionRareData.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/FunctionRareData.cpp	(revision 232227)
>+++ Source/JavaScriptCore/runtime/FunctionRareData.cpp	(working copy)
>@@ -64,14 +64,8 @@ FunctionRareData::FunctionRareData(VM& v
>     : Base(vm, vm.functionRareDataStructure.get())
>     , m_objectAllocationProfile()
>     // We initialize blind so that changes to the prototype after function creation but before
>-    // the optimizer kicks in don't disable optimizations. Once the optimizer kicks in, the
>-    // watchpoint will start watching and any changes will both force deoptimization and disable
>-    // future attempts to optimize. This is necessary because we are guaranteed that the
>-    // allocation profile is changed exactly once prior to optimizations kicking in. We could be
>-    // smarter and count the number of times the prototype is clobbered and only optimize if it
>-    // was clobbered exactly once, but that seems like overkill. In almost all cases it will be
>-    // clobbered once, and if it's clobbered more than once, that will probably only occur
>-    // before we started optimizing, anyway.
>+    // the first allocation don't disable optimizations. This isn't super important, since the
>+    // function is unlikely to allocate a rare data until the first allocation anyway.
>     , m_objectAllocationProfileWatchpoint(ClearWatchpoint)
> {
> }
>@@ -82,6 +76,9 @@ FunctionRareData::~FunctionRareData()
> 
> void FunctionRareData::initializeObjectAllocationProfile(VM& vm, JSGlobalObject* globalObject, JSObject* prototype, size_t inlineCapacity, JSFunction* constructor)
> {
>+    if (m_objectAllocationProfileWatchpoint.isStillValid())
>+        m_objectAllocationProfileWatchpoint.startWatching();
>+    
>     m_objectAllocationProfile.initializeProfile(vm, globalObject, this, prototype, inlineCapacity, constructor, this);
> }
>

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 186237: 341847