Bug 1473149. Add an external string variant that keeps a DynamicAtom alive. r=njn,rwood

The change to call AsStatic() in SetKnownLiveAtom is drive-by performance cleanup.

dom/bindings/DOMString.h

@@ -159,6 +159,26 @@ public:
     return mLength;
   }
 
+  bool HasAtom() const
+  {
+    MOZ_ASSERT(!mString || !mStringBuffer,
+               "Shouldn't have both present!");
+    MOZ_ASSERT(mState > State::Null,
+               "Caller should have checked IsNull() and IsEmpty() first");
+    return mState == State::UnownedAtom;
+  }
+
+  // Get the atom.  This can only be called if HasAtom() returned true.  If
+  // that's true, it will never return null.
+  nsDynamicAtom* Atom() const
+  {
+    MOZ_ASSERT(HasAtom(),
+               "Don't ask for the atom if we don't have it");
+    MOZ_ASSERT(mAtom,
+               "We better have an atom if we claim to");
+    return mAtom;
+  }
+
   // Initialize the DOMString to a (nsStringBuffer, length) pair.  The length
   // does NOT have to be the full length of the (null-terminated) string in the
   // nsStringBuffer.
@@ -214,17 +234,17 @@ public:
   {
     MOZ_ASSERT(mString.isNothing(), "We already have a string?");
     MOZ_ASSERT(mState == State::Empty, "We're already set to a value");
-    MOZ_ASSERT(!mStringBuffer, "Setting stringbuffer twice?");
+    MOZ_ASSERT(!mAtom, "Setting atom twice?");
     MOZ_ASSERT(aAtom || aNullHandling != eNullNotExpected);
     if (aNullHandling == eNullNotExpected || aAtom) {
       if (aAtom->IsStatic()) {
-        // Static atoms are backed by literals.
-        SetLiteralInternal(aAtom->GetUTF16String(), aAtom->GetLength());
+        // Static atoms are backed by literals.  Explicitly call AsStatic() here
+        // to avoid the extra IsStatic() checks in nsAtom::GetUTF16String().
+        SetLiteralInternal(aAtom->AsStatic()->GetUTF16String(),
+                           aAtom->GetLength());
       } else {
-        // Dynamic atoms own their own chars, and never have 0 length because
-        // nsGkAtoms::_empty is a static atom.
-        MOZ_ASSERT(aAtom->GetLength() > 0);
-        AsAString().Assign(aAtom->AsDynamic()->String(), aAtom->GetLength());
+        mAtom = aAtom->AsDynamic();
+        mState = State::UnownedAtom;
       }
     } else if (aNullHandling == eTreatNullAsNull) {
       SetNull();
@@ -277,6 +297,8 @@ public:
       }
     } else if (HasLiteral()) {
       aString.AssignLiteral(Literal(), LiteralLength());
+    } else if (HasAtom()) {
+      mAtom->ToString(aString);
     } else {
       aString = AsAString();
     }
@@ -312,6 +334,9 @@ private:
 
     String, // An XPCOM string stored in mString.
     Literal, // A string literal (static lifetime).
+    UnownedAtom, // mAtom is valid and we are not holding a ref.
+    // If we ever add an OwnedAtom state, XPCStringConvert::DynamicAtomToJSVal
+    // will need to grow an out param for whether the atom was shared.
     OwnedStringBuffer, // mStringBuffer is valid and we have a ref to it.
     UnownedStringBuffer, // mStringBuffer is valid; we are not holding a ref.
     // The two string buffer values must come last.  This lets us avoid doing
@@ -329,6 +354,10 @@ private:
                                  "assertions") mStringBuffer;
     // The literal in the Literal case.
     const char16_t* mLiteral;
+    // The atom in the UnownedAtom case.
+    nsDynamicAtom* MOZ_UNSAFE_REF("The ways in which this can be safe are "
+                                  "documented above and enforced through "
+                                  "assertions") mAtom;
   };
 
   // Length in the stringbuffer and literal cases.

js/xpconnect/src/XPCConvert.cpp

@@ -584,6 +584,8 @@ XPCConvert::JSData2Native(void* d, HandleValue s,
             const char16_t* chars = JS_GetTwoByteExternalStringChars(str);
             ws->AssignLiteral(chars, length);
         } else {
+            // We don't bother checking for a dynamic-atom external string,
+            // because we'd just need to copy out of it anyway.
             if (!AssignJSString(cx, *ws, str))
                 return false;
         }

js/xpconnect/src/XPCString.cpp

@@ -47,6 +47,21 @@ XPCStringConvert::FinalizeDOMString(const JSStringFinalizer* fin, char16_t* char
 const JSStringFinalizer XPCStringConvert::sDOMStringFinalizer =
     { XPCStringConvert::FinalizeDOMString };
 
+// static
+void
+XPCStringConvert::FinalizeDynamicAtom(const JSStringFinalizer* fin,
+                                      char16_t* chars)
+{
+    nsDynamicAtom* atom = nsDynamicAtom::FromChars(chars);
+    // nsDynamicAtom::Release is always-inline and defined in a translation unit
+    // we can't get to here.  So we need to go through nsAtom::Release to call
+    // it.
+    static_cast<nsAtom*>(atom)->Release();
+}
+
+const JSStringFinalizer XPCStringConvert::sDynamicAtomFinalizer =
+    { XPCStringConvert::FinalizeDynamicAtom };
+
 // convert a readable to a JSString, copying string data
 // static
 bool

js/xpconnect/src/xpcpublic.h

@@ -12,6 +12,7 @@
 #include "js/GCAPI.h"
 #include "js/Proxy.h"
 
+#include "nsAtom.h"
 #include "nsISupports.h"
 #include "nsIURI.h"
 #include "nsIPrincipal.h"
@@ -266,6 +267,28 @@ public:
         return true;
     }
 
+    static inline bool
+    DynamicAtomToJSVal(JSContext* cx, nsDynamicAtom* atom,
+                       JS::MutableHandleValue rval)
+    {
+        bool sharedAtom;
+        JSString* str = JS_NewMaybeExternalString(cx, atom->GetUTF16String(),
+                                                  atom->GetLength(),
+                                                  &sDynamicAtomFinalizer,
+                                                  &sharedAtom);
+        if (!str)
+            return false;
+        if (sharedAtom) {
+            // We only have non-owning atoms in DOMString for now.
+            // nsDynamicAtom::AddRef is always-inline and defined in a
+            // translation unit we can't get to here.  So we need to go through
+            // nsAtom::AddRef to call it.
+            static_cast<nsAtom*>(atom)->AddRef();
+        }
+        rval.setString(str);
+        return true;
+    }
+
     static MOZ_ALWAYS_INLINE bool IsLiteral(JSString* str)
     {
         return JS_IsExternalString(str) &&
@@ -279,12 +302,16 @@ public:
     }
 
 private:
-    static const JSStringFinalizer sLiteralFinalizer, sDOMStringFinalizer;
+    static const JSStringFinalizer
+      sLiteralFinalizer, sDOMStringFinalizer, sDynamicAtomFinalizer;
 
     static void FinalizeLiteral(const JSStringFinalizer* fin, char16_t* chars);
 
     static void FinalizeDOMString(const JSStringFinalizer* fin, char16_t* chars);
 
+    static void FinalizeDynamicAtom(const JSStringFinalizer* fin,
+                                    char16_t* chars);
+
     XPCStringConvert() = delete;
 };
 
@@ -366,6 +393,10 @@ bool NonVoidStringToJsval(JSContext* cx, mozilla::dom::DOMString& str,
                                                       str.LiteralLength(), rval);
     }
 
+    if (str.HasAtom()) {
+        return XPCStringConvert::DynamicAtomToJSVal(cx, str.Atom(), rval);
+    }
+
     // It's an actual XPCOM string
     return NonVoidStringToJsval(cx, str.AsAString(), rval);
 }

testing/talos/talos/tests/perf-reftest-singletons/README

@@ -0,0 +1,12 @@
+This directory is for adding short performance tests that will be tracked in
+Talos and receive regression alerts.
+
+To add a test:
+
+1)  Create a test HTML file which includes <script src="util.js"></script>
+2)  In that file, have an onload handler which does the following:
+   i) Any pre-test setup needed.
+  ii) A call to perf_start().
+ iii) The test steps.
+  iv) A call to perf_finish().
+3)  Add your test to the perf_reftest_singletons.manifest file.

testing/talos/talos/tests/perf-reftest-singletons/id-getter-1.html

@@ -0,0 +1,16 @@
+<!doctype html>
+<script src="util.js"></script>
+<script>
+onload = function() {
+  var count = 20000000;
+  var el = document.createElement("span");
+  // A very short string.
+  el.id = "a";
+  var getter = Object.getOwnPropertyDescriptor(Element.prototype, "id").get;
+  perf_start();
+  for (var i = 0; i < count; ++i) {
+    getter.call(el);
+  }
+  perf_finish();
+};
+</script>

testing/talos/talos/tests/perf-reftest-singletons/id-getter-2.html

@@ -0,0 +1,16 @@
+<!doctype html>
+<script src="util.js"></script>
+<script>
+onload = function() {
+  var count = 20000000;
+  var el = document.createElement("span");
+  // The longest string we can fit in a single-byte inline string (15 chars).
+  el.id = "aaaaaaaaaaaaaaa";
+  var getter = Object.getOwnPropertyDescriptor(Element.prototype, "id").get;
+  perf_start();
+  for (var i = 0; i < count; ++i) {
+    getter.call(el);
+  }
+  perf_finish();
+};
+</script>

testing/talos/talos/tests/perf-reftest-singletons/id-getter-3.html

@@ -0,0 +1,16 @@
+<!doctype html>
+<script src="util.js"></script>
+<script>
+onload = function() {
+  var count = 20000000;
+  var el = document.createElement("span");
+  // The shortest string we can't fit in a single-byte inline string (16 chars).
+  el.id = "aaaaaaaaaaaaaaaa";
+  var getter = Object.getOwnPropertyDescriptor(Element.prototype, "id").get;
+  perf_start();
+  for (var i = 0; i < count; ++i) {
+    getter.call(el);
+  }
+  perf_finish();
+};
+</script>

testing/talos/talos/tests/perf-reftest-singletons/id-getter-4.html

@@ -0,0 +1,16 @@
+<!doctype html>
+<script src="util.js"></script>
+<script>
+onload = function() {
+  var count = 20000000;
+  var el = document.createElement("span");
+  // The longest string we can fit in an autostring buffer (63 chars).
+  el.id = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+  var getter = Object.getOwnPropertyDescriptor(Element.prototype, "id").get;
+  perf_start();
+  for (var i = 0; i < count; ++i) {
+    getter.call(el);
+  }
+  perf_finish();
+};
+</script>

testing/talos/talos/tests/perf-reftest-singletons/id-getter-5.html

@@ -0,0 +1,16 @@
+<!doctype html>
+<script src="util.js"></script>
+<script>
+onload = function() {
+  var count = 20000000;
+  var el = document.createElement("span");
+  // The shortest string we can't fit in an autostring buffer (64 chars).
+  el.id = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+  var getter = Object.getOwnPropertyDescriptor(Element.prototype, "id").get;
+  perf_start();
+  for (var i = 0; i < count; ++i) {
+    getter.call(el);
+  }
+  perf_finish();
+};
+</script>

testing/talos/talos/tests/perf-reftest-singletons/id-getter-6.html

@@ -0,0 +1,17 @@
+<!doctype html>
+<script src="util.js"></script>
+<script>
+onload = function() {
+  var count = 20000000;
+  var el = document.createElement("span");
+  // The longest string we can share via the external string cache after
+  // checking the chars (100 chars).
+  el.id = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+  var getter = Object.getOwnPropertyDescriptor(Element.prototype, "id").get;
+  perf_start();
+  for (var i = 0; i < count; ++i) {
+    getter.call(el);
+  }
+  perf_finish();
+};
+</script>

testing/talos/talos/tests/perf-reftest-singletons/id-getter-7.html

@@ -0,0 +1,17 @@
+<!doctype html>
+<script src="util.js"></script>
+<script>
+onload = function() {
+  var count = 20000000;
+  var el = document.createElement("span");
+  // The shortest string we can't share via the external string cache after
+  // checking the chars (101 chars).
+  el.id = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+  var getter = Object.getOwnPropertyDescriptor(Element.prototype, "id").get;
+  perf_start();
+  for (var i = 0; i < count; ++i) {
+    getter.call(el);
+  }
+  perf_finish();
+};
+</script>

testing/talos/talos/tests/perf-reftest-singletons/perf_reftest_singletons.manifest

@@ -15,3 +15,11 @@
 % http://localhost/tests/perf-reftest-singletons/nth-index-2.html
 
 % http://localhost/tests/perf-reftest-singletons/bidi-resolution-1.html
+
+% http://localhost/tests/perf-reftest-singletons/id-getter-1.html
+% http://localhost/tests/perf-reftest-singletons/id-getter-2.html
+% http://localhost/tests/perf-reftest-singletons/id-getter-3.html
+% http://localhost/tests/perf-reftest-singletons/id-getter-4.html
+% http://localhost/tests/perf-reftest-singletons/id-getter-5.html
+% http://localhost/tests/perf-reftest-singletons/id-getter-6.html
+% http://localhost/tests/perf-reftest-singletons/id-getter-7.html

xpcom/ds/nsAtom.h

@@ -172,6 +172,11 @@ public:
     return reinterpret_cast<const char16_t*>(this + 1);
   }
 
+  static nsDynamicAtom* FromChars(char16_t* chars)
+  {
+    return reinterpret_cast<nsDynamicAtom*>(chars) - 1;
+  }
+
 private:
   friend class nsAtomTable;
   friend class nsAtomSubTable;