Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2801603010:
Convert cc::PropertyTree to use a flat_map. (Closed)
DescriptionConvert cc::PropertyTree to use a flat_map.
These maps are typically very small (see bug) and the unordered_map has a large memory overhead (both in terms of space and # of allocations).
BUG=709243
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2801603010
Cr-Commit-Position: refs/heads/master@{#463524}
Committed: https://chromium.googlesource.com/chromium/src/+/fb21662ee5c04e2bfa9de9e4b9a86e5f101c524e
Patch Set 1 #
Messages
Total messages: 20 (12 generated)
Description was changed from ========== Convert cc::PropertyTree to use a flat_map. These maps are typically very small (see bug) and the unordered_map has a large memory overhead (both in terms of space and # of allocations). BUG=709243 ========== to ========== Convert cc::PropertyTree to use a flat_map. These maps are typically very small (see bug) and the unordered_map has a large memory overhead (both in terms of space and # of allocations). BUG=709243 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
Description was changed from ========== Convert cc::PropertyTree to use a flat_map. These maps are typically very small (see bug) and the unordered_map has a large memory overhead (both in terms of space and # of allocations). BUG=709243 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Convert cc::PropertyTree to use a flat_map. These maps are typically very small (see bug) and the unordered_map has a large memory overhead (both in terms of space and # of allocations). BUG=709243 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
brettw@chromium.org changed reviewers: + weiliangc@chromium.org
The CQ bit was checked by brettw@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Hi, I don't know too many details of the load of these property trees other than my profiling run. If there are cases where they get up to hundreds or thousands of items, I would want to benchmark those.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM This map is generally assumed to be small and should be removed as part of SPv2. cc'ing more people working on SPv2 to check if there are some cases we need to worry about this map being big.
The CQ bit was checked by brettw@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by brettw@chromium.org
Thanks, I'll submit this evening if I don't hear any feedback to the contrary.
On 2017/04/10 20:45:56, brettw (plz ping after 24h) wrote: > Thanks, I'll submit this evening if I don't hear any feedback to the contrary. It should be small. Looking at the code, the only time it can become greater than size of the corresponding property tree is when a layer that wasn't in the map changes its host. In this case, we add (layer_id, invalid_node_id). Should we remove that to ensure its always equal to size of the corresponding property tree ?
On 2017/04/10 21:08:11, jaydasika wrote: > On 2017/04/10 20:45:56, brettw (plz ping after 24h) wrote: > > Thanks, I'll submit this evening if I don't hear any feedback to the contrary. > > It should be small. Looking at the code, the only time it can become greater > than size of the corresponding property tree is when a layer that wasn't in the > map changes its host. In this case, we add (layer_id, invalid_node_id). Should > we remove that to ensure its always equal to size of the corresponding property > tree ? You can do that but it should be independent of this change: flat_map is fine either way unless your're making many hundreds of them.
The CQ bit was checked by brettw@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1491882359536530, "parent_rev":
"66b7d1bcac5819a601d529ec6662c4d210711a7e", "commit_rev":
"fb21662ee5c04e2bfa9de9e4b9a86e5f101c524e"}
Message was sent while issue was closed.
Description was changed from ========== Convert cc::PropertyTree to use a flat_map. These maps are typically very small (see bug) and the unordered_map has a large memory overhead (both in terms of space and # of allocations). BUG=709243 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Convert cc::PropertyTree to use a flat_map. These maps are typically very small (see bug) and the unordered_map has a large memory overhead (both in terms of space and # of allocations). BUG=709243 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2801603010 Cr-Commit-Position: refs/heads/master@{#463524} Committed: https://chromium.googlesource.com/chromium/src/+/fb21662ee5c04e2bfa9de9e4b9a8... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/fb21662ee5c04e2bfa9de9e4b9a8... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
