commit 5d6714e3e3813c5c8f9e411e04427356ddf33b6e
author Lei Zhang <thestig@chromium.org> Tue Dec 11 18:53:31 2018 +0000
committer Chromium commit bot <commit-bot@chromium.org> Tue Dec 11 18:53:31 2018 +0000
tree 45de863b33500573f2f9a078f0f155f373464bae
parent baa664a4e0d1db158c623efa3d3267a79c24e486

Reduce reallocation in string classes.

Increase the size of the underlying storage geometrically when
concatenating to reduce the cost of reallocations. Use this along with
Reserve() to make CJS_Util::StringPrintx() faster.

BUG=chromium:912790

Change-Id: I6d143ed02a035ea00a74b90b5c0793557a4d6528
Reviewed-on: https://pdfium-review.googlesource.com/c/46894
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

core/fxcrt/bytestring.cpp

diff --git a/core/fxcrt/bytestring.cpp b/core/fxcrt/bytestring.cpp
index d56bdef..4fc0728 100644
--- a/core/fxcrt/bytestring.cpp
+++ b/core/fxcrt/bytestring.cpp
@@ -455,10 +455,12 @@
     return;
   }
 
+  size_t nConcatLen = std::max(m_pData->m_nDataLength / 2, nSrcLen);
   RetainPtr<StringData> pNewData(
-      StringData::Create(m_pData->m_nDataLength + nSrcLen));
+      StringData::Create(m_pData->m_nDataLength + nConcatLen));
   pNewData->CopyContents(*m_pData);
   pNewData->CopyContentsAt(m_pData->m_nDataLength, pSrcData, nSrcLen);
+  pNewData->m_nDataLength = m_pData->m_nDataLength + nSrcLen;
   m_pData.Swap(pNewData);
 }
 

core/fxcrt/bytestring_unittest.cpp

diff --git a/core/fxcrt/bytestring_unittest.cpp b/core/fxcrt/bytestring_unittest.cpp
index acce2ec..1b932bf 100644
--- a/core/fxcrt/bytestring_unittest.cpp
+++ b/core/fxcrt/bytestring_unittest.cpp
@@ -395,6 +395,24 @@
   EXPECT_EQ("Dogs like me", ByteString("Dogs") + " like me");
   EXPECT_EQ("Oh no, error number 42",
             "Oh no, error number " + ByteString::Format("%d", 42));
+
+  {
+    // Make sure operator+= and Concat() increases string memory allocation
+    // geometrically.
+    int allocations = 0;
+    ByteString str("ABCDEFGHIJKLMN");
+    const char* buffer = str.c_str();
+    for (size_t i = 0; i < 10000; ++i) {
+      str += "!";
+      const char* new_buffer = str.c_str();
+      if (new_buffer != buffer) {
+        buffer = new_buffer;
+        ++allocations;
+      }
+    }
+    EXPECT_LT(allocations, 25);
+    EXPECT_GT(allocations, 10);
+  }
 }
 
 TEST(ByteString, Concat) {

core/fxcrt/widestring.cpp

diff --git a/core/fxcrt/widestring.cpp b/core/fxcrt/widestring.cpp
index 7dc0d21..1ba0011 100644
--- a/core/fxcrt/widestring.cpp
+++ b/core/fxcrt/widestring.cpp
@@ -623,10 +623,12 @@
     return;
   }
 
+  size_t nConcatLen = std::max(m_pData->m_nDataLength / 2, nSrcLen);
   RetainPtr<StringData> pNewData(
-      StringData::Create(m_pData->m_nDataLength + nSrcLen));
+      StringData::Create(m_pData->m_nDataLength + nConcatLen));
   pNewData->CopyContents(*m_pData);
   pNewData->CopyContentsAt(m_pData->m_nDataLength, pSrcData, nSrcLen);
+  pNewData->m_nDataLength = m_pData->m_nDataLength + nSrcLen;
   m_pData.Swap(pNewData);
 }
 

core/fxcrt/widestring_unittest.cpp

diff --git a/core/fxcrt/widestring_unittest.cpp b/core/fxcrt/widestring_unittest.cpp
index 41465c0..09b9645 100644
--- a/core/fxcrt/widestring_unittest.cpp
+++ b/core/fxcrt/widestring_unittest.cpp
@@ -402,6 +402,24 @@
   EXPECT_EQ(L"Dogs like me", WideString(L"Dogs") + L" like me");
   EXPECT_EQ(L"Oh no, error number 42",
             L"Oh no, error number " + WideString::Format(L"%d", 42));
+
+  {
+    // Make sure operator+= and Concat() increases string memory allocation
+    // geometrically.
+    int allocations = 0;
+    WideString str(L"ABCDEFGHIJKLMN");
+    const wchar_t* buffer = str.c_str();
+    for (size_t i = 0; i < 10000; ++i) {
+      str += L"!";
+      const wchar_t* new_buffer = str.c_str();
+      if (new_buffer != buffer) {
+        buffer = new_buffer;
+        ++allocations;
+      }
+    }
+    EXPECT_LT(allocations, 25);
+    EXPECT_GT(allocations, 10);
+  }
 }
 
 TEST(WideString, ConcatInPlace) {

fxjs/cjs_util.cpp

diff --git a/fxjs/cjs_util.cpp b/fxjs/cjs_util.cpp
index 5abf684..ce8dd77 100644
--- a/fxjs/cjs_util.cpp
+++ b/fxjs/cjs_util.cpp
@@ -277,6 +277,7 @@
 WideString CJS_Util::StringPrintx(const WideString& wsFormat,
                                   const WideString& wsSource) {
   WideString wsResult;
+  wsResult.Reserve(wsFormat.GetLength());
   size_t iSourceIdx = 0;
   size_t iFormatIdx = 0;
   CaseMode eCaseMode = kPreserveCase;