Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (199296 => 199297)
--- trunk/Source/_javascript_Core/ChangeLog 2016-04-11 16:50:44 UTC (rev 199296)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-04-11 18:20:59 UTC (rev 199297)
@@ -1,3 +1,59 @@
+2016-04-10 Filip Pizlo <fpi...@apple.com>
+
+ Clean up how we reason about the states of AccessCases
+ https://bugs.webkit.org/show_bug.cgi?id=156454
+
+ Reviewed by Mark Lam.
+
+ Currently when we add an AccessCase to a PolymorphicAccess stub, we regenerate the stub.
+ That means that as we grow a stub to have N cases, we will do O(N^2) generation work. I want
+ to explore buffering AccessCases so that we can do O(N) generation work instead. But to
+ before I go there, I want to make sure that the statefulness of AccessCase makes sense. So,
+ I broke it down into three different states and added assertions about the transitions. I
+ also broke out a separate operation called AccessCase::commit(), which is the work that
+ cannot be buffered since there cannot be any JS effects between when the AccessCase was
+ created and when we do the work in commit().
+
+ This opens up a fairly obvious path to buffering AccessCases: add them to the list without
+ regenerating. Then when we do eventually trigger regeneration, those cases will get cloned
+ and generated automagically. This patch doesn't implement this technique yet, but gives us
+ an opportunity to independently test the scaffolding necessary to do it.
+
+ This is perf-neutral on lots of tests.
+
+ * bytecode/PolymorphicAccess.cpp:
+ (JSC::AccessGenerationResult::dump):
+ (JSC::AccessCase::clone):
+ (JSC::AccessCase::commit):
+ (JSC::AccessCase::guardedByStructureCheck):
+ (JSC::AccessCase::dump):
+ (JSC::AccessCase::generateWithGuard):
+ (JSC::AccessCase::generate):
+ (JSC::AccessCase::generateImpl):
+ (JSC::PolymorphicAccess::regenerateWithCases):
+ (JSC::PolymorphicAccess::regenerate):
+ (WTF::printInternal):
+ * bytecode/PolymorphicAccess.h:
+ (JSC::AccessCase::type):
+ (JSC::AccessCase::state):
+ (JSC::AccessCase::offset):
+ (JSC::AccessCase::viaProxy):
+ (JSC::AccessCase::callLinkInfo):
+ * bytecode/StructureStubInfo.cpp:
+ (JSC::StructureStubInfo::addAccessCase):
+ * bytecode/Watchpoint.h:
+ * dfg/DFGOperations.cpp:
+ * jit/Repatch.cpp:
+ (JSC::repatchGetByID):
+ (JSC::repatchPutByID):
+ (JSC::repatchIn):
+ * runtime/VM.cpp:
+ (JSC::VM::dumpRegExpTrace):
+ (JSC::VM::ensureWatchpointSetForImpureProperty):
+ (JSC::VM::registerWatchpointForImpureProperty):
+ (JSC::VM::addImpureProperty):
+ * runtime/VM.h:
+
2016-04-11 Fujii Hironori <hironori.fu...@jp.sony.com>
[CMake] Make FOLDER property INHERITED
Modified: trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp (199296 => 199297)
--- trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp 2016-04-11 16:50:44 UTC (rev 199296)
+++ trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp 2016-04-11 18:20:59 UTC (rev 199297)
@@ -48,6 +48,14 @@
static const bool verbose = false;
+// EncodedJSValue in JSVALUE32_64 is a 64-bit integer. When being compiled in ARM EABI, it must be aligned on an even-numbered register (r0, r2 or [sp]).
+// To prevent the assembler from using wrong registers, let's occupy r1 or r3 with a dummy argument when necessary.
+#if (COMPILER_SUPPORTS(EABI) && CPU(ARM)) || CPU(MIPS)
+#define EABI_32BIT_DUMMY_ARG CCallHelpers::TrustedImm32(0),
+#else
+#define EABI_32BIT_DUMMY_ARG
+#endif
+
void AccessGenerationResult::dump(PrintStream& out) const
{
out.print(m_kind);
@@ -389,6 +397,24 @@
return result;
}
+Vector<WatchpointSet*, 2> AccessCase::commit(VM& vm, const Identifier& ident)
+{
+ RELEASE_ASSERT(m_state == Primordial);
+
+ Vector<WatchpointSet*, 2> result;
+
+ if ((structure() && structure()->needImpurePropertyWatchpoint())
+ || m_conditionSet.needImpurePropertyWatchpoint())
+ result.append(vm.ensureWatchpointSetForImpureProperty(ident));
+
+ if (additionalSet())
+ result.append(additionalSet());
+
+ m_state = Committed;
+
+ return result;
+}
+
bool AccessCase::guardedByStructureCheck() const
{
if (viaProxy())
@@ -470,6 +496,8 @@
out.print(m_type, ":(");
CommaPrinter comma;
+
+ out.print(comma, m_state);
if (m_type == Transition)
out.print(comma, "structure = ", pointerDump(structure()), " -> ", pointerDump(newStructure()));
@@ -517,6 +545,11 @@
void AccessCase::generateWithGuard(
AccessGenerationState& state, CCallHelpers::JumpList& fallThrough)
{
+ SuperSamplerScope superSamplerScope(false);
+
+ RELEASE_ASSERT(m_state == Committed);
+ m_state = Generated;
+
CCallHelpers& jit = *state.jit;
VM& vm = *jit.vm();
const Identifier& ident = *state.ident;
@@ -730,22 +763,25 @@
break;
} };
- generate(state);
+ generateImpl(state);
}
-// EncodedJSValue in JSVALUE32_64 is a 64-bit integer. When being compiled in ARM EABI, it must be aligned on an even-numbered register (r0, r2 or [sp]).
-// To prevent the assembler from using wrong registers, let's occupy r1 or r3 with a dummy argument when necessary.
-#if (COMPILER_SUPPORTS(EABI) && CPU(ARM)) || CPU(MIPS)
-#define EABI_32BIT_DUMMY_ARG CCallHelpers::TrustedImm32(0),
-#else
-#define EABI_32BIT_DUMMY_ARG
-#endif
-
void AccessCase::generate(AccessGenerationState& state)
{
+ RELEASE_ASSERT(m_state == Committed);
+ m_state = Generated;
+
+ generateImpl(state);
+}
+
+void AccessCase::generateImpl(AccessGenerationState& state)
+{
+ SuperSamplerScope superSamplerScope(false);
if (verbose)
dataLog("Generating code for: ", *this, "\n");
+ ASSERT(m_state == Generated); // We rely on the callers setting this for us.
+
CCallHelpers& jit = *state.jit;
VM& vm = *jit.vm();
CodeBlock* codeBlock = jit.codeBlock();
@@ -757,13 +793,6 @@
ASSERT(m_conditionSet.structuresEnsureValidityAssumingImpurePropertyWatchpoint());
- if ((structure() && structure()->needImpurePropertyWatchpoint())
- || m_conditionSet.needImpurePropertyWatchpoint())
- vm.registerWatchpointForImpureProperty(ident, state.addWatchpoint());
-
- if (additionalSet())
- additionalSet()->add(state.addWatchpoint());
-
for (const ObjectPropertyCondition& condition : m_conditionSet) {
Structure* structure = condition.object()->structure();
@@ -773,6 +802,10 @@
}
if (!condition.structureEnsuresValidityAssumingImpurePropertyWatchpoint(structure)) {
+ // The reason why this cannot happen is that we require that PolymorphicAccess calls
+ // AccessCase::generate() only after it has verified that
+ // AccessCase::couldStillSucceed() returned true.
+
dataLog("This condition is no longer met: ", condition, "\n");
RELEASE_ASSERT_NOT_REACHED();
}
@@ -1363,6 +1396,8 @@
VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo, const Identifier& ident,
Vector<std::unique_ptr<AccessCase>> originalCasesToAdd)
{
+ SuperSamplerScope superSamplerScope(false);
+
// This method will add the originalCasesToAdd to the list one at a time while preserving the
// invariants:
// - If a newly added case canReplace() any existing case, then the existing case is removed before
@@ -1503,6 +1538,8 @@
VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo, const Identifier& ident,
PolymorphicAccess::ListType& cases)
{
+ SuperSamplerScope superSamplerScope(false);
+
if (verbose)
dataLog("Generating code for cases: ", listDump(cases), "\n");
@@ -1538,6 +1575,9 @@
bool allGuardedByStructureCheck = true;
bool hasJSGetterSetterCall = false;
for (auto& entry : cases) {
+ for (WatchpointSet* set : entry->commit(vm, ident))
+ set->add(state.addWatchpoint());
+
allGuardedByStructureCheck &= entry->guardedByStructureCheck();
if (entry->type() == AccessCase::Getter || entry->type() == AccessCase::Setter)
hasJSGetterSetterCall = true;
@@ -1763,6 +1803,23 @@
RELEASE_ASSERT_NOT_REACHED();
}
+void printInternal(PrintStream& out, AccessCase::State state)
+{
+ switch (state) {
+ case AccessCase::Primordial:
+ out.print("Primordial");
+ return;
+ case AccessCase::Committed:
+ out.print("Committed");
+ return;
+ case AccessCase::Generated:
+ out.print("Generated");
+ return;
+ }
+
+ RELEASE_ASSERT_NOT_REACHED();
+}
+
} // namespace WTF
#endif // ENABLE(JIT)
Modified: trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.h (199296 => 199297)
--- trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.h 2016-04-11 16:50:44 UTC (rev 199296)
+++ trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.h 2016-04-11 18:20:59 UTC (rev 199297)
@@ -47,11 +47,52 @@
struct AccessGenerationState;
+// An AccessCase describes one of the cases of a PolymorphicAccess. A PolymorphicAccess represents a
+// planned (to generate in future) or generated stub for some inline cache. That stub contains fast
+// path code for some finite number of fast cases, each described by an AccessCase object.
+//
+// An AccessCase object has a lifecycle that proceeds through several states. Note that the states
+// of AccessCase have a lot to do with the global effect epoch (we'll say epoch for short). This is
+// a simple way of reasoning about the state of the system outside this AccessCase. Any observable
+// effect - like storing to a property, changing an object's structure, etc. - increments the epoch.
+// The states are:
+//
+// Primordial: This is an AccessCase that was just allocated. It does not correspond to any actual
+// code and it is not owned by any PolymorphicAccess. In this state, the AccessCase
+// assumes that it is in the same epoch as when it was created. This is important
+// because it may make claims about itself ("I represent a valid case so long as you
+// register a watchpoint on this set") that could be contradicted by some outside
+// effects (like firing and deleting the watchpoint set in question). This is also the
+// state that an AccessCase is in when it is cloned (AccessCase::clone()).
+//
+// Committed: This happens as soon as some PolymorphicAccess takes ownership of this AccessCase.
+// In this state, the AccessCase no longer assumes anything about the epoch. To
+// accomplish this, PolymorphicAccess calls AccessCase::commit(). This must be done
+// during the same epoch when the AccessCase was created, either by the client or by
+// clone(). When created by the client, committing during the same epoch works because
+// we can be sure that whatever watchpoint sets they spoke of are still valid. When
+// created by clone(), we can be sure that the set is still valid because the original
+// of the clone still has watchpoints on it.
+//
+// Generated: This is the state when the PolymorphicAccess generates code for this case by
+// calling AccessCase::generate() or AccessCase::generateWithGuard(). At this point
+// the case object will have some extra stuff in it, like possibly the CallLinkInfo
+// object associated with the inline cache.
+// FIXME: Moving into the Generated state should not mutate the AccessCase object or
+// put more stuff into it. If we fix this, then we can get rid of AccessCase::clone().
+// https://bugs.webkit.org/show_bug.cgi?id=156456
+//
+// An AccessCase may be destroyed while in any of these states.
+//
+// Note that right now, an AccessCase goes from Primordial to Generated quite quickly.
+// FIXME: Make it possible for PolymorphicAccess to hold onto AccessCases that haven't been
+// generated. That would allow us to significantly reduce the number of regeneration events.
+// https://bugs.webkit.org/show_bug.cgi?id=156457
class AccessCase {
WTF_MAKE_NONCOPYABLE(AccessCase);
WTF_MAKE_FAST_ALLOCATED;
public:
- enum AccessType {
+ enum AccessType : uint8_t {
Load,
MegamorphicLoad,
Transition,
@@ -72,6 +113,12 @@
DirectArgumentsLength,
ScopedArgumentsLength
};
+
+ enum State : uint8_t {
+ Primordial,
+ Committed,
+ Generated
+ };
static std::unique_ptr<AccessCase> tryGet(
VM&, JSCell* owner, AccessType, PropertyOffset, Structure*,
@@ -111,9 +158,8 @@
~AccessCase();
- std::unique_ptr<AccessCase> clone() const;
-
AccessType type() const { return m_type; }
+ State state() const { return m_state; }
PropertyOffset offset() const { return m_offset; }
bool viaProxy() const { return m_rareData ? m_rareData->viaProxy : false; }
@@ -176,8 +222,11 @@
return nullptr;
return m_rareData->callLinkInfo.get();
}
-
- // Is it still possible for this case to ever be taken?
+
+ // Is it still possible for this case to ever be taken? Must call this as a prerequisite for
+ // calling generate() and friends. If this returns true, then you can call generate(). If
+ // this returns false, then generate() will crash. You must call generate() in the same epoch
+ // as when you called couldStillSucceed().
bool couldStillSucceed() const;
static bool canEmitIntrinsicGetter(JSFunction*, Structure*);
@@ -198,6 +247,14 @@
AccessCase();
bool visitWeak(VM&) const;
+
+ // FIXME: This only exists because of how AccessCase puts post-generation things into itself.
+ // https://bugs.webkit.org/show_bug.cgi?id=156456
+ std::unique_ptr<AccessCase> clone() const;
+
+ // Perform any action that must be performed before the end of the epoch in which the case
+ // was created. Returns a set of watchpoint sets that will need to be watched.
+ Vector<WatchpointSet*, 2> commit(VM&, const Identifier&);
// Fall through on success. Two kinds of failures are supported: fall-through, which means that we
// should try a different case; and failure, which means that this was the right case but it needs
@@ -206,9 +263,12 @@
// Fall through on success, add a jump to the failure list on failure.
void generate(AccessGenerationState&);
+
+ void generateImpl(AccessGenerationState&);
void emitIntrinsicGetter(AccessGenerationState&);
AccessType m_type { Load };
+ State m_state { Primordial };
PropertyOffset m_offset { invalidOffset };
// Usually this is the structure that we expect the base object to have. But, this is the *new*
@@ -229,6 +289,8 @@
bool viaProxy;
RefPtr<WatchpointSet> additionalSet;
+ // FIXME: This should probably live in the stub routine object.
+ // https://bugs.webkit.org/show_bug.cgi?id=156456
std::unique_ptr<CallLinkInfo> callLinkInfo;
union {
PropertySlot::GetValueFunc getter;
@@ -433,6 +495,7 @@
void printInternal(PrintStream&, JSC::AccessGenerationResult::Kind);
void printInternal(PrintStream&, JSC::AccessCase::AccessType);
+void printInternal(PrintStream&, JSC::AccessCase::State);
} // namespace WTF
Modified: trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp (199296 => 199297)
--- trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp 2016-04-11 16:50:44 UTC (rev 199296)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp 2016-04-11 18:20:59 UTC (rev 199297)
@@ -112,9 +112,12 @@
if (!accessCase)
return AccessGenerationResult::GaveUp;
- if (cacheType == CacheType::Stub)
+ if (cacheType == CacheType::Stub) {
+ SuperSamplerScope superSamplerScope(true);
+
return u.stub->regenerateWithCase(vm, codeBlock, *this, ident, WTFMove(accessCase));
-
+ }
+
std::unique_ptr<PolymorphicAccess> access = std::make_unique<PolymorphicAccess>();
Vector<std::unique_ptr<AccessCase>> accessCases;
Modified: trunk/Source/_javascript_Core/bytecode/Watchpoint.h (199296 => 199297)
--- trunk/Source/_javascript_Core/bytecode/Watchpoint.h 2016-04-11 16:50:44 UTC (rev 199296)
+++ trunk/Source/_javascript_Core/bytecode/Watchpoint.h 2016-04-11 18:20:59 UTC (rev 199297)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012-2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -95,6 +95,9 @@
friend class LLIntOffsetsExtractor;
public:
JS_EXPORT_PRIVATE WatchpointSet(WatchpointState);
+
+ // FIXME: In many cases, it would be amazing if this *did* fire the watchpoints. I suspect that
+ // this might be hard to get right, but still, it might be awesome.
JS_EXPORT_PRIVATE ~WatchpointSet(); // Note that this will not fire any of the watchpoints; if you need to know when a WatchpointSet dies then you need a separate mechanism for this.
// Fast way of getting the state, which only works from the main thread.
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (199296 => 199297)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2016-04-11 16:50:44 UTC (rev 199296)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2016-04-11 18:20:59 UTC (rev 199297)
@@ -691,7 +691,7 @@
size_t JIT_OPERATION operationRegExpTestGeneric(ExecState* exec, JSGlobalObject* globalObject, EncodedJSValue encodedBase, EncodedJSValue encodedArgument)
{
- SuperSamplerScope superSamplerScope;
+ SuperSamplerScope superSamplerScope(false);
VM& vm = globalObject->vm();
NativeCallFrameTracer tracer(&vm, exec);
Modified: trunk/Source/_javascript_Core/jit/Repatch.cpp (199296 => 199297)
--- trunk/Source/_javascript_Core/jit/Repatch.cpp 2016-04-11 16:50:44 UTC (rev 199296)
+++ trunk/Source/_javascript_Core/jit/Repatch.cpp 2016-04-11 18:20:59 UTC (rev 199297)
@@ -379,6 +379,7 @@
void repatchGetByID(ExecState* exec, JSValue baseValue, const Identifier& propertyName, const PropertySlot& slot, StructureStubInfo& stubInfo, GetByIDKind kind)
{
+ SuperSamplerScope superSamplerScope(false);
GCSafeConcurrentJITLocker locker(exec->codeBlock()->m_lock, exec->vm().heap);
if (tryCacheGetByID(exec, baseValue, propertyName, slot, stubInfo, kind) == GiveUpOnCache)
@@ -533,6 +534,7 @@
void repatchPutByID(ExecState* exec, JSValue baseValue, Structure* structure, const Identifier& propertyName, const PutPropertySlot& slot, StructureStubInfo& stubInfo, PutKind putKind)
{
+ SuperSamplerScope superSamplerScope(false);
GCSafeConcurrentJITLocker locker(exec->codeBlock()->m_lock, exec->vm().heap);
if (tryCachePutByID(exec, baseValue, structure, propertyName, slot, stubInfo, putKind) == GiveUpOnCache)
@@ -596,6 +598,7 @@
ExecState* exec, JSCell* base, const Identifier& ident, bool wasFound,
const PropertySlot& slot, StructureStubInfo& stubInfo)
{
+ SuperSamplerScope superSamplerScope(false);
if (tryRepatchIn(exec, base, ident, wasFound, slot, stubInfo) == GiveUpOnCache)
repatchCall(exec->codeBlock(), stubInfo.callReturnLocation, operationIn);
}
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (199296 => 199297)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2016-04-11 16:50:44 UTC (rev 199296)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2016-04-11 18:20:59 UTC (rev 199297)
@@ -755,14 +755,19 @@
}
#endif
-void VM::registerWatchpointForImpureProperty(const Identifier& propertyName, Watchpoint* watchpoint)
+WatchpointSet* VM::ensureWatchpointSetForImpureProperty(const Identifier& propertyName)
{
auto result = m_impurePropertyWatchpointSets.add(propertyName.string(), nullptr);
if (result.isNewEntry)
result.iterator->value = adoptRef(new WatchpointSet(IsWatched));
- result.iterator->value->add(watchpoint);
+ return result.iterator->value.get();
}
+void VM::registerWatchpointForImpureProperty(const Identifier& propertyName, Watchpoint* watchpoint)
+{
+ ensureWatchpointSetForImpureProperty(propertyName)->add(watchpoint);
+}
+
void VM::addImpureProperty(const String& propertyName)
{
if (RefPtr<WatchpointSet> watchpointSet = m_impurePropertyWatchpointSets.take(propertyName))
Modified: trunk/Source/_javascript_Core/runtime/VM.h (199296 => 199297)
--- trunk/Source/_javascript_Core/runtime/VM.h 2016-04-11 16:50:44 UTC (rev 199296)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2016-04-11 18:20:59 UTC (rev 199297)
@@ -580,6 +580,7 @@
JS_EXPORT_PRIVATE void deleteAllLinkedCode();
JS_EXPORT_PRIVATE void deleteAllRegExpCode();
+ WatchpointSet* ensureWatchpointSetForImpureProperty(const Identifier&);
void registerWatchpointForImpureProperty(const Identifier&, Watchpoint*);
// FIXME: Use AtomicString once it got merged with Identifier.
