Re: [PATCH 6/6] midx.c: include preferred pack correctly with existing MIDX

[Taylor Blau <me@ttaylorr.com>]

On Sun, Aug 21, 2022 at 12:10:42AM +0530, Abhradeep Chakraborty wrote:
> On Sat, Aug 20, 2022 at 3:00 AM Taylor Blau <me@ttaylorr.com> wrote:
> >
> > +               if (-1 < preferred_pack && preferred_pack < start_pack)
> > +                       midx_fanout_add_pack_fanout(&fanout, info,
> > +                                                   preferred_pack, 1,
> > +                                                   cur_fanout);
> > +
>
> All the other changes make sense to me but I have a question about
> this particular change. Instead of adding all the preferred objects
> again (but in this case these are being added from preferred pack) in
> `fanout->entries`, will it be better if we call
> `midx_fanout_add_pack_fanout()` function from
> `midx_fanout_add_midx_fanout()` when above conditions are met?
> Something like this -
>
>     static void midx_fanout_add_midx_fanout(struct midx_fanout *fanout,
>                                         struct multi_pack_index *m,
>                                         struct pack_info *info,
>                                         uint32_t cur_pack,
>                                         int preferred,
>                                         uint32_t cur_fanout)
>     {
>      ...
>           if (cur_fanout)
>                 start = ntohl(m->chunk_oid_fanout[cur_fanout - 1]);
>           end = ntohl(m->chunk_oid_fanout[cur_fanout]);
>           if (preferred) {
>                 midx_fanout_add_pack_fanout(&fanout, info, cur_pack,
>
> preferred, cur_fanout);
>                 return;
>           }
>
>           for (.....) {
>           ........
>     }

A slightly simpler approach might be to see that the pack_midx_entry
structure we get back from calling nth_midxed_pack_midx_entry() contains
the pack from which each object is represented. So if we see one that
collides with the preferred pack, then we can just skip it, since we
know they'll be handled separately down below.

I'll add another patch on top which makes that optimization.

Thanks,
Taylor