Extend the SaneChain optimization to Contiguous arrays
https://bugs.webkit.org/show_bug.cgi?id=144664
Reviewed by Mark Lam.
Previously if you loaded from a hole, you'd either have to take slow path for the array
load (which means C++ calls and prototype chain walks) or you'd exit (if you hadn't
gathered the necessary profiling yet). But that's unnecessary if we know that the
prototype chain is sane - i.e. has no indexed properties. Then we can just return
Undefined for the hole.
Making this change requires setting more watchpoints on the array prototype chain. But
that hit a horrible bug: ArrayPrototype still uses the static lookup tables and builds
itself up lazily. This means that this increased the number of recompilations we'd get
due to the array prototype chain being built up.
So, this change also removes the laziness and static tables from ArrayPrototype.
But to make that change, I also had to add a helper for eagerly building up a prototype
that has builtin functions.
* CMakeLists.txt:
* DerivedSources.make:
* dfg/DFGArrayMode.h:
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileGetByVal):
* runtime/ArrayPrototype.cpp:
(JSC::ArrayPrototype::finishCreation):
(JSC::ArrayPrototype::getOwnPropertySlot): Deleted.
* runtime/ArrayPrototype.h:
* runtime/JSObject.h:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@184032 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index eead3fa..a3e0a02 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -538,17 +538,61 @@
ArrayMode arrayMode = node->arrayMode();
switch (arrayMode.type()) {
+ case Array::Contiguous:
case Array::Double:
if (arrayMode.arrayClass() == Array::OriginalArray
&& arrayMode.speculation() == Array::InBounds) {
JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
- if (globalObject->arrayPrototypeChainIsSane()
- && !(node->flags() & NodeBytecodeUsesAsOther)) {
- m_graph.watchpoints().addLazily(
- globalObject->arrayPrototype()->structure()->transitionWatchpointSet());
- m_graph.watchpoints().addLazily(
- globalObject->objectPrototype()->structure()->transitionWatchpointSet());
- node->setArrayMode(arrayMode.withSpeculation(Array::SaneChain));
+ if (globalObject->arrayPrototypeChainIsSane()) {
+ // Check if SaneChain will work on a per-type basis. Note that:
+ //
+ // 1) We don't want double arrays to sometimes return undefined, since
+ // that would require a change to the return type and it would pessimise
+ // things a lot. So, we'd only want to do that if we actually had
+ // evidence that we could read from a hole. That's pretty annoying.
+ // Likely the best way to handle that case is with an equivalent of
+ // SaneChain for OutOfBounds. For now we just detect when Undefined and
+ // NaN are indistinguishable according to backwards propagation, and just
+ // use SaneChain in that case. This happens to catch a lot of cases.
+ //
+ // 2) We don't want int32 array loads to have to do a hole check just to
+ // coerce to Undefined, since that would mean twice the checks.
+ //
+ // This has two implications. First, we have to do more checks than we'd
+ // like. It's unfortunate that we have to do the hole check. Second,
+ // some accesses that hit a hole will now need to take the full-blown
+ // out-of-bounds slow path. We can fix that with:
+ // https://bugs.webkit.org/show_bug.cgi?id=144668
+
+ bool canDoSaneChain = false;
+ switch (arrayMode.type()) {
+ case Array::Contiguous:
+ // This is happens to be entirely natural. We already would have
+ // returned any JSValue, and now we'll return Undefined. We still do
+ // the check but it doesn't require taking any kind of slow path.
+ canDoSaneChain = true;
+ break;
+
+ case Array::Double:
+ if (!(node->flags() & NodeBytecodeUsesAsOther)) {
+ // Holes look like NaN already, so if the user doesn't care
+ // about the difference between Undefined and NaN then we can
+ // do this.
+ canDoSaneChain = true;
+ }
+ break;
+
+ default:
+ break;
+ }
+
+ if (canDoSaneChain) {
+ m_graph.watchpoints().addLazily(
+ globalObject->arrayPrototype()->structure()->transitionWatchpointSet());
+ m_graph.watchpoints().addLazily(
+ globalObject->objectPrototype()->structure()->transitionWatchpointSet());
+ node->setArrayMode(arrayMode.withSpeculation(Array::SaneChain));
+ }
}
}
break;
