Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Emplacing wherever possible and other misc enhancements #2498

Merged
merged 4 commits into from
Jan 20, 2020
Merged

Emplacing wherever possible and other misc enhancements #2498

merged 4 commits into from
Jan 20, 2020

Conversation

guilhermelawless
Copy link
Contributor

  • Using emplace/emplace_back where possible. Had to make explicit constructors for [tcp_]endpoint_attempt and channel wrappers
  • Looping with range for-loops and using const refs
  • Avoiding one hash and lookup in ::reachout
  • Using insert/emplace to insert multiple elements in some places rather than a loop + single insert/emplace

@guilhermelawless
Emplacing wherever possible and other misc enhancements
3b7be1e 
- Using emplace/emplace_back where possible. Had to make explicit constructors for [tcp_]endpoint_attempt and channel wrappers
- Looping with range for-loops and using const refs
- Avoiding one hash and lookup in ::reachout
- Using insert/emplace to insert multiple elements in some places, which is known to be faster for many elements
@guilhermelawless guilhermelawless added the quality improvements This item indicates the need for or supplies changes that improve maintainability label Jan 20, 2020
@guilhermelawless guilhermelawless added this to the V21.0 milestone Jan 20, 2020
@guilhermelawless guilhermelawless self-assigned this Jan 20, 2020
@cryptocode
Copy link
Contributor

I know we don't use exceptions much (yet), but I seem to recall there being issues with emplace on smart pointer containers and exception safety.

@guilhermelawless
Copy link
Contributor Author

@cryptocode
The only issue I have been able to find references to is emplacing a new pointer leading to a memory leak if it can't emplace due to lack of memory. In any case I've removed several emplace with smart pointers since they can't be made in-place.

The only location left is https://github.com/nanocurrency/nano-node/pull/2498/files#diff-a5e317b08279542bba01035dfeee7990R216 but what is constructed in-place is the pair

@cryptocode
Copy link
Contributor

cryptocode commented Jan 20, 2020 •
edited
Loading

@guilhermelawless I believe there is an exception safety case. Reading Scott Meyers item 42 now, but seems like it's related to custom deleters, which we've used only in one PR (which wasn't even merged). So probably not an issue, just something to be aware of going forward.

@guilhermelawless guilhermelawless merged commit d5ab41e into nanocurrency:develop Jan 20, 2020
@guilhermelawless guilhermelawless deleted the use-emplace branch January 20, 2020 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cryptocode cryptocode cryptocode approved these changes

@wezrule wezrule Awaiting requested review from wezrule

Assignees

@guilhermelawless guilhermelawless

Labels
quality improvements This item indicates the need for or supplies changes that improve maintainability
Projects
None yet
Milestone
V21.0
Development

Successfully merging this pull request may close these issues.
2 participants
@guilhermelawless @cryptocode