Re: [PATCH v2 00/13] more miscellaneous Bloom filter improvements, redux
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Sep 17, 2020, 13:38 UTC
- Message-ID
- <5caab572-1ef0-4a68-76a1-828a2f69cb6d@gmail.com>
- In-Reply-To
- <20200917133442.GA12879@nand.local>
On 9/17/2020 9:34 AM, Taylor Blau wrote:
Show 28 quoted lines
> Junio, > > On Wed, Sep 16, 2020 at 09:10:49PM -0400, Taylor Blau wrote: >> I should have the patch in your inbox by the end of tonight, depending >> on how fast my workstation can run the ASan-enabled test suite 13 times >> ;). > > All finished. This is sufficient to fix the ASan-enabled test suite, > along with fixing a bug where we wouldn't respect the limit on changed > paths when loading an existing commit-graph. This has nothing to do with > the user-specified '--max-new-filters', nor does it mean that we're > storing the limit in the commit-graph file. Instead it's because we're > loading the bloom_filter_settings struct from the graph and > initializing it ourselves, instead of using the default values (which is > the case when we don't load a graph at all). > > Anyway, let's use this instead of 6/13. Here's an inter-diff that shows > the fix and test change: > > diff --git a/commit-graph.c b/commit-graph.c > index 33af6c2430..fc6c6fdc3e 100644 > --- a/commit-graph.c > +++ b/commit-graph.c > @@ -424,6 +424,7 @@ struct commit_graph *parse_commit_graph(struct repository *r, > graph->bloom_filter_settings->hash_version = hash_version; > graph->bloom_filter_settings->num_hashes = get_be32(data + chunk_offset + 4); > graph->bloom_filter_settings->bits_per_entry = get_be32(data + chunk_offset + 8); > + graph->bloom_filter_settings->max_changed_paths = DEFAULT_BLOOM_MAX_CHANGES;
This whitespace looks strange in the inter-diff...
> graph->bloom_filter_settings->bits_per_entry = get_be32(data + chunk_offset + 8); > + graph->bloom_filter_settings->max_changed_paths = DEFAULT_BLOOM_MAX_CHANGES;
...but it is correct in the patch itself.
Show 7 quoted lines
> - grep "{\"hash_version\":1,\"num_hashes\":9,\"bits_per_entry\":15}" trace2.txt &&
> + grep "{\"hash_version\":1,\"num_hashes\":9,\"bits_per_entry\":15,\"max_changed_paths\":512" trace2.txt &&
> GIT_TRACE2_EVENT="$(pwd)/trace2-auto.txt" \
> GIT_TRACE2_EVENT_NESTING=5 \
> git commit-graph write --reachable --changed-paths &&
> - grep "{\"hash_version\":1,\"num_hashes\":9,\"bits_per_entry\":15}" trace2-auto.txt
> + grep "{\"hash_version\":1,\"num_hashes\":9,\"bits_per_entry\":15,\"max_changed_paths\":512" trace2-auto.txtI appreciate the additional tests to guarantee this is set correctly.
Thanks, -Stolee