cluster mempool: cluster linearization algorithm #30126
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
|
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the Possibly this is due to a silent merge conflict (the changes in this pull request being Leave a comment here, if you need help tracking down a confusing failure. |
sipa
force-pushed
the
202405_clusterlin
branch
5 times, most recently
from
079f02d
to
07f68f9
Compare
sipa
force-pushed
the
202405_clusterlin
branch
2 times, most recently
from
079f02d
to
b4bb178
Compare
|
Benchmarks on my Ryzen 5950X system:
This means that for a 64-transaction cluster, it should be possible to linearize (28.59 µs) with 100 candidate search iterations (3.32 µs) plus postlinearize (5.83 µs), within a total of 37.74 µs, on my system. |
theuni
reviewed
May 23, 2024
|
I've dropped the dependency on #29625, and switched to using FastRandomContext instead; there is a measurable slowdown from using the (ChaCha20-based) FastRandomContext over the (xoroshiro128++-based) InsecureRandomContext introduced there, but it's no more than 1-2%. I can switch back to that approach if 29625 were to make it in. |
Merged
Merged
I shall resist. |
|
reACK 448374f via only changes to MakeHardGraph to reflect the intended topology |
|
reACK 448374f via range-diff |
…ypes This primarily adds the DepGraph class, which encapsulates precomputed ancestor/descendant information for a given transaction cluster, with a number of utility features (inspectors for set feerates, computing reduced parents/children, adding transactions, adding dependencies), which will become needed in future commits.
This introduces a bespoke fuzzing-focused serialization format for DepGraphs, and then tests that this format can represent any graph, roundtrips, and then uses that to test the correctness of DepGraph itself. This forms the basis for future fuzz tests that need to work with interesting graphs.
This is a class that encapsulates precomputed ancestor set feerates, and presents an interface for getting the best remaining ancestor set.
Similar to AncestorCandidateFinder, this encapsulates the state needed for finding good candidate sets using a search algorithm.
A fuzz test is added which verifies various of its expected properties, including correctness
This adds a first version of the overall linearization interface, which given a DepGraph constructs a good linearization, by incrementally including good candidate sets (found using AncestorCandidateFinder and SearchCandidateFinder).
Add benchmarks for known bad graphs for the purpose of search (as an upper bound on work per search iterations) and ancestor sorting (as an upper bound on linearization work with no search iterations).
Switch to BFS exploration of the search tree in SearchCandidateFinder instead of DFS exploration. This appears to behave better for real world clusters. As BFS has the downside of needing far larger search queues, switch back to DFS temporarily when the queue grows too large.
To make search non-deterministic, change the BFS logic from always picking the first queue item to randomly picking the first or second queue item.
It encapsulates a given linearization in chunked form, permitting arbitrary subsets of transactions to be removed from the linearization. Its purpose is adding the Intersect function, which is a crucial operation that will be used in a further commit to make Linearize improve existing linearizations.
This implements the LIMO algorithm for linearizing by improving an existing linearization. See https://delvingbitcoin.org/t/limo-combining-the-best-parts-of-linearization-search-and-merging for details.
|
Ok, the mermaid diagrams should be fixed now, match the code, and I've verified (post- #30286) that the claims about difficulty of linearization hold for the code.
I failed. |
|
reACK 647fa37 via I traced through the generated diagrams + code, seems to match. Didn't verify difficulty. |
|
reACK 647fa37, both code and mermaid diagram look correct to me The git diff is always greener |
|
ACK 647fa37 In addition to some manual testing, I ran all the fuzz targets on my machine for a while as well. |
|
Ported to the CMake-based build system in hebasto#290. |
glozow
added a commit
that referenced
this pull request
Aug 5, 2024
…ions bbcee5a clusterlin: improve rechunking in LinearizationChunking (optimization) (Pieter Wuille) 04d7a04 clusterlin: add MergeLinearizations function + fuzz test + benchmark (Pieter Wuille) 4f8958d clusterlin: add PostLinearize + benchmarks + fuzz tests (Pieter Wuille) 0e2812d clusterlin: add algorithms for connectedness/connected components (Pieter Wuille) 0e52728 clusterlin: rename Intersect -> IntersectPrefixes (Pieter Wuille) Pull request description: Part of cluster mempool: #30289 Depends on #30126, and was split off from it. #28676 depends on this. This adds the algorithms for merging & postprocessing linearizations. The `PostLinearize(depgraph, linearization)` function performs an in-place improvement of `linearization`, using two iterations of the [Linearization post-processing](https://delvingbitcoin.org/t/linearization-post-processing-o-n-2-fancy-chunking/201/8) algorithm. The first running from back to front, the second from front to back. The `MergeLinearizations(depgraph, linearization1, linearization2)` function computes a new linearization for the provided cluster, given two existing linearizations for that cluster, which is at least as good as both inputs. The algorithm is described at a high level in [merging incomparable linearizations](https://delvingbitcoin.org/t/merging-incomparable-linearizations/209). For background and references, see [Introduction to cluster linearization](https://delvingbitcoin.org/t/introduction-to-cluster-linearization/1032). ACKs for top commit: sdaftuar: ACK bbcee5a glozow: code review ACK bbcee5a instagibbs: ACK bbcee5a Tree-SHA512: d2b5a3f132d1ef22ddf9c56421ab8b397efe45b3c4c705548dda56f5b39fe4b8f57a0d2a4c65b338462d80bb5b9b84a9a39efa1b4f390420a8005ce31817774e
|
post merge ACK 647fa37 |
glozow
added a commit
that referenced
this pull request
Sep 16, 2024
9ad2fe7 clusterlin: only start/use search when enough iterations left (Pieter Wuille) bd04435 clusterlin: improve heuristic to decide split transaction (optimization) (Pieter Wuille) 71f2629 clusterlin: include topological pot subsets automatically (optimization) (Pieter Wuille) e20fda7 clusterlin: reduce computation of unnecessary pot sets (optimization) (Pieter Wuille) 6060a94 clusterlin bench: add example hard cluster benchmarks (Pieter Wuille) 2965fbf clusterlin: track upper bound potential set for work items (optimization) (Pieter Wuille) 9e43e4c clusterlin: use feerate-sorted depgraph in SearchCandidateFinder (Pieter Wuille) b80e6df clusterlin: add reordering support for DepGraph (Pieter Wuille) 85a285a clusterlin: separate initial search entries per component (optimization) (Pieter Wuille) e4faea9 clusterlin bench: have low/high iter benchmarks instead of per-iter (Pieter Wuille) Pull request description: Part of cluster mempool: #30289 Depends on #30126, and was split off from it. This improves the candidate search algorithm introduced in the previous PR with a variety of optimizations. The resulting search algorithm largely follows Section 2 of [How to linearize your cluster](https://delvingbitcoin.org/t/how-to-linearize-your-cluster/303#h-2-finding-high-feerate-subsets-5), though with a few changes: * Connected component analysis is performed inside the search algorithm (creating initial work items per component for each candidate), rather than once at a higher level. This duplicates some work but is significantly simpler in implementation. * No ancestor-set based presplitting inside the search is performed; instead, the `best` value is initialized with the best topologically valid set known to the LIMO algorithm before search starts: the better one out of the highest-feerate remaining ancestor set, and the highest-feerate prefix of remaining transactions in `old_linearization`. * Work items are represented using an included set *inc* and an undefined set *und*, rather than included and excluded. * Potential sets *pot* are not computed for work items with empty *inc*. At a high level, the only missing optimization from that post is bottleneck analysis; my thinking is that it only really helps with clusters that are already relatively cheap to linearize (doing so would need to be done at a higher level, not inside the search algorithm). --- Overview of the impact of each commit here on linearize performance: * **[clusterlin bench: have low/high iter benchmarks instead of per-iter](https://github.com/bitcoin/bitcoin/pull/30286/commits/21a184db63bd13cf0c2dff9e6e61ca85f2ff4454)**: no impact * **[separate initial search entries per component (optimization)](https://github.com/bitcoin/bitcoin/pull/30286/commits/c84c5c86ba6c12ce11e997d0c59d60694c667488)**: reduce iterations, increase start-up cost * **[add reordering support for DepGraph](https://github.com/bitcoin/bitcoin/pull/30286/commits/019ff2960976b677d91bab47e75be6ed180b37a3)**: no impact * **[use feerate-sorted depgraph in SearchCandidateFinder](https://github.com/bitcoin/bitcoin/pull/30286/commits/8e27dd5a220237da5d362376db6406e47f063776)**: typically reduce iterations, increase start-up cost * **[track upper bound potential set for work items](https://github.com/bitcoin/bitcoin/pull/30286/commits/781e0fb3aa5cbd0983b01547481eefa0804dfb5b)**: reduce iterations, increase cost per iteration * **[reduce computation of unnecessary pot sets](https://github.com/bitcoin/bitcoin/pull/30286/commits/9fe834fa979c8ab038aec57dac2f468ca70c41a9)**: reduce cost per iteration * **[include topological pot subsets automatically](https://github.com/bitcoin/bitcoin/pull/30286/commits/30612710a4e5fc8c950bd1ff4ab9fa843b59132d)**: reduce iterations, increase cost per iteration * **[improve heuristic to decide split transaction](https://github.com/bitcoin/bitcoin/pull/30286/commits/1880c00ab1661dbfc867f69e7556b28bc3cf7a42)**: typically reduce iterations, increase cost per iteration * **[only start/use search when enough iterations left](https://github.com/bitcoin/bitcoin/pull/30286/commits/12760a57b3a6cd1aeb3b7532311f648de2b45aa4)**: just account for start-up cost as equivalent iterations ACKs for top commit: sdaftuar: ACK 9ad2fe7 instagibbs: reACK 9ad2fe7 glozow: reACK 9ad2fe7, just have a question about the docs Tree-SHA512: 108bcbb0676f36071eb83954059b5f3d6646c745015b644a2a5d7f5a8ac9424c2d01d339fa6318a3aff4cf313308e85bb80b0090899720a3fcba027b8025590a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of cluster mempool: #30289
This introduces low-level cluster linearization code, including tests and some benchmarks. It is currently not hooked up to anything.
Ultimately, what this PR adds is a function
Linearizewhich operates on instances ofDepGraph(instances of which represent pre-processed transaction clusters) to produce and/or improve linearizations for that cluster.To provide assurance, the code heavily relies on fuzz tests. A novel approach is used here, where the fuzz input is parsed using the serialization.h framework rather than
FuzzedDataProvider, with a custom serializer/deserializer forDepGraphobjects. By including serialization, it's possible to ascertain that the format can represent every relevant cluster, as well as potentially permitting the construction of ad-hoc fuzz inputs from clusters (not included in this PR, but used during development).The$S$ in every iteration found as the best out of (a) the best remaining ancestor set and (b) randomized computationally-bounded search. It incrementally builds up a linearization by finding good topologically-valid subsets to move to the front, in such a way that the resulting linearization has a diagram that is at least as good as the
Linearize(depgraph, iteration_limit, rng_seed, old_linearization)function is an implementation of the (single) LIMO algorithm, with theold_linearizationpassed in (if any).iteration_limitandrng_seedonly control the (b) randomized search. Even with 0 iterations, the result will be as good as the old linearization, and the included sets at every point will have a feerate at least as high as the best remaining ancestor set at that point.The search algorithm used in the (b) step is very basic, and largely matches Section 2.1 of How to Linearize your Cluster.. See #30286 for optimizations to make it more efficient.
For background and references, see Introduction to cluster linearization.