Skip to content
This repository has been archived by the owner on Jun 17, 2022. It is now read-only.

Remove hand-coded UniValue destructor. #21

Merged
merged 1 commit into from
Oct 30, 2019
Merged

Remove hand-coded UniValue destructor. #21

merged 1 commit into from
Oct 30, 2019

Conversation

martinus
Copy link

When the hand-written destructor is removed, the compiler will automatically create a proper one, with correct noexcept. This allows std::vector<UniValue> to be resized without having to copy all elements first, which makes JSON generation of a bitcoin block (as in the benchmark "BlockToJsonVerbose") 25% faster on my machine.

When the hand-written destructor is removed, the compiler will automatically create a proper one, with correct `noexcept`. This allows `std::vector<UniValue>` to be resized without having to copy all elements first, which makes JSON generation of a bitcoin block (as in the benchmark "BlockToJsonVerbose") 25% faster on my machine.
@maflcko
Copy link

maflcko commented Oct 30, 2019

ACK. This is described here: https://en.cppreference.com/w/cpp/language/destructor#Implicitly-declared_destructor

Even just for the sake of code clarity, we shouldn't pretend to be smarter than the compiler in writing a destructor.

@maflcko
Copy link

maflcko commented Oct 30, 2019

Unless there are objections, this will be merged by Oct 31st

@laanwj
Copy link
Member

laanwj commented Oct 30, 2019

ACK

maflcko pushed a commit that referenced this pull request Oct 30, 2019
b4cdfc4 Remove hand-coded UniValue destructor. (Martin Ankerl)

Pull request description:

  When the hand-written destructor is removed, the compiler will automatically create a proper one, with correct `noexcept`. This allows `std::vector<UniValue>` to be resized without having to copy all elements first, which makes JSON generation of a bitcoin block (as in the benchmark "BlockToJsonVerbose") 25% faster on my machine.

Top commit has no ACKs.

Tree-SHA512: a4a5a352d946e795b7d78b98b781969caa103acb19770d22d62efd16f42b2845c476dee57df729e983018e065e3b450f3f1e3c95cf68b2f359ee4ca24fba217f
@maflcko maflcko merged commit b4cdfc4 into bitcoin-core:master Oct 30, 2019
@martinus martinus deleted the 2019-10-noexcept-univalue-destructor branch October 31, 2019 10:08
maflcko pushed a commit to bitcoin/bitcoin that referenced this pull request Nov 1, 2019
fa0b3da Squashed 'src/univalue/' changes from 7890db9..5a58a46 (MarcoFalke)

Pull request description:

  Only change is a performance improvement. See bitcoin-core/univalue-subtree#21 (comment) and bitcoin-core/univalue-subtree#15 (comment)

ACKs for top commit:
  jnewbery:
    ACK fa439e8
  laanwj:
    ACK fa439e8

Tree-SHA512: 35ea8f76ea4806182949c8eb5a8b652d1aaeec03ee023838e7cb29abcb81c61d59b38f15708564a78574722df57d13f61ef17d0f670a4056819705ef892321e0
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 1, 2019
fa0b3da Squashed 'src/univalue/' changes from 7890db9..5a58a46 (MarcoFalke)

Pull request description:

  Only change is a performance improvement. See bitcoin-core/univalue-subtree#21 (comment) and bitcoin-core/univalue-subtree#15 (comment)

ACKs for top commit:
  jnewbery:
    ACK fa439e8
  laanwj:
    ACK fa439e8

Tree-SHA512: 35ea8f76ea4806182949c8eb5a8b652d1aaeec03ee023838e7cb29abcb81c61d59b38f15708564a78574722df57d13f61ef17d0f670a4056819705ef892321e0
wqking added a commit to wqking-misc/Phore that referenced this pull request Dec 22, 2019
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 14, 2020
fa0b3da Squashed 'src/univalue/' changes from 7890db9..5a58a46 (MarcoFalke)

Pull request description:

  Only change is a performance improvement. See bitcoin-core/univalue-subtree#21 (comment) and bitcoin-core/univalue-subtree#15 (comment)

ACKs for top commit:
  jnewbery:
    ACK fa439e8
  laanwj:
    ACK fa439e8

Tree-SHA512: 35ea8f76ea4806182949c8eb5a8b652d1aaeec03ee023838e7cb29abcb81c61d59b38f15708564a78574722df57d13f61ef17d0f670a4056819705ef892321e0
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 14, 2020
fa0b3da Squashed 'src/univalue/' changes from 7890db9..5a58a46 (MarcoFalke)

Pull request description:

  Only change is a performance improvement. See bitcoin-core/univalue-subtree#21 (comment) and bitcoin-core/univalue-subtree#15 (comment)

ACKs for top commit:
  jnewbery:
    ACK fa439e8
  laanwj:
    ACK fa439e8

Tree-SHA512: 35ea8f76ea4806182949c8eb5a8b652d1aaeec03ee023838e7cb29abcb81c61d59b38f15708564a78574722df57d13f61ef17d0f670a4056819705ef892321e0
sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 2020
fa0b3da Squashed 'src/univalue/' changes from 7890db9..5a58a46 (MarcoFalke)

Pull request description:

  Only change is a performance improvement. See bitcoin-core/univalue-subtree#21 (comment) and bitcoin-core/univalue-subtree#15 (comment)

ACKs for top commit:
  jnewbery:
    ACK fa439e8
  laanwj:
    ACK fa439e8

Tree-SHA512: 35ea8f76ea4806182949c8eb5a8b652d1aaeec03ee023838e7cb29abcb81c61d59b38f15708564a78574722df57d13f61ef17d0f670a4056819705ef892321e0
malbit pushed a commit to malbit/raptoreum that referenced this pull request Nov 5, 2021
fa0b3da36cd7cf0aada22f2d459296b81274c2f9 Squashed 'src/univalue/' changes from 7890db99d6..5a58a46671 (MarcoFalke)

Pull request description:

  Only change is a performance improvement. See bitcoin-core/univalue-subtree#21 (comment) and bitcoin-core/univalue-subtree#15 (comment)

ACKs for top commit:
  jnewbery:
    ACK fa439e88af944082875e1fdb1cd8bb5a74b88b56
  laanwj:
    ACK fa439e88af944082875e1fdb1cd8bb5a74b88b56

Tree-SHA512: 35ea8f76ea4806182949c8eb5a8b652d1aaeec03ee023838e7cb29abcb81c61d59b38f15708564a78574722df57d13f61ef17d0f670a4056819705ef892321e0
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Feb 5, 2022
fa0b3da Squashed 'src/univalue/' changes from 7890db9..5a58a46 (MarcoFalke)

Pull request description:

  Only change is a performance improvement. See bitcoin-core/univalue-subtree#21 (comment) and bitcoin-core/univalue-subtree#15 (comment)

ACKs for top commit:
  jnewbery:
    ACK fa439e8
  laanwj:
    ACK fa439e8

Tree-SHA512: 35ea8f76ea4806182949c8eb5a8b652d1aaeec03ee023838e7cb29abcb81c61d59b38f15708564a78574722df57d13f61ef17d0f670a4056819705ef892321e0
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants