Clean up how we reason about the states of AccessCases
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Apr 2016 18:20:59 +0000 (18:20 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Apr 2016 18:20:59 +0000 (18:20 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@199297 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp
Source/JavaScriptCore/bytecode/PolymorphicAccess.h
Source/JavaScriptCore/bytecode/StructureStubInfo.cpp
Source/JavaScriptCore/bytecode/Watchpoint.h
Source/JavaScriptCore/dfg/DFGOperations.cpp
Source/JavaScriptCore/jit/Repatch.cpp
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h

index 3368889..06e12a2 100644 (file)
@@ -1,3 +1,59 @@
+2016-04-10  Filip Pizlo  <fpizlo@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.Fujii@jp.sony.com>
 
         [CMake] Make FOLDER property INHERITED
index ae9885c..a03bf88 100644 (file)
@@ -48,6 +48,14 @@ namespace JSC {
 
 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 @@ std::unique_ptr<AccessCase> AccessCase::clone() const
     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 @@ void AccessCase::dump(PrintStream& out) const
     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 @@ bool AccessCase::visitWeak(VM& vm) const
 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 @@ void AccessCase::generateWithGuard(
         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 @@ void AccessCase::generate(AccessGenerationState& state)
 
     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 @@ void AccessCase::generate(AccessGenerationState& state)
         }
 
         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 @@ AccessGenerationResult PolymorphicAccess::regenerateWithCases(
     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 @@ MacroAssemblerCodePtr PolymorphicAccess::regenerate(
     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 @@ MacroAssemblerCodePtr PolymorphicAccess::regenerate(
     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 @@ void printInternal(PrintStream& out, AccessCase::AccessType type)
     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)
index 0eaa75c..9e96da9 100644 (file)
@@ -47,11 +47,52 @@ class ScratchRegisterAllocator;
 
 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 @@ public:
         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 @@ public:
 
     ~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 @@ public:
             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 @@ private:
     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 @@ private:
 
     // 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 @@ private:
         
         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 @@ namespace WTF {
 
 void printInternal(PrintStream&, JSC::AccessGenerationResult::Kind);
 void printInternal(PrintStream&, JSC::AccessCase::AccessType);
+void printInternal(PrintStream&, JSC::AccessCase::State);
 
 } // namespace WTF
 
index ff9fed2..a3b2c1a 100644 (file)
@@ -112,9 +112,12 @@ AccessGenerationResult StructureStubInfo::addAccessCase(
     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;
index 56fe1f3..d37c0f3 100644 (file)
@@ -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 @@ class WatchpointSet : public ThreadSafeRefCounted<WatchpointSet> {
     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.
index e8c7ae0..1f52419 100644 (file)
@@ -691,7 +691,7 @@ size_t JIT_OPERATION operationRegExpTest(ExecState* exec, JSGlobalObject* global
 
 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);
index 71ceb89..fdc34c2 100644 (file)
@@ -379,6 +379,7 @@ static InlineCacheAction tryCacheGetByID(ExecState* exec, JSValue baseValue, con
 
 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 @@ static InlineCacheAction tryCachePutByID(ExecState* exec, JSValue baseValue, Str
 
 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 @@ void repatchIn(
     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);
 }
index 0a103fa..0a2279c 100644 (file)
@@ -755,12 +755,17 @@ void VM::dumpRegExpTrace()
 }
 #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)
index 0872a88..64f8bb2 100644 (file)
@@ -580,6 +580,7 @@ public:
     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.