Re: Git Test Coverage Report (Sept 19)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Sep 23, 2019 at 10:00:37AM -0400, Derrick Stolee wrote:

> > builtin/pack-objects.c
> > 7c59828b 2694) (reuse_packfile_bitmap &&
> > 7c59828b 2695)  bitmap_walk_contains(bitmap_git, reuse_packfile_bitmap, oid));
> 
> This change to obj_is_packed(oid) is a small change in a
> really big commit. Here is the change:

I'd suggest not looking too closely at this one yet. This was a
preliminary split by Christian of some older code that I had written. I
think it needs to be split up more, and probably does need more test
coverage (bitmaps are only enabled in a few specific scripts).

> @@ -2571,7 +2706,9 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,
>  
>  static int obj_is_packed(const struct object_id *oid)
>  {
> -       return !!packlist_find(&to_pack, oid, NULL);
> +       return packlist_find(&to_pack, oid, NULL) ||
> +               (reuse_packfile_bitmap &&
> +                bitmap_walk_contains(bitmap_git, reuse_packfile_bitmap, oid));
>  }
> 
> So, every time this is called, the || is short-circuited because
> packlist_find() returns true.

That does surprise me, though. I'm not sure if it's insufficient
testing, or if there's a bug. 

> Does this change the meaning of this method? obj_is_packed() would
> only return true if we found the object specifically in the to_pack
> list. Now, we would also return true if the object is in the bitmap
> walk.
> 
> If this is only a performance improvement, and the bitmap_walk_contains()
> method would return the same as packlist_find(), then should the order
> be swapped? Or rather, should reuse_packfile_bitmap indicate which to use
> as the full result?

No, there should be cases where the walk mentions some objects that
aren't added to the to_pack list (that's the point of the reuse code; it
fast-tracks a big chunk of objects directly to the output).

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux