Re: [PATCH v2 14/25] shallow.c: implement a generic shallow boundary finder based on rev-list

From: Duy Nguyen <pclouds@gmail.com>
Date: Feb 15, 2016, 08:00 UTC
Message-ID: <CACsJy8CjnD-f-fyizQ6DE6ik5vYQqYq+R-YSEJ7XZ9XTHEH1Tg@mail.gmail.com>
In-Reply-To: <xmqqvb5y7vuj.fsf@gitster.mtv.corp.google.com>

On Tue, Feb 9, 2016 at 4:09 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 24 quoted lines

>> +     is_repository_shallow(); /* make sure shallows are read */
>> +
>> +     init_revisions(&revs, NULL);
>> +     save_commit_buffer = 0;
>> +     setup_revisions(ac, av, &revs, NULL);
>> +
>> +     /* Mark all reachable commits as NOT_SHALLOW */
>> +     if (prepare_revision_walk(&revs))
>> +             die("revision walk setup failed");
>> +     traverse_commit_list(&revs, show_commit, NULL, &not_shallow_flag);
>> +
>> +     /*
>> +      * mark border commits SHALLOW + NOT_SHALLOW.
>> +      * We cannot clear NOT_SHALLOW right now. Imagine border
>> +      * commit A is processed first, then commit B, whose parent is
>> +      * A, later. If NOT_SHALLOW on A is cleared at step 1, B
>> +      * itself is considered border at step 2, which is incorrect.
>> +      */
>> +     nr = get_max_object_index();
>> +     for (i = 0; i < nr; i++) {
>
> I'd really like not to see a loop over 0..get_max_object_index().
> Are there many codepaths that peek into the in-core entire object
> store already?

You started it with check_non_tip(). At least that's how I know about this loop. But I think that's the only code path, not counting this.

> Would it work equally well to keep track of the
> commits discovered in show_commit() to use as the set of commits
> you need to visit in this second pass?

We can't do this in show_commit. In this loop, we check not_shallow_flag of parent commits. If one parent commit is not show_commit'd yet, the flag is not set and we may incorrectly think this is a border commit. The only way to avoid going through the entire in-core object database is keeping a new commit_list and go through it here. Which way is preferred?

Show 18 quoted lines

>> +             struct object *o = get_indexed_object(i);
>> +             struct commit *c = (struct commit *)o;
>> +
>> +             if (!o || o->type != OBJ_COMMIT ||
>> +                 !(o->flags & not_shallow_flag))
>> +                     continue;
>> +
>> +             if (parse_commit(c))
>> +                     die("unable to parse commit %s",
>> +                         oid_to_hex(&c->object.oid));
>> +
>> +             for (p = c->parents; p; p = p->next)
>> +                     if (!(p->item->object.flags & not_shallow_flag)) {
>> +                             o->flags |= shallow_flag;
>> +                             commit_list_insert(c, &result);
>> +                             break;
>> +                     }
>> +     }

-- 
Duy