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

Re: [PATCH 1/3] commit-reach: implement get_reachable_subset

From
Derrick Stolee <stolee@gmail.com>
Date
Oct 31, 2018, 12:01 UTC
Message-ID
<15ef2018-4bb1-430f-32fd-5676a1b5ac1a@gmail.com>
In-Reply-To
<xmqqy3aeyehs.fsf@gitster-ct.c.googlers.com>
On 10/30/2018 11:35 PM, Junio C Hamano wrote:
Show 7 quoted lines
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> +struct commit_list *get_reachable_subset(struct commit **from, int nr_from,
>> +					 struct commit **to, int nr_to,
>> +					 int reachable_flag)
> This is OR'ed into object.flags, and I somoehow expected to see it
> as 'unsigned int', not a signed one.
Will do. Thanks.
Show 41 quoted lines
>
>> +{
>> +	struct commit **item;
>> +	struct commit *current;
>> +	struct commit_list *found_commits = NULL;
>> +	struct commit **to_last = to + nr_to;
>> +	struct commit **from_last = from + nr_from;
>> +	uint32_t min_generation = GENERATION_NUMBER_INFINITY;
>> +	int num_to_find = 0;
>> +
>> +	struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
>> +
>> +	for (item = to; item < to_last; item++) {
>> +		struct commit *c = *item;
>> +		
>> +		parse_commit(c);
>> +		if (c->generation < min_generation)
>> +			min_generation = c->generation;
>> +
>> +		if (!(c->object.flags & PARENT1)) {
>> +			c->object.flags |= PARENT1;
>> +			num_to_find++;
>> +		}
>> +	}
>> +
>> +	for (item = from; item < from_last; item++) {
>> +		struct commit *c = *item;
>> +		if (!(c->object.flags & PARENT2)) {
>> +			c->object.flags |= PARENT2;
>> +			parse_commit(c);
>> +
>> +			prio_queue_put(&queue, *item);
>> +		}
>> +	}
> OK, we marked "to" with PARENT1 and counted them in num_to_find
> without dups.  We also marked "from" with PARENT2 and threw them in
> the "queue" without dups.
>
> Mental note: the caller must guarantee that everybody reachable from
> "to" and "from" have PARENT1 and PARENT2 clear.  This might deserve
> to be in the comment before the function.
I'll put that in the header file.
[snip]
Show 8 quoted lines
> OK, this all makes sense.  Unlike merge-base traversals, this does
> not have to traverse from the "to" side at all, which makes it quite
> simpler and straight-forward.
>
> I do wonder if we can now reimplement in_merge_bases_many() in terms
> of this helper, and if that gives us a better performance.  It asks
> "is 'commit', i.e. a single 'to', an ancestor of, i.e. reachable
> from, one of the 'references', i.e.  'from'"?

We could do this, but it does come with a performance hit when the following are all true:

1. 'to' is not reachable from any 'from' commits.
2. The 'to' and 'from' commits are close in commit-date.
3. Generation numbers are not available, or the topology is skewed to have
    commits with high commit date and low generation number.

Since in_merge_bases_many() calls paint_down_to_common(), it has the same issues with the current generation numbers. This can be fixed when we have the next version of generation numbers available.

I'll make a note to have in_merge_bases_many() call get_reachable_subset() conditionally (like the generation_numbers_available() trick in the --topo-order series) after the generation numbers are settled and implemented.

Thanks, -Stolee

Previous: Junio C HamanoNext: Junio C Hamano
Message 4 of 22 in “Make add_missing_tags() linear”
  1. 0/3 Make add_missing_tags() linearDerrick Stolee via GitGitGadget, Oct 30, 2018
  2. 1/3 commit-reach: implement get_reachable_subsetDerrick Stolee via GitGitGadget, Oct 30, 2018
  3. Junio C HamanoOct 31, 2018
  4. Derrick StoleeOct 31, 2018
  5. Junio C HamanoNov 2, 2018
  6. Elijah NewrenOct 31, 2018
  7. Derrick StoleeOct 31, 2018
  8. 2/3 test-reach: test get_reachable_subsetDerrick Stolee via GitGitGadget, Oct 30, 2018
  9. 3/3 remote: make add_missing_tags() linearDerrick Stolee via GitGitGadget, Oct 30, 2018
  10. Junio C HamanoOct 31, 2018
  11. Elijah NewrenOct 31, 2018
  12. Derrick StoleeOct 31, 2018
  13. Elijah NewrenNov 1, 2018
  14. Derrick StoleeNov 1, 2018
  15. Elijah NewrenNov 1, 2018
  16. Derrick StoleeNov 1, 2018
  17. Elijah NewrenNov 2, 2018
  18. Derrick StoleeNov 2, 2018
  19. 0/3 Make add_missing_tags() linearDerrick Stolee via GitGitGadget, Nov 2, 2018
  20. 1/3 commit-reach: implement get_reachable_subsetDerrick Stolee via GitGitGadget, Nov 2, 2018
  21. 2/3 test-reach: test get_reachable_subsetDerrick Stolee via GitGitGadget, Nov 2, 2018
  22. 3/3 remote: make add_missing_tags() linearDerrick Stolee via GitGitGadget, Nov 2, 2018

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.