Use emplace reserve and std::move to optimize [chromium/src : main]
Daniel Cheng (Gerrit)
Daniel Cheng added 4 comments![]( "Open in Gerrit")
Daniel ChengThere are other similar places. Can they be modified together?
1. I think this sort of change is more valuable in conjunction with data that it matters, or when it's a very straightforward transformation. For example, emplace_back(value) or push_back(value) to emplace_back(std::move(value)) or push_back(std::move(value)) is a simple transform to review. Something like "add a reserve()" call is less obviously a benefit.
2. Things like [Abseil's TotW #112](https://abseil.io/tips/112) mean that migrating to use emplace or emplace_back aren't necessarily a straight up win either.
3. I would suggest focusing on other changes for now instead of making more changes like this; there are better ways to use reviewer time.
auto [it, inserted] = trial_groups.emplace(trial, group);try_emplace() would be better most likely.
static_cast<std::size_t>(debug::CrashKeySize::Size256));I'd probably leave this one; we aren't necessarily going to use up to Size256 bytes anyway.
event_filters_.emplace_back(*predicate_name);
event_filters_.back().InitializeFromConfigDict(event_filter_dict);I believe emplace_back returns a reference now so this can just be:
```suggestion
event_filters_.emplace_back(*predicate_name).InitializeFromConfigDict(
event_filter_dict);
```
Related details
- Lei Zhang
- gate kibr
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gate kibr (Gerrit)
gate kibr added 4 comments![]( "Open in Gerrit")
Daniel ChengThere are other similar places. Can they be modified together?
1. I think this sort of change is more valuable in conjunction with data that it matters, or when it's a very straightforward transformation. For example, emplace_back(value) or push_back(value) to emplace_back(std::move(value)) or push_back(std::move(value)) is a simple transform to review. Something like "add a reserve()" call is less obviously a benefit.
2. Things like [Abseil's TotW #112](https://abseil.io/tips/112) mean that migrating to use emplace or emplace_back aren't necessarily a straight up win either.
3. I would suggest focusing on other changes for now instead of making more changes like this; there are better ways to use reviewer time.
ok
auto [it, inserted] = trial_groups.emplace(trial, group);try_emplace() would be better most likely.
trial_groups use std::set
static_cast<std::size_t>(debug::CrashKeySize::Size256));I'd probably leave this one; we aren't necessarily going to use up to Size256 bytes anyway.
Done
event_filters_.emplace_back(*predicate_name);
event_filters_.back().InitializeFromConfigDict(event_filter_dict);I believe emplace_back returns a reference now so this can just be:
```suggestion
event_filters_.emplace_back(*predicate_name).InitializeFromConfigDict(
event_filter_dict);
```
- Daniel Cheng
- Lei Zhang
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |