Re: [PATCH 3/6] connected: refactor iterator to return next object ID directly

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1460 bytes --]

On Fri, Aug 20, 2021 at 07:43:06PM +0200, René Scharfe wrote:
> Am 20.08.21 um 12:08 schrieb Patrick Steinhardt:
> > The object ID iterator used by the connectivity checks returns the next
> > object ID via an out-parameter and then uses a return code to indicate
> > whether an item was found. This is a bit roundabout: instead of a
> > separate error code, we can just retrun the next object ID directly and
> 
> s/retrun/return/
> 
> > use `NULL` pointers as indicator that the iterator got no items left.
> > Furthermore, this avoids a copy of the object ID.
> >
> > Refactor the iterator and all its implementations to return object IDs
> > directly. While I was honestly hoping for a small speedup given that we
> > can now avoid a copy, both versions perform the same. Still, the end
> > result is easier to understand and thus it makes sense to keep this
> > refactoring regardless.
> 
> check_connected() calls find_pack_entry_one() on the object ID hash,
> which copies it anyway.  Perhaps that and caching prevent the expected
> speedup?
> 
> The private copy made sure check_connected() could not modify the
> object IDs.  It still only reads them with this patch, but the compiler
> no longer prevents writes.  The iterators could return const pointers
> to restore that guarantee.

Right, will change the signature and re-benchmark. I'd be surprised if
it significantly changed the picture, but let's see.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]