Skip to content
Permalink
Browse files
The compiler should always register a structure when it adds its tran…
…sitionWatchPointSet.

https://bugs.webkit.org/show_bug.cgi?id=178420
<rdar://problem/34814024>

Reviewed by Saam Barati and Filip Pizlo.

JSTests:

* stress/regress-178420.js: Added.
(new.Array.10000.map):

Source/JavaScriptCore:

Instead of invoking addLazily() to add a structure's transitionWatchpointSet, we
now invoke Graph::registerAndWatchStructureTransition() on the structure.
registerAndWatchStructureTransition() both registers the structure and add its
transitionWatchpointSet to the plan desired watchpoints.

Graph::registerAndWatchStructureTransition() is based on Graph::registerStructure()
except registerAndWatchStructureTransition() adds the structure's
transitionWatchpointSet unconditionally.

* dfg/DFGArgumentsEliminationPhase.cpp:
* dfg/DFGArrayMode.cpp:
(JSC::DFG::ArrayMode::refine const):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleIntrinsicCall):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):

* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::registerAndWatchStructureTransition):
* dfg/DFGGraph.h:

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileGetByValOnString):
- The second set of addLazily()s is redundant.  This set is executed only when
  prototypeChainIsSane is true, and prototypeChainIsSane can only be true if and
  only if we've executed the if statement above it.  That preceding if statement
  already registerAndWatchStructureTransition() the same 2 structures.  Hence,
  this second set can be deleted.

* dfg/DFGWatchpointCollectionPhase.cpp:
(JSC::DFG::WatchpointCollectionPhase::addLazily):
- Deleted an unused function.

* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):



Canonical link: https://commits.webkit.org/194668@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223614 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Mark Lam committed Oct 18, 2017
1 parent 297c8c2 commit fa393ff2a725c03d07099df8d7983f5391ca6ad7
@@ -1,3 +1,14 @@
2017-10-18 Mark Lam <mark.lam@apple.com>

The compiler should always register a structure when it adds its transitionWatchPointSet.
https://bugs.webkit.org/show_bug.cgi?id=178420
<rdar://problem/34814024>

Reviewed by Saam Barati and Filip Pizlo.

* stress/regress-178420.js: Added.
(new.Array.10000.map):

2017-10-18 Yusuke Suzuki <utatane.tea@gmail.com>

[JSC] __proto__ getter should be fast
@@ -0,0 +1,17 @@
// This test passes if it does not crash.

var arr0 = [42];
var arr4 = [,,,,,,,,,,,,,,,,,,,,,,,,];

new Array(10000).map((function() {
arr4[-35] = 1.1;
}), this);

arr0[0] = [];
gc();

Array.prototype.__proto__ = {};
gc();

for(var i = 0; i < 65536; i++)
arr0['a'+i] = 1.1;
@@ -1,3 +1,47 @@
2017-10-18 Mark Lam <mark.lam@apple.com>

The compiler should always register a structure when it adds its transitionWatchPointSet.
https://bugs.webkit.org/show_bug.cgi?id=178420
<rdar://problem/34814024>

Reviewed by Saam Barati and Filip Pizlo.

Instead of invoking addLazily() to add a structure's transitionWatchpointSet, we
now invoke Graph::registerAndWatchStructureTransition() on the structure.
registerAndWatchStructureTransition() both registers the structure and add its
transitionWatchpointSet to the plan desired watchpoints.

Graph::registerAndWatchStructureTransition() is based on Graph::registerStructure()
except registerAndWatchStructureTransition() adds the structure's
transitionWatchpointSet unconditionally.

* dfg/DFGArgumentsEliminationPhase.cpp:
* dfg/DFGArrayMode.cpp:
(JSC::DFG::ArrayMode::refine const):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleIntrinsicCall):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):

* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::registerAndWatchStructureTransition):
* dfg/DFGGraph.h:

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileGetByValOnString):
- The second set of addLazily()s is redundant. This set is executed only when
prototypeChainIsSane is true, and prototypeChainIsSane can only be true if and
only if we've executed the if statement above it. That preceding if statement
already registerAndWatchStructureTransition() the same 2 structures. Hence,
this second set can be deleted.

* dfg/DFGWatchpointCollectionPhase.cpp:
(JSC::DFG::WatchpointCollectionPhase::addLazily):
- Deleted an unused function.

* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):

2017-10-18 Yusuke Suzuki <utatane.tea@gmail.com>

[JSC] Remove unused private name structure
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2015-2016 Apple Inc. All rights reserved.
* Copyright (C) 2015-2017 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -248,20 +248,20 @@ class ArgumentsEliminationPhase : public Phase {
// If we're out-of-bounds then we proceed only if the prototype chain
// for the allocation is sane (i.e. doesn't have indexed properties).
JSGlobalObject* globalObject = m_graph.globalObjectFor(edge->origin.semantic);
InlineWatchpointSet& objectPrototypeTransition = globalObject->objectPrototype()->structure()->transitionWatchpointSet();
Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure();
if (edge->op() == CreateRest) {
InlineWatchpointSet& arrayPrototypeTransition = globalObject->arrayPrototype()->structure()->transitionWatchpointSet();
if (arrayPrototypeTransition.isStillValid()
&& objectPrototypeTransition.isStillValid()
Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure();
if (arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
&& objectPrototypeStructure->transitionWatchpointSetIsStillValid()
&& globalObject->arrayPrototypeChainIsSane()) {
m_graph.watchpoints().addLazily(arrayPrototypeTransition);
m_graph.watchpoints().addLazily(objectPrototypeTransition);
m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
break;
}
} else {
if (objectPrototypeTransition.isStillValid()
if (objectPrototypeStructure->transitionWatchpointSetIsStillValid()
&& globalObject->objectPrototypeIsSane()) {
m_graph.watchpoints().addLazily(objectPrototypeTransition);
m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
break;
}
}
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2012-2016 Apple Inc. All rights reserved.
* Copyright (C) 2012-2017 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -214,8 +214,8 @@ ArrayMode ArrayMode::refine(
&& arrayClass() == Array::OriginalArray
&& globalObject->arrayPrototypeChainIsSane()
&& !graph.hasExitSite(node->origin.semantic, OutOfBounds)) {
graph.watchpoints().addLazily(globalObject->arrayPrototype()->structure()->transitionWatchpointSet());
graph.watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
graph.registerAndWatchStructureTransition(globalObject->arrayPrototype()->structure());
graph.registerAndWatchStructureTransition(globalObject->objectPrototype()->structure());
if (globalObject->arrayPrototypeChainIsSane())
return withSpeculation(Array::SaneChain);
}
@@ -2229,21 +2229,21 @@ bool ByteCodeParser::handleIntrinsicCall(Node* callee, int resultOperand, Intrin
case Array::Contiguous: {
JSGlobalObject* globalObject = m_graph.globalObjectFor(currentNodeOrigin().semantic);

InlineWatchpointSet& objectPrototypeTransition = globalObject->objectPrototype()->structure()->transitionWatchpointSet();
InlineWatchpointSet& arrayPrototypeTransition = globalObject->arrayPrototype()->structure()->transitionWatchpointSet();
Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure();
Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure();

// FIXME: We could easily relax the Array/Object.prototype transition as long as we OSR exitted if we saw a hole.
// https://bugs.webkit.org/show_bug.cgi?id=173171
if (globalObject->arraySpeciesWatchpoint().state() == IsWatched
&& globalObject->havingABadTimeWatchpoint()->isStillValid()
&& arrayPrototypeTransition.isStillValid()
&& objectPrototypeTransition.isStillValid()
&& arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
&& objectPrototypeStructure->transitionWatchpointSetIsStillValid()
&& globalObject->arrayPrototypeChainIsSane()) {

m_graph.watchpoints().addLazily(globalObject->arraySpeciesWatchpoint());
m_graph.watchpoints().addLazily(globalObject->havingABadTimeWatchpoint());
m_graph.watchpoints().addLazily(arrayPrototypeTransition);
m_graph.watchpoints().addLazily(objectPrototypeTransition);
m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);

insertChecks();

@@ -2318,19 +2318,19 @@ bool ByteCodeParser::handleIntrinsicCall(Node* callee, int resultOperand, Intrin
case Array::Contiguous: {
JSGlobalObject* globalObject = m_graph.globalObjectFor(currentNodeOrigin().semantic);

InlineWatchpointSet& objectPrototypeTransition = globalObject->objectPrototype()->structure()->transitionWatchpointSet();
InlineWatchpointSet& arrayPrototypeTransition = globalObject->arrayPrototype()->structure()->transitionWatchpointSet();
Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure();
Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure();

// FIXME: We could easily relax the Array/Object.prototype transition as long as we OSR exitted if we saw a hole.
// https://bugs.webkit.org/show_bug.cgi?id=173171
if (globalObject->havingABadTimeWatchpoint()->isStillValid()
&& arrayPrototypeTransition.isStillValid()
&& objectPrototypeTransition.isStillValid()
&& arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
&& objectPrototypeStructure->transitionWatchpointSetIsStillValid()
&& globalObject->arrayPrototypeChainIsSane()) {

m_graph.watchpoints().addLazily(globalObject->havingABadTimeWatchpoint());
m_graph.watchpoints().addLazily(arrayPrototypeTransition);
m_graph.watchpoints().addLazily(objectPrototypeTransition);
m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);

insertChecks();

@@ -759,10 +759,8 @@ class FixupPhase : public Phase {
}

if (canDoSaneChain) {
m_graph.watchpoints().addLazily(
globalObject->arrayPrototype()->structure()->transitionWatchpointSet());
m_graph.watchpoints().addLazily(
globalObject->objectPrototype()->structure()->transitionWatchpointSet());
m_graph.registerAndWatchStructureTransition(globalObject->arrayPrototype()->structure());
m_graph.registerAndWatchStructureTransition(globalObject->objectPrototype()->structure());
if (globalObject->arrayPrototypeChainIsSane())
node->setArrayMode(arrayMode.withSpeculation(Array::SaneChain));
}
@@ -1218,16 +1216,16 @@ class FixupPhase : public Phase {
// When we go down the fast path, we don't consult the prototype chain, so we must prove
// that it doesn't contain any indexed properties, and that any holes will result in
// jsUndefined().
InlineWatchpointSet& objectPrototypeTransition = globalObject->objectPrototype()->structure()->transitionWatchpointSet();
InlineWatchpointSet& arrayPrototypeTransition = globalObject->arrayPrototype()->structure()->transitionWatchpointSet();
if (node->child1()->shouldSpeculateArray()
&& arrayPrototypeTransition.isStillValid()
&& objectPrototypeTransition.isStillValid()
Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure();
Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure();
if (node->child1()->shouldSpeculateArray()
&& arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
&& objectPrototypeStructure->transitionWatchpointSetIsStillValid()
&& globalObject->arrayPrototypeChainIsSane()
&& m_graph.isWatchingArrayIteratorProtocolWatchpoint(node->child1().node())
&& m_graph.isWatchingHavingABadTimeWatchpoint(node->child1().node())) {
m_graph.watchpoints().addLazily(objectPrototypeTransition);
m_graph.watchpoints().addLazily(arrayPrototypeTransition);
m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
fixEdge<ArrayUse>(node->child1());
} else
fixEdge<CellUse>(node->child1());
@@ -1502,6 +1502,12 @@ RegisteredStructure Graph::registerStructure(Structure* structure, StructureRegi
return RegisteredStructure::createPrivate(structure);
}

void Graph::registerAndWatchStructureTransition(Structure* structure)
{
m_plan.weakReferences.addLazily(structure);
m_plan.watchpoints.addLazily(structure->transitionWatchpointSet());
}

void Graph::assertIsRegistered(Structure* structure)
{
// It's convenient to be able to call this with a maybe-null structure.
@@ -228,6 +228,7 @@ class Graph : public virtual Scannable {
return registerStructure(structure, ignored);
}
RegisteredStructure registerStructure(Structure*, StructureRegistrationResult&);
void registerAndWatchStructureTransition(Structure*);
void assertIsRegistered(Structure* structure);

// CodeBlock is optional, but may allow additional information to be dumped (e.g. Identifier names).
@@ -2137,14 +2137,11 @@ void SpeculativeJIT::compileGetByValOnString(Node* node)
// on a stringPrototypeChainIsSane() guaranteeing that the prototypes have no negative
// indexed properties either.
// https://bugs.webkit.org/show_bug.cgi?id=144668
m_jit.graph().watchpoints().addLazily(globalObject->stringPrototype()->structure()->transitionWatchpointSet());
m_jit.graph().watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
m_jit.graph().registerAndWatchStructureTransition(globalObject->stringPrototype()->structure());
m_jit.graph().registerAndWatchStructureTransition(globalObject->objectPrototype()->structure());
prototypeChainIsSane = globalObject->stringPrototypeChainIsSane();
}
if (prototypeChainIsSane) {
m_jit.graph().watchpoints().addLazily(globalObject->stringPrototype()->structure()->transitionWatchpointSet());
m_jit.graph().watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());

#if USE(JSVALUE64)
addSlowPathGenerator(std::make_unique<SaneStringGetByValSlowPathGenerator>(
outOfBounds, this, JSValueRegs(scratchReg), baseReg, propertyReg));
@@ -112,10 +112,6 @@ class WatchpointCollectionPhase : public Phase {
{
m_graph.watchpoints().addLazily(set);
}
void addLazily(InlineWatchpointSet& set)
{
m_graph.watchpoints().addLazily(set);
}

JSGlobalObject* globalObject()
{
@@ -5759,9 +5759,9 @@ class LowerDFGToB3 {
// SaneChainOutOfBounds.
// https://bugs.webkit.org/show_bug.cgi?id=144668

m_graph.watchpoints().addLazily(globalObject->stringPrototype()->structure()->transitionWatchpointSet());
m_graph.watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
m_graph.registerAndWatchStructureTransition(globalObject->stringPrototype()->structure());
m_graph.registerAndWatchStructureTransition(globalObject->objectPrototype()->structure());

prototypeChainIsSane = globalObject->stringPrototypeChainIsSane();
}
if (prototypeChainIsSane) {

0 comments on commit fa393ff

Please sign in to comment.