git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 06/12] bloom: use num_changes not nr for limit detection

From
SZEDER Gábor <szeder.dev@gmail.com>
Date
Aug 4, 2020, 14:51 UTC
Message-ID
<20200804145144.GQ2898@szeder.dev>
In-Reply-To
<8278b5c0918c6b357e1f1ab3ee65ed2091ec3f61.1589198180.git.gitgitgadget@gmail.com>
On Mon, May 11, 2020 at 11:56:13AM +0000, Derrick Stolee via GitGitGadget wrote:
Show 38 quoted lines
> From: Derrick Stolee <dstolee@microsoft.com>
> 
> As diff_tree_oid() computes a diff, it will terminate early if the
> total number of changed paths is strictly larger than max_changes.
> This includes the directories that changed, not just the file paths.
> However, only the file paths are reflected in the resulting diff
> queue's "nr" value.
> 
> Use the "num_changes" from diffopt to check if the diff terminated
> early. This is incredibly important, as it can result in incorrect
> filters! For example, the first commit in the Linux kernel repo
> reports only 471 changes, but since these are nested inside several
> directories they expand to 513 "real" changes, and in fact the
> total list of changes is not reported. Thus, the computed filter
> for this commit is incorrect.
> 
> Demonstrate the subtle difference by using one fewer file change
> in the 'get bloom filter for commit with 513 changes' test. Before,
> this edited 513 files inside "bigDir" which hit this inequality.
> However, dropping the file count by one demonstrates how the
> previous inequality was incorrect but the new one is correct.
> 
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  bloom.c          | 2 +-
>  t/t0095-bloom.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/bloom.c b/bloom.c
> index 196cda0a1bd..e2ede44126c 100644
> --- a/bloom.c
> +++ b/bloom.c
> @@ -215,7 +215,7 @@ struct bloom_filter *get_bloom_filter(struct repository *r,
>  		diff_tree_oid(NULL, &c->object.oid, "", &diffopt);
>  	diffcore_std(&diffopt);
>  
> -	if (diff_queued_diff.nr <= max_changes) {
> +	if (diffopt.num_changes <= max_changes) {
The field 'diffopt.num_changes' is marked with:
  /* For internal use only. */
Show 19 quoted lines
>  		struct hashmap pathmap;
>  		struct pathmap_hash_entry *e;
>  		struct hashmap_iter iter;
> diff --git a/t/t0095-bloom.sh b/t/t0095-bloom.sh
> index 6defeb544f1..48a90625596 100755
> --- a/t/t0095-bloom.sh
> +++ b/t/t0095-bloom.sh
> @@ -100,7 +100,7 @@ test_expect_success EXPENSIVE 'get bloom filter for commit with 513 changes' '
>  	rm actual &&
>  	rm expect &&
>  	mkdir bigDir &&
> -	for i in $(test_seq 0 512)
> +	for i in $(test_seq 0 511)
>  	do
>  		echo $i >bigDir/$i
>  	done &&
> -- 
> gitgitgadget
> 
Previous: Derrick Stolee via GitGitGadgetNext: Derrick Stolee via GitGitGadget
Message 40 of 42 in “Integrating line-log and changed-path Bloom filters”
  1. 00/12 Integrating line-log and changed-path Bloom filtersDerrick Stolee via GitGitGadget, May 1, 2020
  2. 01/12 bloom: fix whitespace around tab lengthDerrick Stolee via GitGitGadget, May 1, 2020
  3. Taylor BlauMay 1, 2020
  4. 03/12 Documentation: changed-path Bloom filters use byte wordsDerrick Stolee via GitGitGadget, May 1, 2020
  5. Taylor BlauMay 1, 2020
  6. 02/12 test-bloom: fix usage typoDerrick Stolee via GitGitGadget, May 1, 2020
  7. Taylor BlauMay 1, 2020
  8. 04/12 bloom: de-duplicate directory entriesDerrick Stolee via GitGitGadget, May 1, 2020
  9. Taylor BlauMay 1, 2020
  10. 05/12 bloom: parse commit before computing filtersDerrick Stolee via GitGitGadget, May 1, 2020
  11. Taylor BlauMay 1, 2020
  12. 07/12 completion: offer '--(no-)patch' among 'git log' optionsSZEDER Gábor via GitGitGadget, May 1, 2020
  13. Taylor BlauMay 1, 2020
  14. 08/12 line-log: remove unused fields from 'struct line_log_data'SZEDER Gábor via GitGitGadget, May 1, 2020
  15. Taylor BlauMay 1, 2020
  16. 06/12 bloom: use num_changes not nr for limit detectionDerrick Stolee via GitGitGadget, May 1, 2020
  17. Taylor BlauMay 1, 2020
  18. 09/12 t4211-line-log: add tests for parent oidsSZEDER Gábor via GitGitGadget, May 1, 2020
  19. Taylor BlauMay 4, 2020
  20. 11/12 line-log: try to use generation number-based topo-orderingSZEDER Gábor via GitGitGadget, May 1, 2020
  21. Taylor BlauMay 4, 2020
  22. 10/12 line-log: more responsive, incremental 'git log -L'SZEDER Gábor via GitGitGadget, May 1, 2020
  23. Taylor BlauMay 4, 2020
  24. Derrick StoleeMay 4, 2020
  25. 12/12 line-log: integrate with changed-path Bloom filtersDerrick Stolee via GitGitGadget, May 1, 2020
  26. Taylor BlauMay 4, 2020
  27. Junio C HamanoMay 1, 2020
  28. 00/12 Integrating line-log and changed-path Bloom filtersDerrick Stolee via GitGitGadget, May 11, 2020
  29. 02/12 test-bloom: fix usage typoDerrick Stolee via GitGitGadget, May 11, 2020
  30. 03/12 bloom: parse commit before computing filtersDerrick Stolee via GitGitGadget, May 11, 2020
  31. 05/12 bloom: de-duplicate directory entriesDerrick Stolee via GitGitGadget, May 11, 2020
  32. SZEDER GáborJun 7, 2020
  33. 07/12 completion: offer '--(no-)patch' among 'git log' optionsSZEDER Gábor via GitGitGadget, May 11, 2020
  34. 08/12 line-log: remove unused fields from 'struct line_log_data'SZEDER Gábor via GitGitGadget, May 11, 2020
  35. 09/12 t4211-line-log: add tests for parent oidsSZEDER Gábor via GitGitGadget, May 11, 2020
  36. 10/12 line-log: more responsive, incremental 'git log -L'SZEDER Gábor via GitGitGadget, May 11, 2020
  37. 12/12 line-log: integrate with changed-path Bloom filtersDerrick Stolee via GitGitGadget, May 11, 2020
  38. 11/12 line-log: try to use generation number-based topo-orderingSZEDER Gábor via GitGitGadget, May 11, 2020
  39. 06/12 bloom: use num_changes not nr for limit detectionDerrick Stolee via GitGitGadget, May 11, 2020
  40. SZEDER GáborAug 4, 2020
  41. 01/12 bloom: fix whitespace around tab lengthDerrick Stolee via GitGitGadget, May 11, 2020
  42. 04/12 Documentation: changed-path Bloom filters use byte wordsDerrick Stolee via GitGitGadget, May 11, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.