Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Nov 20, 2019

This adds and uses a new transaction type PureTransaction.
The underlying class has no transaction hash compiled in. So
it is safe to use (and faster) wherever it compiles.

The existing CMutableTransaction retains the behavior to calculate the
tx hash on demand. Use of CMutableTransaction makes compile-time
complexity analysis impossible.

The existing CTransaction retains the behavior to cache the hash on
initialization.

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 20, 2019

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #25694 (refactor: Make CTransaction constructor explicit by MarcoFalke)
  • #25665 (BResult improvements, allow returning separate value on failure by ryanofsky)
  • #25232 (rpc: Faster getblock API by AaronDewes)
  • #23599 ([WIP] DRAFT NOMERGE Tidy up RPCTxSerializationFlags by MarcoFalke)

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.

@maflcko
Copy link
Member Author

maflcko commented Nov 20, 2019

So I did some measurements: While this is a clear performance win, it seems the performance is dominated by shuffling the data around multiple times:

Before:
disk -> deserialize -> hash -> serialize -> hex string -> json

After:
disk -> deserialize -> ____ -> serialize -> hex string -> json

Hashing takes 10%-25% on intel CPUs. Didn't try arm.

master:
Screenshot from 2019-11-20 10-47-50

faaa142:

Screenshot from 2019-11-20 11-01-35

@maflcko
Copy link
Member Author

maflcko commented Nov 20, 2019

I guess it would be smarter to see if the rpc serialization flags are matching the ones of the block on disk and then just serve it raw. See #13151

@laanwj
Copy link
Member

laanwj commented Dec 17, 2019

I guess it would be smarter to see if the rpc serialization flags are matching the ones of the block on disk and then just serve it raw. See #13151

If so, it would definitely be a simpler/smaller change!

MarcoFalke added 4 commits July 25, 2022 11:26
Those new types are currently unused. The underlying class has no
transaction hash compiled in. So it is safe to use wherever it compiles.

The existing CMutableTransaction retains the behavior to calculate the
tx hash on demand. Use of CMutableTransaction makes compile-time
complexity analysis impossible.

The existing CTransaction retains the behavior to cache the hash on
initialization.
@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

@achow101
Copy link
Member

Closing this as it has not had any activity in a while. If you are interested in continuing work on this, please leave a comment so that it can be reopened.

@achow101 achow101 closed this Oct 12, 2022
@bitcoin bitcoin deleted a comment from a1love-web Dec 8, 2022
@maflcko maflcko deleted the 1911-PureTx branch December 8, 2022 11:41
@bitcoin bitcoin locked and limited conversation to collaborators Dec 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants