This seems to fix the JSBench regression.
<rdar://problem/30019309>
Fascinating! What does it mean for the watchpoint to be lazy?
(In reply to comment #2) > Fascinating! > > What does it mean for the watchpoint to be lazy? It means that we don't initialize the watchpoint until the function using it is called.
(In reply to comment #3) > (In reply to comment #2) > > Fascinating! > > > > What does it mean for the watchpoint to be lazy? > > It means that we don't initialize the watchpoint until the function using it > is called. By using, you mean watching or...? Is this an alternate way of saying whether the watchpoint is initialized watched or if the watchpoint is blind initially?
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > Fascinating! > > > > > > What does it mean for the watchpoint to be lazy? > > > > It means that we don't initialize the watchpoint until the function using it > > is called. > > By using, you mean watching or...? > > Is this an alternate way of saying whether the watchpoint is initialized > watched or if the watchpoint is blind initially? The watchpoint is blind initially. So we start it off as ClearWatchpoint. From ClearWatchpoint, once slice is called for the first time, we can either do: ClearWatchpoint => Invalidated ClearWatchpoint => IsWatched depending on the state of the world. We do this transition when slice is called for the fist time and reaches a certain execution point (past some number of exception checks regarding toNumber() on parameters).
Created attachment 298786 [details] patch
Attachment 298786 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 298786 [details] patch Super cool. Looks good to me.
Comment on attachment 298786 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=298786&action=review r=me too. > Source/JavaScriptCore/runtime/ArrayPrototype.cpp:-1362 > - ASSERT(constructorSlot.slotBase() == this); > - ASSERT(constructorSlot.isCacheableValue()); I'm surprised I put these assertions in, they are very clearly wrong... since someone could have changed these before we set the watchpoint.
(In reply to comment #9) > Comment on attachment 298786 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=298786&action=review > > r=me too. > > > Source/JavaScriptCore/runtime/ArrayPrototype.cpp:-1362 > > - ASSERT(constructorSlot.slotBase() == this); > > - ASSERT(constructorSlot.isCacheableValue()); > > I'm surprised I put these assertions in, they are very clearly wrong... > since someone could have changed these before we set the watchpoint. You didn't. I made them assertions because I switched this function to run from global object construction. But since that's no longer the case, I moved back to the checks.
Comment on attachment 298786 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=298786&action=review > Source/JavaScriptCore/runtime/ArrayPrototype.cpp:1411 > + // We only watch this from the DFG, and the DFG makes sure to only start watching if the watchpoint is in the IsWatched state. > + RELEASE_ASSERT(!globalObject->arraySpeciesWatchpoint().isBeingWatched()); Code seems to say is *not* in the IsWatched state.
(In reply to comment #11) > Comment on attachment 298786 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=298786&action=review > > > Source/JavaScriptCore/runtime/ArrayPrototype.cpp:1411 > > + // We only watch this from the DFG, and the DFG makes sure to only start watching if the watchpoint is in the IsWatched state. > > + RELEASE_ASSERT(!globalObject->arraySpeciesWatchpoint().isBeingWatched()); > > Code seems to say is *not* in the IsWatched state. This function name is somewhat confusing, but it's not talking about IsWatched state, it's talking about if anybody is on the Watchpoint's list.
landed in: https://trac.webkit.org/changeset/210745