Re: regression in multi-threaded git-pack-index

On Tue, Mar 19, 2013 at 3:17 PM, Thomas Rast [off-list ref] wrote:

quoted

but the line in question is:

  if (deepest_delta < delta_obj->delta_depth)

...

Duy, what was the reasoning why resolve_delta() does not need to hold
locks when it looks when it looks at deepest_delta?  My coffee levels
aren't up to this task yet.  It certainly seems extremely dubious to me,
as the code uses the global deepest_delta in threaded sections.  You can
probably argue that the load/store is atomic on most(?) platforms, but
you get no guarantees that deepest_delta at any time in fact holds the
maximum value of delta_obj->delta_depth.

Now that I have had dinner (and energy restored), the explanation
might be because I missed it. resolve_delta() deals with data in
delta_obj and does not share global state, except this one and
nr_resolved_deltas. The latter is protected. I guess we could protect
this one with a mutex. But only do so when "--verify" is specified

if (stat) {
   lock();
   if (deepest_delta < delta_obj->delta_depth)
      deepest_delta = delta_obj->delta_depth;
   unlock();
}

so that we don't need to hold/release lock when index-pack is run.
-- 
Duy