Bug 1383245 - Prevent repeatedly computing the hash of a NodeInfoInner object; r=smaug
authorEhsan Akhgari <ehsan@mozilla.com>
Fri, 21 Jul 2017 15:43:18 -0400
changeset 419066 2ba59555d5299cc236923d91d8cf8c1e7e85f51a
parent 419065 1165fd9f0b437fd8fc6acc738029911ef23709c8
child 419067 66f0d5a2c077325dcd716a2a0bc6192bc4fc9fae
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1383245
milestone56.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 1383245 - Prevent repeatedly computing the hash of a NodeInfoInner object; r=smaug In practice we never modify the name and atom after setting them, so we make them const here, and compute the hash once if needed lazily.
dom/base/NodeInfo.cpp
dom/base/NodeInfo.h
dom/base/nsNodeInfoManager.cpp
--- a/dom/base/NodeInfo.cpp
+++ b/dom/base/NodeInfo.cpp
@@ -36,29 +36,21 @@ using mozilla::dom::NodeInfo;
 NodeInfo::~NodeInfo()
 {
   mOwnerManager->RemoveNodeInfo(this);
 }
 
 NodeInfo::NodeInfo(nsIAtom *aName, nsIAtom *aPrefix, int32_t aNamespaceID,
                    uint16_t aNodeType, nsIAtom* aExtraName,
                    nsNodeInfoManager *aOwnerManager)
+  : mDocument(aOwnerManager->GetDocument()),
+    mInner(aName, aPrefix, aNamespaceID, aNodeType, aExtraName),
+    mOwnerManager(aOwnerManager)
 {
   CheckValidNodeInfo(aNodeType, aName, aNamespaceID, aExtraName);
-  MOZ_ASSERT(aOwnerManager, "Invalid aOwnerManager");
-
-  // Initialize mInner
-  mInner.mName = aName;
-  mInner.mPrefix = aPrefix;
-  mInner.mNamespaceID = aNamespaceID;
-  mInner.mNodeType = aNodeType;
-  mOwnerManager = aOwnerManager;
-  mInner.mExtraName = aExtraName;
-
-  mDocument = aOwnerManager->GetDocument();
 
   // Now compute our cached members.
 
   // Qualified name.  If we have no prefix, use ToString on
   // mInner.mName so that we get to share its buffer.
   if (aPrefix) {
     mQualifiedName = nsDependentAtomString(mInner.mPrefix) +
                      NS_LITERAL_STRING(":") +
--- a/dom/base/NodeInfo.h
+++ b/dom/base/NodeInfo.h
@@ -241,38 +241,43 @@ protected:
    * the key is automatically deleted.
    */
 
   class NodeInfoInner
   {
   public:
     NodeInfoInner()
       : mName(nullptr), mPrefix(nullptr), mNamespaceID(kNameSpaceID_Unknown),
-        mNodeType(0), mNameString(nullptr), mExtraName(nullptr)
+        mNodeType(0), mNameString(nullptr), mExtraName(nullptr),
+        mHash(0), mHashInitialized(false)
     {
     }
     NodeInfoInner(nsIAtom *aName, nsIAtom *aPrefix, int32_t aNamespaceID,
                     uint16_t aNodeType, nsIAtom* aExtraName)
       : mName(aName), mPrefix(aPrefix), mNamespaceID(aNamespaceID),
-        mNodeType(aNodeType), mNameString(nullptr), mExtraName(aExtraName)
+        mNodeType(aNodeType), mNameString(nullptr), mExtraName(aExtraName),
+        mHash(aName->hash()), mHashInitialized(true)
     {
     }
     NodeInfoInner(const nsAString& aTmpName, nsIAtom *aPrefix,
                     int32_t aNamespaceID, uint16_t aNodeType)
       : mName(nullptr), mPrefix(aPrefix), mNamespaceID(aNamespaceID),
-        mNodeType(aNodeType), mNameString(&aTmpName), mExtraName(nullptr)
+        mNodeType(aNodeType), mNameString(&aTmpName), mExtraName(nullptr),
+        mHash(0), mHashInitialized(false)
     {
     }
 
-    nsCOMPtr<nsIAtom> mName;
+    const nsCOMPtr<nsIAtom> mName;
     nsCOMPtr<nsIAtom> mPrefix;
     int32_t             mNamespaceID;
     uint16_t            mNodeType; // As defined by nsIDOMNode.nodeType
-    const nsAString*    mNameString;
+    const nsAString* const mNameString;
     nsCOMPtr<nsIAtom> mExtraName; // Only used by PIs and DocTypes
+    PLHashNumber      mHash;
+    bool              mHashInitialized;
   };
 
   // nsNodeInfoManager needs to pass mInner to the hash table.
   friend class ::nsNodeInfoManager;
 
   // This is a non-owning reference, but it's safe since it's set to nullptr
   // by nsNodeInfoManager::DropDocumentReference when the document is destroyed.
   nsIDocument* MOZ_NON_OWNING_REF mDocument; // Cache of mOwnerManager->mDocument
--- a/dom/base/nsNodeInfoManager.cpp
+++ b/dom/base/nsNodeInfoManager.cpp
@@ -39,19 +39,24 @@ using mozilla::dom::NodeInfo;
 
 static LazyLogModule gNodeInfoManagerLeakPRLog("NodeInfoManagerLeak");
 
 PLHashNumber
 nsNodeInfoManager::GetNodeInfoInnerHashValue(const void *key)
 {
   MOZ_ASSERT(key, "Null key passed to NodeInfo::GetHashValue!");
 
-  auto *node = reinterpret_cast<const NodeInfo::NodeInfoInner*>(key);
+  auto *node = const_cast<NodeInfo::NodeInfoInner*>
+    (reinterpret_cast<const NodeInfo::NodeInfoInner*>(key));
+  if (!node->mHashInitialized) {
+    node->mHash = node->mName ? node->mName->hash() : HashString(*(node->mNameString));
+    node->mHashInitialized = true;
+  }
 
-  return node->mName ? node->mName->hash() : HashString(*(node->mNameString));
+  return node->mHash;
 }
 
 
 int
 nsNodeInfoManager::NodeInfoInnerKeyCompare(const void *key1, const void *key2)
 {
   MOZ_ASSERT(key1 && key2, "Null key passed to NodeInfoInnerKeyCompare!");