Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(111)

Issue 2825853002: Improvements to uses of base::SmallMap (Closed)

Created:
3 years, 8 months ago by brettw
Modified:
3 years, 8 months ago
Reviewers:
danakj, dcheng, estark
CC:
chromium-reviews, Dirk Pranke, feature-media-reviews_chromium.org, kalyank, tfarina, vmpstr+watch_chromium.org, wfh+watch_chromium.org, jam, cbentzel+watch_chromium.org, net-reviews_chromium.org, ozone-reviews_chromium.org, media-router+watch_chromium.org, darin-cc_chromium.org, agrieve+watch_chromium.org, tracing+reviews_chromium.org, piman+watch_chromium.org, danakj+watch_chromium.org, scheduler-bugs_chromium.org, cc-bugs_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Improvements to uses of base::SmallMap Rename base::SmallMap to base::small_map. Add a readme to base/containers to codify naming rules for containers. Convert some uses of SmallMap to flat_map where appropriate. Add an IPC serialization rule for flat_map (required by one of the SmallMap->flat_map conversions) Improve the small_map documentation slightly. CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2825853002 Cr-Commit-Position: refs/heads/master@{#465911} Committed: https://chromium.googlesource.com/chromium/src/+/ca2ec763405c18960ad1ee66e5ac964a00c257e5

Patch Set 1 #

Total comments: 19

Patch Set 2 : Code review #

Total comments: 1

Patch Set 3 : Checkpoint #

Patch Set 4 : IPC #

Patch Set 5 : Fixes #

Patch Set 6 : Unit tests #

Total comments: 5

Patch Set 7 : Fixes #

Total comments: 4

Patch Set 8 : Review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+208 lines, -133 lines) Patch
A base/containers/README.md View 1 1 chunk +20 lines, -0 lines 0 comments Download
M base/containers/small_map.h View 1 2 3 4 5 6 10 chunks +39 lines, -44 lines 0 comments Download
M base/containers/small_map_unittest.cc View 20 chunks +47 lines, -46 lines 0 comments Download
M base/trace_event/trace_event_memory_overhead.h View 1 chunk +1 line, -1 line 0 comments Download
M base/trace_event/trace_event_memory_overhead.cc View 1 chunk +1 line, -1 line 0 comments Download
M cc/resources/resource_provider.h View 1 chunk +1 line, -1 line 0 comments Download
M cc/scheduler/begin_frame_source.h View 1 1 chunk +1 line, -1 line 0 comments Download
M cc/scheduler/begin_frame_source.cc View 1 1 chunk +0 lines, -2 lines 0 comments Download
M cc/tiles/picture_layer_tiling.h View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M cc/tiles/picture_layer_tiling.cc View 1 2 chunks +3 lines, -7 lines 0 comments Download
M cc/trees/layer_tree_host_impl.cc View 1 2 3 4 5 6 7 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/media/router/presentation_service_delegate_impl.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -2 lines 0 comments Download
M content/browser/renderer_host/input/input_router_impl.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M content/zygote/zygote_linux.h View 1 2 3 4 5 6 1 chunk +3 lines, -4 lines 0 comments Download
M gpu/perftests/texture_upload_perftest.cc View 1 2 chunks +2 lines, -3 lines 0 comments Download
M ipc/ipc_message_utils.h View 1 2 3 4 5 6 7 3 chunks +53 lines, -5 lines 0 comments Download
M ipc/ipc_message_utils_unittest.cc View 1 2 3 4 5 1 chunk +20 lines, -0 lines 0 comments Download
M net/quic/platform/impl/quic_containers_impl.h View 1 chunk +1 line, -1 line 0 comments Download
M storage/browser/blob/blob_memory_controller.cc View 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/value.h View 1 chunk +1 line, -1 line 0 comments Download
M ui/latency/latency_info.h View 1 2 2 chunks +3 lines, -7 lines 0 comments Download
M ui/ozone/platform/drm/common/drm_util.cc View 1 2 3 4 5 2 chunks +2 lines, -3 lines 0 comments Download

Messages

Total messages: 56 (34 generated)
brettw
I made the small_map comments slightly less wrong but they're still not great. I'm working ...
3 years, 8 months ago (2017-04-18 20:27:06 UTC) #6
danakj
I don't believe most of these should be small_map anyhow. I left some comments. LGTM ...
3 years, 8 months ago (2017-04-18 21:35:51 UTC) #7
brettw
Checkpoint
3 years, 8 months ago (2017-04-18 23:21:07 UTC) #10
brettw
IPC
3 years, 8 months ago (2017-04-19 00:12:31 UTC) #11
brettw
Fixes
3 years, 8 months ago (2017-04-19 16:18:49 UTC) #16
brettw
Unit tests
3 years, 8 months ago (2017-04-19 16:28:37 UTC) #19
brettw
This needs a re-review. I had to write some comparators and an IPC serializer for ...
3 years, 8 months ago (2017-04-19 16:29:49 UTC) #23
danakj
LGTM can you update the CL desc to explain about the conversions to flat_map (or ...
3 years, 8 months ago (2017-04-19 17:32:17 UTC) #26
brettw
https://codereview.chromium.org/2825853002/diff/100001/ipc/ipc_message_utils.h File ipc/ipc_message_utils.h (right): https://codereview.chromium.org/2825853002/diff/100001/ipc/ipc_message_utils.h#newcode948 ipc/ipc_message_utils.h:948: GetParamSize(sizer, static_cast<int>(p.size())); On 2017/04/19 17:32:16, danakj wrote: > checked_cast ...
3 years, 8 months ago (2017-04-19 17:55:04 UTC) #28
brettw
Fixes
3 years, 8 months ago (2017-04-19 17:55:34 UTC) #29
estark
I'm going to take a look at this shortly, but since I'm new to IPC ...
3 years, 8 months ago (2017-04-19 23:25:39 UTC) #35
estark
lgtm but please wait for dcheng to tell me all the stuff I missed :) ...
3 years, 8 months ago (2017-04-20 00:59:49 UTC) #36
dcheng
LGTM https://codereview.chromium.org/2825853002/diff/120001/base/containers/small_map.h File base/containers/small_map.h (right): https://codereview.chromium.org/2825853002/diff/120001/base/containers/small_map.h#newcode43 base/containers/small_map.h:43: // generate more code than std::map (at least ...
3 years, 8 months ago (2017-04-20 01:30:47 UTC) #37
dcheng
On 2017/04/20 00:59:49, estark wrote: > lgtm but please wait for dcheng to tell me ...
3 years, 8 months ago (2017-04-20 01:33:51 UTC) #38
brettw
https://codereview.chromium.org/2825853002/diff/120001/ipc/ipc_message_utils.h File ipc/ipc_message_utils.h (right): https://codereview.chromium.org/2825853002/diff/120001/ipc/ipc_message_utils.h#newcode974 ipc/ipc_message_utils.h:974: vect.resize(size); On 2017/04/20 01:30:47, dcheng (OOO through May 2) ...
3 years, 8 months ago (2017-04-20 04:24:24 UTC) #39
brettw
Review comments
3 years, 8 months ago (2017-04-20 04:24:55 UTC) #40
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2825853002/140001
3 years, 8 months ago (2017-04-20 04:25:33 UTC) #43
brettw
Review comments
3 years, 8 months ago (2017-04-20 04:26:11 UTC) #44
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2825853002/160001
3 years, 8 months ago (2017-04-20 04:26:38 UTC) #47
brettw
Review comments
3 years, 8 months ago (2017-04-20 04:28:07 UTC) #48
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2825853002/140001
3 years, 8 months ago (2017-04-20 04:29:40 UTC) #53
commit-bot: I haz the power
3 years, 8 months ago (2017-04-20 06:10:40 UTC) #56
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/ca2ec763405c18960ad1ee66e5ac...

Powered by Google App Engine
This is Rietveld 408576698