Title: [232598] trunk/Source/_javascript_Core
- Revision
- 232598
- Author
- [email protected]
- Date
- 2018-06-07 14:01:19 -0700 (Thu, 07 Jun 2018)
Log Message
FunctionRareData::m_objectAllocationProfileWatchpoint is racy
https://bugs.webkit.org/show_bug.cgi?id=186237
Reviewed by Saam Barati.
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):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (232597 => 232598)
--- trunk/Source/_javascript_Core/ChangeLog 2018-06-07 20:45:19 UTC (rev 232597)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-06-07 21:01:19 UTC (rev 232598)
@@ -1,3 +1,29 @@
+2018-06-02 Filip Pizlo <[email protected]>
+
+ FunctionRareData::m_objectAllocationProfileWatchpoint is racy
+ https://bugs.webkit.org/show_bug.cgi?id=186237
+
+ Reviewed by Saam Barati.
+
+ 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-07 Saam Barati <[email protected]>
Make DFG to FTL OSR entry code more sane by removing bad RELEASE_ASSERTS and making it trigger compiles in outer loops before inner ones
Modified: trunk/Source/_javascript_Core/runtime/FunctionRareData.cpp (232597 => 232598)
--- trunk/Source/_javascript_Core/runtime/FunctionRareData.cpp 2018-06-07 20:45:19 UTC (rev 232597)
+++ trunk/Source/_javascript_Core/runtime/FunctionRareData.cpp 2018-06-07 21:01:19 UTC (rev 232598)
@@ -64,14 +64,8 @@
: 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 @@
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);
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
