On Thu, May 26, 2016 at 08:02:36AM +0000, Eric Wong wrote:
> > That's probably OK in practice, as I think the actual pack-write already
> > does a linear walk of the object table to generate the pack index. So
> > presumably nobody checkpoints often enough for it to be a problem. And
> > the solution would be to keep a separate list of pointers to objects for
> > the current pack-id, which would trivially fix both this case and the
> > one in create_index(). So we can punt on it until somebody actually
> > runs into it, I think.
>
> I might checkpoint that much and run into the problem soon :)
> Too tired now; maybe in a day or two I'll be able to make sense
> of C again :x
In theory the list of objects in the allocation pool are contiguous for
a particular pack. So you could break out of the double-loop at the
first non-matching object you see:
diff --git a/fast-import.c b/fast-import.c
index 83558dc..f214bd3 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -900,10 +900,13 @@ static const char *create_index(void)
c = idx;
for (o = blocks; o; o = o->next_pool)
for (e = o->next_free; e-- != o->entries;)
if (pack_id == e->pack_id)
*c++ = &e->idx;
+ else
+ goto done;
+done:
last = idx + object_count;
if (c != last)
die("internal consistency error creating the index");
tmpfile = write_idx_file(NULL, idx, object_count, &pack_idx_opts,
pack_data->sha1);
But that seems to break some of the tests, so I think there is some case
which does not match my theory. :)
Another strategy is to note that we cross-check against object_count
here. So you could simply break out of the loop when we have found
object_count matches.
We don't keep similar counters for branches/tags (which have a similar
quadratic problem, though presumably much smaller "n"), but it would be
easy to keep an offset in start_packfile() and just start the iteration
there.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
