commit 1d0ecacaef43c03ac2e8a8a15d494c7e3fa13f34
author Tsuyoshi Horo <horo@chromium.org> Thu May 30 05:05:12 2024
committer Chromium LUCI CQ <chromium-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu May 30 05:05:12 2024
tree 43ad54b89e3af81c75c3af763a282d639c7e49e1
parent d724878be53a2f7b25854c6c0976b6dfaaf5bad2

Make WTF::SharedBuffer support appending data without copying

When loading a subresource, we use `WTF::SharedBuffer` to store resource
response data in `data_` of `blink::Resource`. Currently, there is no
way to append data to `WTF::SharedBuffer` without copying the data. This
is not a big issue for normal subresource loading where the main thread
reads data from the mojo data pipe using two-step read methods
(`BeginReadData()` and `EndReadData()`).

However, when `BackgroundResourceFetch` feature is enabled, the
background thread reads script resource response data from the mojo data
pipe, passes the data to the main thread as a `Vector<char>`, copies the
data, and appends it to `data_` of blink::Resource. This is not good
from a performance point of view. We need a way to append data without
copying.

Currently, `WTF::SharedBuffer` has two fields to hold data.
`Vector<char> buffer_` for the first part of data and
`Vector<Segment> segment_` for the following parts of data. `Segment` is
a memory buffer of fixed size (4KB).

This CL changes this `Segment` to a class with `size_t start_position_`
and `Vector<char> data_`, and makes WTF::SharedBuffer have
`Vector<Segment> segment_`. So we can append `Vector<char>` to
`WTF::SharedBuffer` without copying.

Performance testing shows that:
  [1] This change does not have a significant impact on performance when
      calling `Append()` with a span that causes data copying.
  [2] With this change, calling `Append()` with passing `Vector<char>`
      no longer causes data copying as it did before.
  [3] This change has a small negative performance impact on calling
      `GetIteratorAt()`. Before this CL, it was possible to find the
      segment containing the data at a particular location in constant
      time. But after this CL, finding the segment using
      `std::upper_bound` takes `O(log(segments_.size()))`.

=== Results of performance tests using https://crrev.com/c/5568780 ===

[1] SharedBuffer::Append(base::span) executed 1000 times

    data   |  MacBook Pro 2021   |   Android Pixel 2   |
    size   |  Before  |  After   |  Before  |  After   |
  ---------|----------|----------|----------|----------|
        10 | 0.000031 | 0.000061 | 0.000118 | 0.000236 |
       100 | 0.000078 | 0.000085 | 0.000259 | 0.000455 |
      1000 | 0.000183 | 0.000188 | 0.000744 | 0.000861 |
     10000 | 0.001196 | 0.001208 | 0.006413 | 0.006576 |
    100000 | 0.011896 | 0.011178 | 0.062527 | 0.060133 |
   1000000 | 0.117630 | 0.103153 | 0.590470 | 0.616549 |

[2] SharedBuffer::Append(std::move(Vector)) executed 1000 times

    data   |  MacBook Pro 2021   |   Android Pixel 2   |
    size   |  Before  |  After   |  Before  |  After   |
  ---------|----------|----------|----------|----------|
        10 | 0.000043 | 0.000054 | 0.000116 | 0.000138 |
       100 | 0.000057 | 0.000026 | 0.000238 | 0.000109 |
      1000 | 0.000394 | 0.000024 | 0.002911 | 0.000108 |
     10000 | 0.001012 | 0.000032 | 0.008447 | 0.000117 |
    100000 | 0.014492 | 0.000029 | 0.059325 | 0.000143 |
   1000000 | 0.136521 | 0.000032 | 0.942730 | 0.000178 |

[3] SharedBuffer::GetIteratorAt() executed 1,000,000 times

    data   |  MacBook Pro 2021   |   Android Pixel 2   |
    count  |  Before  |  After   |  Before  |  After   |
  ---------|----------|----------|----------|----------|
         1 | 0.016616 | 0.014748 | 0.082286 | 0.074225 |
        10 | 0.016741 | 0.021211 | 0.082153 | 0.085651 |
       100 | 0.016951 | 0.028073 | 0.085792 | 0.102812 |
      1000 | 0.016806 | 0.036941 | 0.082096 | 0.120652 |
     10000 | 0.016732 | 0.050081 | 0.081994 | 0.143143 |
    100000 | 0.017321 | 0.064823 | 0.081415 | 0.230030 |


Bug: 342859647
Change-Id: I835342e88f26ca48006af183290b9a90fb919216
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5573212
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1307864}