Bug 1530208. Fix isEqualNode to not do a bunch of string-copying. r=mccr8
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 04 Mar 2019 19:43:47 +0000
changeset 520168 a1fc95a1f08d57bf14bd36b6420ccf9f8035e443
parent 520167 face9d1e886872033b70fbdff5a40eaa244db3fb
child 520169 469b9a04697237a28a1a76e364803c9645b1e1ba
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1530208
milestone67.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 1530208. Fix isEqualNode to not do a bunch of string-copying. r=mccr8 Differential Revision: https://phabricator.services.mozilla.com/D21824
dom/base/CharacterData.h
dom/base/nsINode.cpp
dom/base/nsTextFragment.cpp
dom/base/nsTextFragment.h
--- a/dom/base/CharacterData.h
+++ b/dom/base/CharacterData.h
@@ -203,16 +203,24 @@ class CharacterData : public nsIContent 
 
 #ifdef DEBUG
   void ToCString(nsAString& aBuf, int32_t aOffset, int32_t aLen) const;
 #endif
 
   NS_DECL_CYCLE_COLLECTION_SKIPPABLE_SCRIPT_HOLDER_CLASS_INHERITED(
       CharacterData, nsIContent)
 
+  /**
+   * Compare two CharacterData nodes for text equality.
+   */
+  MOZ_MUST_USE
+  bool TextEquals(const CharacterData* aOther) const {
+    return mText.TextEquals(aOther->mText);
+  }
+
  protected:
   virtual ~CharacterData();
 
   Element* GetNameSpaceElement() final {
     return Element::FromNodeOrNull(GetParentNode());
   }
 
   nsresult SetTextInternal(
--- a/dom/base/nsINode.cpp
+++ b/dom/base/nsINode.cpp
@@ -811,16 +811,22 @@ uint16_t nsINode::CompareDocumentPositio
 
 bool nsINode::IsSameNode(nsINode* other) { return other == this; }
 
 bool nsINode::IsEqualNode(nsINode* aOther) {
   if (!aOther) {
     return false;
   }
 
+  // Might as well do a quick check to avoid walking our kids if we're
+  // obviously the same.
+  if (aOther == this) {
+    return true;
+  }
+
   nsAutoString string1, string2;
 
   nsINode* node1 = this;
   nsINode* node2 = aOther;
   do {
     uint16_t nodeType = node1->NodeType();
     if (nodeType != node2->NodeType()) {
       return false;
@@ -863,22 +869,20 @@ bool nsINode::IsEqualNode(nsINode* aOthe
         break;
       }
       case TEXT_NODE:
       case COMMENT_NODE:
       case CDATA_SECTION_NODE:
       case PROCESSING_INSTRUCTION_NODE: {
         MOZ_ASSERT(node1->IsCharacterData());
         MOZ_ASSERT(node2->IsCharacterData());
-        string1.Truncate();
-        static_cast<CharacterData*>(node1)->AppendTextTo(string1);
-        string2.Truncate();
-        static_cast<CharacterData*>(node2)->AppendTextTo(string2);
-
-        if (!string1.Equals(string2)) {
+        auto* data1 = static_cast<CharacterData*>(node1);
+        auto* data2 = static_cast<CharacterData*>(node2);
+
+        if (!data1->TextEquals(data2)) {
           return false;
         }
 
         break;
       }
       case DOCUMENT_NODE:
       case DOCUMENT_FRAGMENT_NODE:
         break;
--- a/dom/base/nsTextFragment.cpp
+++ b/dom/base/nsTextFragment.cpp
@@ -485,8 +485,46 @@ size_t nsTextFragment::SizeOfExcludingTh
 // every allocation
 void nsTextFragment::UpdateBidiFlag(const char16_t* aBuffer, uint32_t aLength) {
   if (mState.mIs2b && !mState.mIsBidi) {
     if (HasRTLChars(MakeSpan(aBuffer, aLength))) {
       mState.mIsBidi = true;
     }
   }
 }
+
+bool nsTextFragment::TextEquals(const nsTextFragment& aOther) const {
+  if (!Is2b()) {
+    // We're 1-byte.
+    if (!aOther.Is2b()) {
+      nsDependentCSubstring ourStr(Get1b(), GetLength());
+      return ourStr.Equals(
+          nsDependentCSubstring(aOther.Get1b(), aOther.GetLength()));
+    }
+
+    // We're 1-byte, the other thing is 2-byte.  Instead of implementing a
+    // separate codepath for this, just use our code below.
+    return aOther.TextEquals(*this);
+  }
+
+  nsDependentSubstring ourStr(Get2b(), GetLength());
+  if (aOther.Is2b()) {
+    return ourStr.Equals(
+        nsDependentSubstring(aOther.Get2b(), aOther.GetLength()));
+  }
+
+  // We can't use EqualsASCII here, because the other string might not
+  // actually be ASCII.  Just roll our own compare; do it in the simple way.
+  // Bug 1532356 tracks not having to roll our own.
+  if (GetLength() != aOther.GetLength()) {
+    return false;
+  }
+
+  const char16_t* ourChars = Get2b();
+  const char* otherChars = aOther.Get1b();
+  for (uint32_t i = 0; i < GetLength(); ++i) {
+    if (ourChars[i] != static_cast<char16_t>(otherChars[i])) {
+      return false;
+    }
+  }
+
+  return true;
+}
--- a/dom/base/nsTextFragment.h
+++ b/dom/base/nsTextFragment.h
@@ -231,16 +231,22 @@ class nsTextFragment final {
     // NS_MAX_TEXT_FRAGMENT_LENGTH.
     uint32_t mLength : 29;
   };
 
 #define NS_MAX_TEXT_FRAGMENT_LENGTH (static_cast<uint32_t>(0x1FFFFFFF))
 
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
+  /**
+   * Check whether the text in this fragment is the same as the text in the
+   * other fragment.
+   */
+  MOZ_MUST_USE bool TextEquals(const nsTextFragment& aOther) const;
+
  private:
   void ReleaseText();
 
   /**
    * Scan the contents of the fragment and turn on mState.mIsBidi if it
    * includes any Bidi characters.
    */
   void UpdateBidiFlag(const char16_t* aBuffer, uint32_t aLength);