New issue
Advanced search Search tips

Issue 798868 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

blink::WebVector should support the vector construction patterns we need

Project Member Reported by pwnall@chromium.org, Jan 3 2018

Issue description

In a couple of scenarios, I find myself building a WebVector by first constructing a std::vector with the desired data, and then using the WebVector constructor that std::moves a std::vector in.

For example, WebVector's sized constructor cannot be used with types that do not have a default constructor, or for types that don't support moving and have an expensive copy operator. In that case, I've been doing the following dance.

std::vector<WebType> vector;
vector.reserve(final_size);
for (size_t i = 0; i < final_size; ++i)
  vector.emplace_back(...);
WebVector<WebType> web_vector(std::move(vector));

We should add any necessary supporting functions to WebVector so we can construct the vectors we need directly, without going through an std::vector. I will be submitting CLs for the cases I encounter.

So far, this bug covers:
* in-place construction via emplace_back, for types where default construction + assignment is expensive
* types without a default constructor
* resize() for use with std::unique
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2294ef785bb25924601c2ecc83278fbb948f9022

commit 2294ef785bb25924601c2ecc83278fbb948f9022
Author: Victor Costan <pwnall@chromium.org>
Date: Thu Jan 04 04:59:32 2018

Blink: In-place element construction support for WebVector.

Bug:  798868 
Change-Id: Ifca3a9be15fdc396f60b205c830be4cae0541821
Reviewed-on: https://chromium-review.googlesource.com/849474
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526924}
[modify] https://crrev.com/2294ef785bb25924601c2ecc83278fbb948f9022/third_party/WebKit/Source/platform/WebVectorTest.cpp
[modify] https://crrev.com/2294ef785bb25924601c2ecc83278fbb948f9022/third_party/WebKit/public/platform/WebVector.h

Status: Fixed (was: Assigned)
https://crrev.com/c/850576 also addresses this issue. I set the Bug: header on it incorrectly.
Description: Show this description
Status: Started (was: Fixed)
Summary: blink::WebVector should support the vector construction patterns we need (was: blink::WebVector should support in-place element construction)
I repurposed this issue to track all the WebVector construction patterns that I'm using.
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5ffcbbf7625bdfc6cc239f531fc3a6437fbf776a

commit 5ffcbbf7625bdfc6cc239f531fc3a6437fbf776a
Author: Victor Costan <pwnall@chromium.org>
Date: Mon Jan 08 21:37:16 2018

Blink: Add resize() to WebVector.

This makes it possible to use std::sort and std::unique for duplicate removal
directly on WebVector.

Bug:  798868 
Change-Id: Ib5981be5acb1a5224e217def55f93285419044dd
Reviewed-on: https://chromium-review.googlesource.com/852554
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527763}
[modify] https://crrev.com/5ffcbbf7625bdfc6cc239f531fc3a6437fbf776a/third_party/WebKit/Source/platform/WebVectorTest.cpp
[modify] https://crrev.com/5ffcbbf7625bdfc6cc239f531fc3a6437fbf776a/third_party/WebKit/public/platform/WebVector.h

Status: Fixed (was: Started)

Sign in to add a comment