MarcoFalke
commented at 12:40 PM on July 30, 2022:
member
Merge will create several copies unconditionally:
To initialize the args a, and b
ret, which is the merge of the two args
So change the code to let the caller decide how many copies they need/want:
a, and b must be explicitly moved or copied by the caller
ret is no longer needed, as a can be used for it in place "for free"
fanquake added the label Refactoring on Jul 30, 2022
DrahtBot
commented at 2:01 PM on July 30, 2022:
contributor
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Conflicts
Reviewers, this pull request conflicts with the following ones:
#24149 (Signing support for Miniscript Descriptors by darosior)
#22838 (descriptors: Be able to specify change and receiving in a single descriptor string by achow101)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
achow101
commented at 3:53 PM on August 4, 2022:
member
Related is #16116. Could you also pull in the benchmark from that?
achow101
commented at 6:58 PM on August 8, 2022:
member
ACKfa00d8f84dca8ad9c5a3580e0f7a027f472cdc41
achow101
commented at 6:59 PM on August 8, 2022:
member
Silent merge conflict with master
DrahtBot added the label Needs rebase on Aug 11, 2022
MarcoFalke referenced this in commit bf3f05f41d on Aug 12, 2022
MarcoFalke force-pushed on Aug 12, 2022
refactor: Avoid copies in FlatSigningProvider Mergefa3f15f2dd
MarcoFalke force-pushed on Aug 12, 2022
DrahtBot removed the label Needs rebase on Aug 12, 2022
MarcoFalke
commented at 3:54 PM on August 12, 2022:
member
Rebased and used @Empact's idea to transform this into a member function from https://github.com/bitcoin/bitcoin/pull/16116/files (thanks), which is a lot faster than an elided copy and std::move(). I've kept the argument as r-ref to allow stealing the pointers, which again should be faster than copying the entries individually.
furszy approved
furszy
commented at 4:00 PM on August 12, 2022:
member
looks good, ACKfa3f15f2
achow101
commented at 4:03 PM on August 12, 2022:
member
ACKfa3f15f2dd94ae597a66037f5928fe4e90fe099d
sidhujag referenced this in commit f35b6d28c6 on Aug 12, 2022
ryanofsky approved
ryanofsky
commented at 8:40 PM on August 17, 2022:
contributor
Code review ACKfa3f15f2dd94ae597a66037f5928fe4e90fe099d. Confirmed that all the places std::move was added the argument actually did seem safe to move from. Compiler enforces that temporary copies are explicitly created in non-move cases.
achow101 merged this on Aug 17, 2022
achow101 closed this on Aug 17, 2022
sidhujag referenced this in commit 8e42a8dda3 on Aug 18, 2022
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2026-04-17 06:13 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me