On Mon, 21 Jan 2008, Sergey Vlasov wrote: > On Mon, Jan 21, 2008 at 11:07:15AM -0500, Nicolas Pitre wrote: > > On Mon, 21 Jan 2008, Sergey Vlasov wrote: > > > > > When partitioning the work amongst threads, dividing the number of > > > objects by the number of threads may return 0 when there are less > > > objects than threads; this will cause the subsequent code to segfault > > > when accessing list[sub_size-1]. Fix this by ensuring that sub_size > > > is not zero if there is at least one object to process. > > > > No. Forcing one object in a thread is counter productive since it won't > > have anything to delta against. Instead, the thread should be allowed > > to have zero objects and let the other threads have more. > > > > This patch would be a proper fix: > > > > diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c > > index ec10238..d3efeff 100644 > > --- a/builtin-pack-objects.c > > +++ b/builtin-pack-objects.c > > @@ -1672,7 +1672,8 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size, > > p[i].data_ready = 0; > > > > /* try to split chunks on "path" boundaries */ > > - while (sub_size < list_size && list[sub_size]->hash && > > + while (sub_size && sub_size < list_size && > > + list[sub_size]->hash && > > list[sub_size]->hash == list[sub_size-1]->hash) > > sub_size++; > > Actually there will not be any significant differences - with my patch > the object distribution between threads will be 1, 1, ..., 0, 0..., > and with your patch it would be 0, 0, ..., 1, 1, ... Or more likely 0, 0, ..., 2. And the code is simpler. > We could even introduce some limit on the number of objects below > which multithreaded packing is not attempted, so that packing a small > number of objects would be more efficient. Possibly. But that's not a requirement at this moment. Nicolas - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
