Optimize Boundary Finding with Progressive Text Accumulation

Optimize Boundary Finding with Progressive Text Accumulation

When searching for boundaries, we ensure that each time |copyTextTo()|
is called, the length of text copied is bounded by the buffer capacity,
and hence, avoiding copying everything of a large text node all at once.

This is part of a bigger CL [1].

[1] https://codereview.chromium.org/1604783002

BUG=109587
TEST=n/a; no behavior change

Committed: https://crrev.com/0fb56f3c6fa1f13cd17e714217900b73c1b75c3d
Cr-Commit-Position: refs/heads/master@{#375565}

[Xiaocheng, 2016-02-10 07:56:43]

Description was changed from

==========
Optimize previousBoundary and nextBoundary with TextAccumulator

This is a follow-up CL of [1]. Both CLs are part of [2].

[1] https://codereview.chromium.org/1678803002
[2] https://codereview.chromium.org/1604783002

BUG=109587
TEST=n/a; no behavior change
==========

to

==========
Optimize previousBoundary and nextBoundary with TextAccumulator

With |TextAccumulator|, when searching for boundaries, we no longer
copy everything of a huge text node into a temporary buffer all
at once. Instead, we progressively copy text to a |TextAccumulator|,
and achieve better performance when the boundary is not far from
where the search started.

This is a follow-up CL of [1]. Both CLs are part of [2].

[1] https://codereview.chromium.org/1678803002
[2] https://codereview.chromium.org/1604783002

BUG=109587
TEST=n/a; no behavior change
==========

[Xiaocheng, 2016-02-16 07:10:23]

Description was changed from

==========
Optimize previousBoundary and nextBoundary with TextAccumulator

With |TextAccumulator|, when searching for boundaries, we no longer
copy everything of a huge text node into a temporary buffer all
at once. Instead, we progressively copy text to a |TextAccumulator|,
and achieve better performance when the boundary is not far from
where the search started.

This is a follow-up CL of [1]. Both CLs are part of [2].

[1] https://codereview.chromium.org/1678803002
[2] https://codereview.chromium.org/1604783002

BUG=109587
TEST=n/a; no behavior change
==========

to

==========
Optimize Boundary Finding with Progressive Text Accumulation

When searching for boundaries, we ensure that each time |copyTextTo()|
is called, the amount of text copied is linear to the buffer capacity,
and hence, avoiding copying everything of a large text node all at once.

This is part of a bigger CL [1].

[1] https://codereview.chromium.org/1604783002

BUG=109587
TEST=n/a; no behavior change
==========

[Xiaocheng, 2016-02-16 07:23:28]

xiaochengh@chromium.org changed reviewers:
+ yosin@chromium.org

[Xiaocheng, 2016-02-16 07:23:29]

PTAL.

Following the discussion thread of https://codereview.chromium.org/1678803002,
this patch
implements the idea of using TextBuffer's |capacity()| for progressive text
accumulation.

[yosin_UTC9, 2016-02-16 07:55:13]

lgtm w/ nit

https://codereview.chromium.org/1687813002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/VisibleUnits.cpp (right):

https://codereview.chromium.org/1687813002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/VisibleUnits.cpp:693: int itRemain = 0;
|itRemain| is confusing name, e.g. it is an iterator since "it" looks like
prefix.

Option 1: (not good, since life time of |boundaryOffset| is too long.)
Can we declare |boundaryOffset| here and use it as replacement of |itRemain|?

int boundaryOffset = 0;
....
// was int boundaryOffset = itRemain + next
boundaryOffset += next;

Option 2: (Fumm...)
s/itRemain/remainingLength/

Option 3: 
better name!

[Xiaocheng, 2016-02-16 12:33:24]

Description was changed from

==========
Optimize Boundary Finding with Progressive Text Accumulation

When searching for boundaries, we ensure that each time |copyTextTo()|
is called, the amount of text copied is linear to the buffer capacity,
and hence, avoiding copying everything of a large text node all at once.

This is part of a bigger CL [1].

[1] https://codereview.chromium.org/1604783002

BUG=109587
TEST=n/a; no behavior change
==========

to

==========
Optimize Boundary Finding with Progressive Text Accumulation

When searching for boundaries, we ensure that each time |copyTextTo()|
is called, the length of text copied is bounded by the buffer capacity,
and hence, avoiding copying everything of a large text node all at once.

This is part of a bigger CL [1].

[1] https://codereview.chromium.org/1604783002

BUG=109587
TEST=n/a; no behavior change
==========

[Xiaocheng, 2016-02-16 12:39:30]

The CQ bit was checked by xiaochengh@chromium.org

[Xiaocheng, 2016-02-16 12:39:30]

Thanks for the review.

|remainingLength| looks a good enough name to me. I'll take it :)

[Xiaocheng, 2016-02-16 12:39:30]

The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/1687813002/#ps60001
(title: " ")

[commit-bot: I haz the power, 2016-02-16 12:40:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1687813002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1687813002/60001

[commit-bot: I haz the power, 2016-02-16 14:20:41]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-02-16 22:52:34]

Description was changed from

==========
Optimize Boundary Finding with Progressive Text Accumulation

When searching for boundaries, we ensure that each time |copyTextTo()|
is called, the length of text copied is bounded by the buffer capacity,
and hence, avoiding copying everything of a large text node all at once.

This is part of a bigger CL [1].

[1] https://codereview.chromium.org/1604783002

BUG=109587
TEST=n/a; no behavior change
==========

to

==========
Optimize Boundary Finding with Progressive Text Accumulation

When searching for boundaries, we ensure that each time |copyTextTo()|
is called, the length of text copied is bounded by the buffer capacity,
and hence, avoiding copying everything of a large text node all at once.

This is part of a bigger CL [1].

[1] https://codereview.chromium.org/1604783002

BUG=109587
TEST=n/a; no behavior change

Committed: https://crrev.com/0fb56f3c6fa1f13cd17e714217900b73c1b75c3d
Cr-Commit-Position: refs/heads/master@{#375565}
==========

[commit-bot: I haz the power, 2016-02-16 22:52:35]

Patchset 4 (id:??) landed as
https://crrev.com/0fb56f3c6fa1f13cd17e714217900b73c1b75c3d
Cr-Commit-Position: refs/heads/master@{#375565}