QString::replace: fix a performance regression

eb60e940202857dc155f1a0e499364962faad7f6 refactored the QString::replace
helpers as to avoid a full copy of a string (when detaching) only to
then modify it in order to apply replacements into the new copy.

Unfortunately it also introduced a regression: if we are replacing into
a string that does not need detaching (that is: it's non-static and
there are no shallow copies of it), and the string does not have
sufficient capacity, then the code chose to allocate a brand new string
and do piecewise copies/replacements into the new string.

This is suboptimal, as it precludes the possibility that the string gets
reallocated in place (via a realloc() that doesn't actually move the
string data in memory), which is a huge performance win. (It also
precludes the possibility of compacting the string data "to the left" in
case we do have sufficient capacity, but not at the "right end".)

This caused a 3x slowdown when replacing in large strings.

Solve this by removing the capacity check and unconditionally
reserve()ing enough space so that we can then do in-place replacements.

Change-Id: I921e3ea65222eca8125996d8d3ea77e9c09ba205
Pick-to: 6.7
Fixes: QTBUG-127549
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
(cherry picked from commit 315210de916d060c044c01e53ff249d676122b1b)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Giuseppe D'Angelo 2024-08-19 18:51:57 +02:00 committed by Qt Cherry-pick Bot
parent 6d8bab3073
commit 1f3e8c3162

View File

@ -3771,11 +3771,13 @@ static void replace_helper(QString &str, QSpan<size_t> indices, qsizetype blen,
const qsizetype oldSize = str.data_ptr().size;
const qsizetype adjust = indices.size() * (after.size() - blen);
const qsizetype newSize = oldSize + adjust;
if (str.data_ptr().needsDetach() || needsReallocate(str, newSize)) {
if (str.data_ptr().needsDetach()) {
replace_with_copy(str, indices, blen, after);
return;
}
str.reserve(newSize);
if (QtPrivate::q_points_into_range(after.begin(), str))
// Copy after if it lies inside our own d.b area (which we could
// possibly invalidate via a realloc or modify by replacement)