Bug 1364816 part 5. Make getting window names a bit faster by avoiding various intermediate strings. r=qdot,jandem
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 06 Jun 2017 21:21:45 -0400
changeset 413102 2d37f2bce087264116abf04a5133fbd3b031507c
parent 413101 79157ef8e455f91349e4bf5c73c13f5ecb9b63cf
child 413103 e190f8af99aa26f83407a29de8f4512c888ec0ad
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqdot, jandem
bugs1364816
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1364816 part 5. Make getting window names a bit faster by avoiding various intermediate strings. r=qdot,jandem
dom/base/nsGlobalWindow.cpp
dom/base/nsGlobalWindow.h
dom/base/nsObjectLoadingContent.cpp
dom/base/nsObjectLoadingContent.h
dom/bindings/Codegen.py
dom/bindings/WebIDLGlobalNameHash.cpp
dom/bindings/WebIDLGlobalNameHash.h
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -5133,17 +5133,17 @@ nsGlobalWindow::MayResolve(jsid aId)
 
   nsAutoString name;
   AssignJSFlatString(name, JSID_TO_FLAT_STRING(aId));
 
   return nameSpaceManager->LookupName(name);
 }
 
 void
-nsGlobalWindow::GetOwnPropertyNames(JSContext* aCx, nsTArray<nsString>& aNames,
+nsGlobalWindow::GetOwnPropertyNames(JSContext* aCx, JS::AutoIdVector& aNames,
                                     ErrorResult& aRv)
 {
   MOZ_ASSERT(IsInnerWindow());
   // "Components" is marked as enumerable but only resolved on demand :-/.
   //aNames.AppendElement(NS_LITERAL_STRING("Components"));
 
   nsScriptNameSpaceManager* nameSpaceManager = GetNameSpaceManager();
   if (nameSpaceManager) {
@@ -5158,25 +5158,33 @@ nsGlobalWindow::GetOwnPropertyNames(JSCo
     // already-resolved names, so we can just return the so-far-unresolved ones.
     //
     // We can tell which case we're in by whether aCx is in our wrapper's
     // compartment.  If not, we're in the Xray case.
     WebIDLGlobalNameHash::NameType nameType =
       js::IsObjectInContextCompartment(wrapper, aCx) ?
         WebIDLGlobalNameHash::UnresolvedNamesOnly :
         WebIDLGlobalNameHash::AllNames;
-    WebIDLGlobalNameHash::GetNames(aCx, wrapper, nameType, aNames);
+    if (!WebIDLGlobalNameHash::GetNames(aCx, wrapper, nameType, aNames)) {
+      aRv.NoteJSContextException(aCx);
+    }
 
     for (auto i = nameSpaceManager->GlobalNameIter(); !i.Done(); i.Next()) {
       const GlobalNameMapEntry* entry = i.Get();
       if (nsWindowSH::NameStructEnabled(aCx, this, entry->mKey,
                                         entry->mGlobalName)) {
         // Just append all of these; even if they get deleted our resolve hook
         // just goes ahead and recreates them.
-        aNames.AppendElement(entry->mKey);
+        JSString* str = JS_AtomizeUCStringN(aCx,
+                                            entry->mKey.BeginReading(),
+                                            entry->mKey.Length());
+        if (!str || !aNames.append(NON_INTEGER_ATOM_TO_JSID(str))) {
+          aRv.NoteJSContextException(aCx);
+          return;
+        }
       }
     }
   }
 }
 
 /* static */ bool
 nsGlobalWindow::IsPrivilegedChromeWindow(JSContext* aCx, JSObject* aObj)
 {
--- a/dom/base/nsGlobalWindow.h
+++ b/dom/base/nsGlobalWindow.h
@@ -481,17 +481,17 @@ public:
 
   bool DoResolve(JSContext* aCx, JS::Handle<JSObject*> aObj,
                  JS::Handle<jsid> aId,
                  JS::MutableHandle<JS::PropertyDescriptor> aDesc);
   // The return value is whether DoResolve might end up resolving the given id.
   // If in doubt, return true.
   static bool MayResolve(jsid aId);
 
-  void GetOwnPropertyNames(JSContext* aCx, nsTArray<nsString>& aNames,
+  void GetOwnPropertyNames(JSContext* aCx, JS::AutoIdVector& aNames,
                            mozilla::ErrorResult& aRv);
 
   // Object Management
   static already_AddRefed<nsGlobalWindow> Create(nsGlobalWindow *aOuterWindow);
 
   static nsGlobalWindow *FromSupports(nsISupports *supports)
   {
     // Make sure this matches the casts we do in QueryInterface().
--- a/dom/base/nsObjectLoadingContent.cpp
+++ b/dom/base/nsObjectLoadingContent.cpp
@@ -3896,17 +3896,17 @@ bool
 nsObjectLoadingContent::MayResolve(jsid aId)
 {
   // We can resolve anything, really.
   return true;
 }
 
 void
 nsObjectLoadingContent::GetOwnPropertyNames(JSContext* aCx,
-                                            nsTArray<nsString>& /* unused */,
+                                            JS::AutoIdVector& /* unused */,
                                             ErrorResult& aRv)
 {
   // Just like DoResolve, just make sure we're instantiated.  That will do
   // the work our Enumerate hook needs to do.  This purposefully does not fire
   // for xray resolves, see bug 967694
   RefPtr<nsNPAPIPluginInstance> pi;
   aRv = ScriptRequestPluginInstance(aCx, getter_AddRefs(pi));
 }
--- a/dom/base/nsObjectLoadingContent.h
+++ b/dom/base/nsObjectLoadingContent.h
@@ -176,17 +176,17 @@ class nsObjectLoadingContent : public ns
     bool DoResolve(JSContext* aCx, JS::Handle<JSObject*> aObject,
                    JS::Handle<jsid> aId,
                    JS::MutableHandle<JS::PropertyDescriptor> aDesc);
     // The return value is whether DoResolve might end up resolving the given
     // id.  If in doubt, return true.
     static bool MayResolve(jsid aId);
 
     // Helper for WebIDL enumeration
-    void GetOwnPropertyNames(JSContext* aCx, nsTArray<nsString>& /* unused */,
+    void GetOwnPropertyNames(JSContext* aCx, JS::AutoIdVector& /* unused */,
                              mozilla::ErrorResult& aRv);
 
     // WebIDL API
     nsIDocument* GetContentDocument(nsIPrincipal& aSubjectPrincipal);
     void GetActualType(nsAString& aType) const
     {
       CopyUTF8toUTF16(mContentType, aType);
     }
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -8924,31 +8924,21 @@ class CGEnumerateHook(CGAbstractBindingM
                 Argument('bool', 'enumerableOnly')]
         # Our "self" is actually the "obj" argument in this case, not the thisval.
         CGAbstractBindingMethod.__init__(
             self, descriptor, ENUMERATE_HOOK_NAME,
             args, getThisObj="", callArgs="")
 
     def generate_code(self):
         return CGGeneric(dedent("""
-            AutoTArray<nsString, 8> names;
             binding_detail::FastErrorResult rv;
-            self->GetOwnPropertyNames(cx, names, rv);
+            self->GetOwnPropertyNames(cx, properties, rv);
             if (rv.MaybeSetPendingException(cx)) {
               return false;
             }
-            JS::Rooted<JS::Value> v(cx);
-            JS::Rooted<jsid> id(cx);
-            for (auto& name : names) {
-              if (!xpc::NonVoidStringToJsval(cx, name, &v) ||
-                  !JS_ValueToId(cx, v, &id) ||
-                  !properties.append(id)) {
-                return false;
-              }
-            }
             return true;
             """))
 
     def definition_body(self):
         if self.descriptor.isGlobal():
             # Enumerate standard classes
             prefix = dedent("""
                 // This is OK even though we're a newEnumerate hook: this will
@@ -11200,25 +11190,22 @@ class CGEnumerateOwnPropertiesViaGetOwnP
                 Argument('JS::AutoIdVector&', 'props')]
         CGAbstractBindingMethod.__init__(self, descriptor,
                                          "EnumerateOwnPropertiesViaGetOwnPropertyNames",
                                          args, getThisObj="",
                                          callArgs="")
 
     def generate_code(self):
         return CGGeneric(dedent("""
-            AutoTArray<nsString, 8> names;
             binding_detail::FastErrorResult rv;
-            self->GetOwnPropertyNames(cx, names, rv);
+            self->GetOwnPropertyNames(cx, props, rv);
             if (rv.MaybeSetPendingException(cx)) {
               return false;
             }
-            // OK to pass null as "proxy" because it's ignored if
-            // shadowPrototypeProperties is true
-            return AppendNamedPropertyIds(cx, nullptr, names, true, props);
+            return true;
             """))
 
 
 class CGPrototypeTraitsClass(CGClass):
     def __init__(self, descriptor, indent=''):
         templateArgs = [Argument('prototypes::ID', 'PrototypeID')]
         templateSpecialization = ['prototypes::id::' + descriptor.name]
         enums = [ClassEnum('', ['Depth'],
--- a/dom/bindings/WebIDLGlobalNameHash.cpp
+++ b/dom/bindings/WebIDLGlobalNameHash.cpp
@@ -302,30 +302,34 @@ WebIDLGlobalNameHash::MayResolve(jsid aI
   // Rooting analysis thinks nsTHashtable<...>::Contains may GC because it ends
   // up calling through PLDHashTableOps' matchEntry function pointer, but we
   // know WebIDLNameTableEntry::KeyEquals can't cause a GC.
   JS::AutoSuppressGCAnalysis suppress;
   return sWebIDLGlobalNames->Contains(key);
 }
 
 /* static */
-void
+bool
 WebIDLGlobalNameHash::GetNames(JSContext* aCx, JS::Handle<JSObject*> aObj,
-                               NameType aNameType, nsTArray<nsString>& aNames)
+                               NameType aNameType, JS::AutoIdVector& aNames)
 {
   // aObj is always a Window here, so GetProtoAndIfaceCache on it is safe.
   ProtoAndIfaceCache* cache = GetProtoAndIfaceCache(aObj);
   for (auto iter = sWebIDLGlobalNames->Iter(); !iter.Done(); iter.Next()) {
     const WebIDLNameTableEntry* entry = iter.Get();
     // If aNameType is not AllNames, only include things whose entry slot in the
     // ProtoAndIfaceCache is null.
     if ((aNameType == AllNames ||
          !cache->EntrySlotIfExists(entry->mConstructorId)) &&
         (!entry->mEnabled || entry->mEnabled(aCx, aObj))) {
-      AppendASCIItoUTF16(nsDependentCString(sNames + entry->mNameOffset,
-                                            entry->mNameLength),
-                         *aNames.AppendElement());
+      JSString* str = JS_AtomizeStringN(aCx, sNames + entry->mNameOffset,
+                                        entry->mNameLength);
+      if (!str || !aNames.append(NON_INTEGER_ATOM_TO_JSID(str))) {
+        return false;
+      }
     }
   }
+
+  return true;
 }
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/bindings/WebIDLGlobalNameHash.h
+++ b/dom/bindings/WebIDLGlobalNameHash.h
@@ -58,18 +58,20 @@ public:
   // The type of names we're asking for.
   enum NameType {
     // All WebIDL names enabled for aObj.
     AllNames,
     // Only the names that are enabled for aObj and have not been resolved for
     // aObj in the past (and therefore can't have been deleted).
     UnresolvedNamesOnly
   };
-  static void GetNames(JSContext* aCx, JS::Handle<JSObject*> aObj,
-                       NameType aNameType, nsTArray<nsString>& aNames);
+  // Returns false if an exception has been thrown on aCx.
+  static bool GetNames(JSContext* aCx, JS::Handle<JSObject*> aObj,
+                       NameType aNameType,
+                       JS::AutoIdVector& aNames);
 
 private:
   friend struct WebIDLNameTableEntry;
 
   // The total number of names that we will add to the hash.
   // The value of sCount is generated by Codegen.py in RegisterBindings.cpp.
   static const uint32_t sCount;