Skip to content

WIP: Benchmark TxGraph queries#1735

Closed
ValuedMammal wants to merge 3 commits intobitcoindevkit:masterfrom
ValuedMammal:bench/tx-graph
Closed

WIP: Benchmark TxGraph queries#1735
ValuedMammal wants to merge 3 commits intobitcoindevkit:masterfrom
ValuedMammal:bench/tx-graph

Conversation

@ValuedMammal
Copy link
Copy Markdown
Collaborator

@ValuedMammal ValuedMammal commented Nov 19, 2024

Add bench directory to the workspace and begin benchmarking performance of tx-graph queries. I'm currently working on creating benches for these scenarios

  • Finding the canonical set from a large number of tx conflicts
  • Same as above, but further complicated by "nesting" conflicts
  • Conflicts with potentially many descendants

cc #1687

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@ValuedMammal
chore: add crate bench to workspace
0646980 
chain: add optional dep `criterion`. Ideally this should be
conditional on "--cfg=bdk_bench". See LDK for
example
@ValuedMammal
chore: gate criterion benches behind cfg(bdk_bench)
27a0c9b 
ci: bump nightly docs toolchain to 2024-11-17
@ValuedMammal ValuedMammal self-assigned this Nov 20, 2024
@notmandatory notmandatory added this to the 1.0.0-beta milestone Nov 21, 2024
@evanlinjin
Copy link
Copy Markdown
Member

evanlinjin commented Nov 23, 2024

Why do we need a separate crate for benchmarks?

@evanlinjin evanlinjin marked this pull request as draft November 28, 2024 10:50
@ValuedMammal
Copy link
Copy Markdown
Collaborator Author

ValuedMammal commented Dec 1, 2024

Quick status update: I used the bench tests as of a650408 to produce some initial performance numbers, see #1687 (comment). I ran the same tests on #1670 and the result was such a vast improvement that the current benches in their basic form may not even be very useful post 1.0 but it gives us a starting point. @evanlinjin has also offered to continue work on this PR. Overall it's good to know that ongoing development is backed up by the data.

I also had a question about whether it would be worth having a CI job for benchmarks and the process by which to monitor changes in performance, thoughts?

Why do we need a separate crate for benchmarks?

In theory you could have several bench tests from different crates run together as a single criterion_group, but maybe not necessary for what we need at present. Also it seems that making it a dev-dependency could add to compile times even for normal non-benching development?

evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 3, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
482c222 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 3, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
4b39650 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 3, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
0a13820 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 3, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
999edc9 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 3, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
ba3ec15 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 3, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
6f1f8c1 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

CI and README is updated to pin `csv`.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 3, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
ee94242 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

CI and README is updated to pin `csv`.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 3, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
169e51e 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

CI and README is updated to pin `csv`.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 3, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
6151ae8 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

CI and README is updated to pin `csv`.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 3, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
d107e4b 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

CI and README is updated to pin `csv`.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
@evanlinjin
Copy link
Copy Markdown
Member

evanlinjin commented Dec 3, 2024

Replaced by #1670

@evanlinjin evanlinjin closed this Dec 3, 2024
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 6, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
6cb50e8 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

CI and README is updated to pin `csv`.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 6, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
238e61b 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

CI and README is updated to pin `csv`.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
@notmandatory notmandatory removed this from the 1.0.0-beta milestone Dec 6, 2024
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 6, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
762ce63 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

CI and README is updated to pin `csv`.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 6, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
6a6cbe7 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

CI and README is updated to pin `csv`.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 6, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
87267a4 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

CI and README is updated to pin `csv`.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 6, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
07622d2 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

CI and README is updated to pin `csv`.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 10, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
b2a32d8 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

CI and README is updated to pin `csv`.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
evanlinjin added a commit to evanlinjin/bdk that referenced this pull request Dec 10, 2024
@evanlinjin @ValuedMammal
perf(chain): add benchmarks for canonicalization logic
d4102b4 
This is mostly taken from bitcoindevkit#1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.

CI and README is updated to pin `csv`.

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
@ValuedMammal ValuedMammal mentioned this pull request Dec 16, 2024
Merged
@ValuedMammal ValuedMammal deleted the bench/tx-graph branch December 20, 2024 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evanlinjin evanlinjin Awaiting requested review from evanlinjin evanlinjin will be requested when the pull request is marked ready for review evanlinjin is a code owner

@oleonardolima oleonardolima Awaiting requested review from oleonardolima oleonardolima will be requested when the pull request is marked ready for review oleonardolima is a code owner

Assignees

@evanlinjin evanlinjin

@ValuedMammal ValuedMammal

Labels

module-blockchain tests

Projects

BDK Wallet
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ValuedMammal @evanlinjin @notmandatory