Re: [PATCH 4/6] packed_ref_iterator_begin(): make optimization more general

[Jeff King]

On Wed, Jan 24, 2018 at 12:14:14PM +0100, Michael Haggerty wrote:

> We can return an empty iterator not only if the `packed-refs` file is
> missing, but also if it is empty or if there are no references whose
> names succeed `prefix`. Optimize away those cases as well by moving
> the call to `find_reference_location()` higher in the function and
> checking whether the determined start position is the same as
> `snapshot->eof`. (This is possible now because the previous commit
> made `find_reference_location()` robust against empty snapshots.)

Makes sense.

> @@ -937,11 +942,6 @@ static struct ref_iterator *packed_ref_iterator_begin(
>   iter->snapshot = snapshot;
>   acquire_snapshot(snapshot);
>  
> - if (prefix && *prefix)
> - start = find_reference_location(snapshot, prefix, 0);
> - else
> - start = snapshot->start;
> -

I did a double-take here that we are now looking at the snapshot without
calling acquire_snapshot(). But that function is just about taking a
refcount on it. The actual acquisition of data happens in
get_snapshot().

-Peff

[PATCH 4/6] packed_ref_iterator_begin(): make optimization more general

[Michael Haggerty]

We can return an empty iterator not only if the `packed-refs` file is
missing, but also if it is empty or if there are no references whose
names succeed `prefix`. Optimize away those cases as well by moving
the call to `find_reference_location()` higher in the function and
checking whether the determined start position is the same as
`snapshot->eof`. (This is possible now because the previous commit
made `find_reference_location()` robust against empty snapshots.)

Signed-off-by: Michael Haggerty 
---
 refs/packed-backend.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 361affd7ad..988c45402b 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -927,7 +927,12 @@ static struct ref_iterator *packed_ref_iterator_begin(
 */
snapshot = get_snapshot(refs);
 
-   if (!snapshot->buf)
+   if (prefix && *prefix)
+   start = find_reference_location(snapshot, prefix, 0);
+   else
+   start = snapshot->start;
+
+   if (start == snapshot->eof)
return empty_ref_iterator_begin();
 
iter = xcalloc(1, sizeof(*iter));
@@ -937,11 +942,6 @@ static struct ref_iterator *packed_ref_iterator_begin(
iter->snapshot = snapshot;
acquire_snapshot(snapshot);
 
-   if (prefix && *prefix)
-   start = find_reference_location(snapshot, prefix, 0);
-   else
-   start = snapshot->start;
-
iter->pos = start;
iter->eof = snapshot->eof;
strbuf_init(&iter->refname_buf, 0);
-- 
2.14.2