Re: [PATCH 08/27] pack: move approximate object count to object store

On Fri, Feb 23, 2018 at 02:22:14PM -0800, Stefan Beller wrote:

quoted

quoted

+     /*
+      * A fast, rough count of the number of objects in the repository.
+      * These two fields are not meant for direct access. Use
+      * approximate_object_count() instead.
+      */
+     unsigned long approximate_object_count;
+     unsigned approximate_object_count_valid : 1;

Patch looks fine and is effectively a no-op, though what is the need for
both of these variables?  Maybe it can be simplified down to just use
one?  Just musing as its out of the scope of this patch and we probably
shouldn't try to change that in this series.

I agree we should. It was introduced in e323de4ad7f (sha1_file:
allow sha1_loose_object_info to handle arbitrary repositories, 2017-08-30)
and I think it was seen as a clever optimization trick back then?

I think you meant 8e3f52d778 (find_unique_abbrev: move logic out of
get_short_sha1(), 2016-10-03)?

Yes, it was just to avoid the dual-meaning of "0" for "not set" and a
repository with no packfiles.  It would probably be fine to get rid of
it. If you have no packfiles then you probably don't have enough objects
to worry about micro-optimizing. And anyway, the "wasted" case wouldn't
even make any syscalls (it would do a noop prepare_packed_git() and then
realize the packed_git list is empty).

-Peff