From 072682ce44feeb59b6696e5aa484f453b719fec6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 7 Mar 2009 16:07:58 +0000 Subject: [PATCH] optimize Replace() to use a linear algorithm (closes #9135) git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@59420 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- docs/changes.txt | 1 + src/common/string.cpp | 69 ++++++++++++++++++++++++++---------- tests/benchmarks/strings.cpp | 12 +++++++ tests/strings/strings.cpp | 8 +++-- 4 files changed, 70 insertions(+), 20 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index d0ef4c1da9..d90c86629a 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -373,6 +373,7 @@ All: - Added wxLog::Log(). - Added wxProtocolLog and use it in wxFTP. - Added wxXmlResource::GetResourceNode(). +- Optimize wxString::Replace() to use an O(N) algorithm (Kuang-che Wu). All (Unix): diff --git a/src/common/string.cpp b/src/common/string.cpp index 56af51ae22..e07b7eea8f 100644 --- a/src/common/string.cpp +++ b/src/common/string.cpp @@ -36,6 +36,7 @@ #include #include "wx/hashmap.h" +#include "wx/vector.h" // string handling functions used by wxString: #if wxUSE_UNICODE_UTF8 @@ -1397,30 +1398,62 @@ size_t wxString::Replace(const wxString& strOld, break; } } - else // general case + else if ( !bReplaceAll) + { + size_t pos = m_impl.find(strOld, 0); + if ( pos != npos ) + { + m_impl.replace(pos, strOld.m_impl.length(), strNew.m_impl); + uiCount = 1; + } + } + else // replace all occurrences { const size_t uiOldLen = strOld.m_impl.length(); const size_t uiNewLen = strNew.m_impl.length(); - for ( size_t pos = 0; ; ) + // first scan the string to find all positions at which the replacement + // should be made + wxVector replacePositions; + + size_t pos; + for ( pos = m_impl.find(strOld.m_impl, 0); + pos != npos; + pos = m_impl.find(strOld.m_impl, pos + uiOldLen)) { - pos = m_impl.find(strOld.m_impl, pos); - if ( pos == npos ) - break; - - // replace this occurrence of the old string with the new one - m_impl.replace(pos, uiOldLen, strNew.m_impl); - - // move up pos past the string that was replaced - pos += uiNewLen; - - // increase replace count - uiCount++; - - // stop after the first one? - if ( !bReplaceAll ) - break; + replacePositions.push_back(pos); + ++uiCount; } + + if ( !uiCount ) + return 0; + + // allocate enough memory for the whole new string + wxString tmp; + tmp.m_impl.reserve(m_impl.length() + uiCount*(uiNewLen - uiOldLen)); + + // copy this string to tmp doing replacements on the fly + size_t replNum = 0; + for ( pos = 0; replNum < uiCount; replNum++ ) + { + const size_t nextReplPos = replacePositions[replNum]; + + if ( pos != nextReplPos ) + { + tmp.m_impl.append(m_impl, pos, nextReplPos - pos); + } + + tmp.m_impl.append(strNew.m_impl); + pos = nextReplPos + uiOldLen; + } + + if ( pos != m_impl.length() ) + { + // append the rest of the string unchanged + tmp.m_impl.append(m_impl, pos, m_impl.length() - pos); + } + + swap(tmp); } return uiCount; diff --git a/tests/benchmarks/strings.cpp b/tests/benchmarks/strings.cpp index 9449f49110..cc09a35418 100644 --- a/tests/benchmarks/strings.cpp +++ b/tests/benchmarks/strings.cpp @@ -281,6 +281,18 @@ BENCHMARK_FUNC(ReplaceAll) return str.Replace("x", "y") != 0; } +BENCHMARK_FUNC(ReplaceLonger) +{ + wxString str('x', ASCIISTR_LEN); + return str.Replace("x", "yy") != 0; +} + +BENCHMARK_FUNC(ReplaceShorter) +{ + wxString str('x', ASCIISTR_LEN); + return str.Replace("xx", "y") != 0; +} + // ---------------------------------------------------------------------------- // string buffers: wx[W]CharBuffer diff --git a/tests/strings/strings.cpp b/tests/strings/strings.cpp index 09686b11da..7fcc223c9c 100644 --- a/tests/strings/strings.cpp +++ b/tests/strings/strings.cpp @@ -318,7 +318,7 @@ void StringTestCase::Replace() { \ wxString s(o,olen); \ s.replace( pos , len , replacement ); \ - CPPUNIT_ASSERT( s == wxString(r,rlen) ); \ + CPPUNIT_ASSERT_EQUAL( wxString(r,rlen), s ); \ } TEST_NULLCHARREPLACE( _T("null\0char"), 9, 5, 1, _T("d"), @@ -328,7 +328,7 @@ void StringTestCase::Replace() { \ wxString s(o,olen); \ s.Replace( olds, news, all ); \ - CPPUNIT_ASSERT( s == wxString(r,rlen) ); \ + CPPUNIT_ASSERT_EQUAL( wxString(r,rlen), s ); \ } TEST_WXREPLACE( _T("null\0char"), 9, _T("c"), _T("de"), true, @@ -338,6 +338,10 @@ void StringTestCase::Replace() _T("null\0char"), 9 ); TEST_WXREPLACE( "life", 4, "f", "", false, "lie", 3 ); + TEST_WXREPLACE( "life", 4, "f", "", true, "lie", 3 ); + TEST_WXREPLACE( "life", 4, "fe", "ve", true, "live", 4 ); + TEST_WXREPLACE( "xx", 2, "x", "yy", true, "yyyy", 4 ); + TEST_WXREPLACE( "xxx", 3, "xx", "z", true, "zx", 2 ); #undef TEST_WXREPLACE #undef TEST_NULLCHARREPLACE