Reserve space in the resource ID set.

Reserve space in the resource ID set.

Roughly every resource transferred should be used, so reserve that much space in the set. This improves performance on the SurfaceAggregator perftests by around 10%.

Committed: https://crrev.com/bf22ae4e24ec9fd558c868ab6720f53138f95cb4
Cr-Commit-Position: refs/heads/master@{#327394}

[jbauman, 2015-04-25 02:07:45]

jbauman@chromium.org changed reviewers:
+ danakj@chromium.org

[danakj, 2015-04-25 17:16:44]

https://codereview.chromium.org/1100713004/diff/40001/cc/surfaces/surface_agg...
File cc/surfaces/surface_aggregator.cc (right):

https://codereview.chromium.org/1100713004/diff/40001/cc/surfaces/surface_agg...
cc/surfaces/surface_aggregator.cc:174: std::max(static_cast<size_t>(1),
frame_data->resource_list.size());
why the Max 1?

https://codereview.chromium.org/1100713004/diff/40001/cc/surfaces/surface_agg...
cc/surfaces/surface_aggregator.cc:178: // Pre-standard hash-tables only
implement resize, which behaves similarly
wow really? I'm surprised this is all we have in our toolchain. leave a TODO to
fix this with c++11?

can we do similar in DRLI too? that's affect children who don't use surfaces yet
(pdf viewer seems to be one eg)

[jbauman, 2015-04-28 02:44:58]

PTAL

On 2015/04/25 17:16:44, danakj wrote:
>
https://codereview.chromium.org/1100713004/diff/40001/cc/surfaces/surface_agg...
> File cc/surfaces/surface_aggregator.cc (right):
> 
>
https://codereview.chromium.org/1100713004/diff/40001/cc/surfaces/surface_agg...
> cc/surfaces/surface_aggregator.cc:174: std::max(static_cast<size_t>(1),
> frame_data->resource_list.size());
> why the Max 1?

resizing to 0 causes stlport to explode.
> 
>
https://codereview.chromium.org/1100713004/diff/40001/cc/surfaces/surface_agg...
> cc/surfaces/surface_aggregator.cc:178: // Pre-standard hash-tables only
> implement resize, which behaves similarly
> wow really? I'm surprised this is all we have in our toolchain. leave a TODO
to
> fix this with c++11?

Ok, done.
> 
> can we do similar in DRLI too? that's affect children who don't use surfaces
yet
> (pdf viewer seems to be one eg)

done.

[danakj, 2015-04-28 17:23:08]

On Mon, Apr 27, 2015 at 7:44 PM, <jbauman@chromium.org> wrote:

> PTAL
>
> On 2015/04/25 17:16:44, danakj wrote:
>
>
>
https://codereview.chromium.org/1100713004/diff/40001/cc/surfaces/surface_agg...
>
>> File cc/surfaces/surface_aggregator.cc (right):
>>
>
>
>
>
https://codereview.chromium.org/1100713004/diff/40001/cc/surfaces/surface_agg...
>
>> cc/surfaces/surface_aggregator.cc:174: std::max(static_cast<size_t>(1),
>> frame_data->resource_list.size());
>> why the Max 1?
>>
>
> resizing to 0 causes stlport to explode.


Can you TODO about this and change it after we lose stlport? Magic 1 is
magic.

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2015-04-28 17:25:46]

https://codereview.chromium.org/1100713004/diff/60001/cc/layers/delegated_ren...
File cc/layers/delegated_renderer_layer_impl.cc (right):

https://codereview.chromium.org/1100713004/diff/60001/cc/layers/delegated_ren...
cc/layers/delegated_renderer_layer_impl.cc:127: std::max(static_cast<size_t>(1),
frame_data->resource_list.size());
actually instead of max(1,..) can you just if(reserve_size) down at the resize()
call?

https://codereview.chromium.org/1100713004/diff/60001/cc/layers/delegated_ren...
cc/layers/delegated_renderer_layer_impl.cc:132: // to reserve for these keys.
TODO: Fix when C++11 is supported everywhere.
TODO needs a name

https://codereview.chromium.org/1100713004/diff/60001/cc/surfaces/surface_agg...
File cc/surfaces/surface_aggregator.cc (right):

https://codereview.chromium.org/1100713004/diff/60001/cc/surfaces/surface_agg...
cc/surfaces/surface_aggregator.cc:173: // Resizing to 0 breaks stlport hash_set.
TODO here pls

https://codereview.chromium.org/1100713004/diff/60001/cc/surfaces/surface_agg...
cc/surfaces/surface_aggregator.cc:180: // to reserve for these keys. TODO: Fix
when C++11 is supported everywhere.
TODO needs a name and I think should start the line.

[jbauman, 2015-04-28 21:59:36]

Ok, PTAL.

[danakj, 2015-04-28 22:00:01]

Thanks! LGTM

[jbauman, 2015-04-28 22:00:26]

The CQ bit was checked by jbauman@chromium.org

[commit-bot: I haz the power, 2015-04-28 22:02:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1100713004/80001

[commit-bot: I haz the power, 2015-04-28 23:56:59]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-04-28 23:57:51]

Patchset 5 (id:??) landed as
https://crrev.com/bf22ae4e24ec9fd558c868ab6720f53138f95cb4
Cr-Commit-Position: refs/heads/master@{#327394}