Commit 167904e8 authored by Daniel McArdle's avatar Daniel McArdle Committed by Commit Bot

Avoid vector::insert in base::OffsetAdjuster::MergeSequentialAdjustments

This CL trades space for time in |MergeSequentialAdjustments|. By
allocating a new |Adjustments| vector, we can avoid calling
|std::vector::insert| in a loop, which costs roughly O(n^2) time. With
the additional vector, we only need to call |std::vector::push_back|,
costing roughly O(n) time in total.

Bug: 1017193
Change-Id: I7beb2c3c0b6a9cb7616a22f4a4355285fd281894
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2070605Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Dan McArdle <dmcardle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746171}
parent 39798422
...@@ -90,16 +90,22 @@ void OffsetAdjuster::MergeSequentialAdjustments( ...@@ -90,16 +90,22 @@ void OffsetAdjuster::MergeSequentialAdjustments(
auto adjusted_iter = adjustments_on_adjusted_string->begin(); auto adjusted_iter = adjustments_on_adjusted_string->begin();
auto first_iter = first_adjustments.begin(); auto first_iter = first_adjustments.begin();
// Simultaneously iterate over all |adjustments_on_adjusted_string| and // Simultaneously iterate over all |adjustments_on_adjusted_string| and
// |first_adjustments|, adding adjustments to or correcting the adjustments // |first_adjustments|, pushing adjustments at the end of
// in |adjustments_on_adjusted_string| as we go. |shift| keeps track of the // |adjustments_builder| as we go. |shift| keeps track of the current number
// current number of characters collapsed by |first_adjustments| up to this // of characters collapsed by |first_adjustments| up to this point.
// point. |currently_collapsing| keeps track of the number of characters // |currently_collapsing| keeps track of the number of characters collapsed by
// collapsed by |first_adjustments| into the current |adjusted_iter|'s // |first_adjustments| into the current |adjusted_iter|'s length. These are
// length. These are characters that will change |shift| as soon as we're // characters that will change |shift| as soon as we're done processing the
// done processing the current |adjusted_iter|; they are not yet reflected in // current |adjusted_iter|; they are not yet reflected in |shift|.
// |shift|.
size_t shift = 0; size_t shift = 0;
size_t currently_collapsing = 0; size_t currently_collapsing = 0;
// While we *could* update |adjustments_on_adjusted_string| in place by
// inserting new adjustments into the middle, we would be repeatedly calling
// |std::vector::insert|. That would cost O(n) time per insert, relative to
// distance from end of the string. By instead allocating
// |adjustments_builder| and calling |std::vector::push_back|, we only pay
// amortized constant time per push. We are trading space for time.
Adjustments adjustments_builder;
while (adjusted_iter != adjustments_on_adjusted_string->end()) { while (adjusted_iter != adjustments_on_adjusted_string->end()) {
if ((first_iter == first_adjustments.end()) || if ((first_iter == first_adjustments.end()) ||
((adjusted_iter->original_offset + shift + ((adjusted_iter->original_offset + shift +
...@@ -112,6 +118,7 @@ void OffsetAdjuster::MergeSequentialAdjustments( ...@@ -112,6 +118,7 @@ void OffsetAdjuster::MergeSequentialAdjustments(
adjusted_iter->original_offset += shift; adjusted_iter->original_offset += shift;
shift += currently_collapsing; shift += currently_collapsing;
currently_collapsing = 0; currently_collapsing = 0;
adjustments_builder.push_back(*adjusted_iter);
++adjusted_iter; ++adjusted_iter;
} else if ((adjusted_iter->original_offset + shift) > } else if ((adjusted_iter->original_offset + shift) >
first_iter->original_offset) { first_iter->original_offset) {
...@@ -127,15 +134,9 @@ void OffsetAdjuster::MergeSequentialAdjustments( ...@@ -127,15 +134,9 @@ void OffsetAdjuster::MergeSequentialAdjustments(
DCHECK_LE(first_iter->original_offset + first_iter->output_length, DCHECK_LE(first_iter->original_offset + first_iter->output_length,
adjusted_iter->original_offset + shift); adjusted_iter->original_offset + shift);
// Add the |first_adjustment_iter| to the full set of adjustments while // Add the |first_iter| to the full set of adjustments.
// making sure |adjusted_iter| continues pointing to the same element.
// We do this by inserting the |first_adjustment_iter| right before
// |adjusted_iter|, then incrementing |adjusted_iter| so it points to
// the following element.
shift += first_iter->original_length - first_iter->output_length; shift += first_iter->original_length - first_iter->output_length;
adjusted_iter = adjustments_on_adjusted_string->insert( adjustments_builder.push_back(*first_iter);
adjusted_iter, *first_iter);
++adjusted_iter;
++first_iter; ++first_iter;
} else { } else {
// The first adjustment adjusted something that then got further adjusted // The first adjustment adjusted something that then got further adjusted
...@@ -168,10 +169,10 @@ void OffsetAdjuster::MergeSequentialAdjustments( ...@@ -168,10 +169,10 @@ void OffsetAdjuster::MergeSequentialAdjustments(
// (Their offsets are already correct with respect to the original string.) // (Their offsets are already correct with respect to the original string.)
// Append them all. // Append them all.
DCHECK(adjusted_iter == adjustments_on_adjusted_string->end()); DCHECK(adjusted_iter == adjustments_on_adjusted_string->end());
adjustments_on_adjusted_string->insert( adjustments_builder.insert(adjustments_builder.end(), first_iter,
adjustments_on_adjusted_string->end(), first_iter, first_adjustments.end());
first_adjustments.end());
} }
*adjustments_on_adjusted_string = std::move(adjustments_builder);
} }
// Converts the given source Unicode character type to the given destination // Converts the given source Unicode character type to the given destination
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment