Re: [PATCH v7 2/3] graph: add a 2 commit buffer for lookahead

>
> The hardcoded size-2 lookahead buffer was my suggestion,
> so I am responding inline with my thoughts although Pablo is
> the right person for making further changes (if any).
>
> On Mon, 6 Jul 2026, Chandra Pratap <chandrapratap3519@gmail.com> wrote:
> > Do we need to NULL out the retrieved buffer entries? If so, it is
> > worthwhile asserting that the entire buffer is NULLed out in the
> > !graph->lookahead_nr check above.
>
> You're right, it's not technically needed, and there are many places
> in the repo where stale data remains in buffers, and it would be possible
> to do that here too. I don't think it matters much in practice though,
> and NULLing them out would perhaps prevent some accidental reuse on bugs
> (NULL would crash instead).
>
> As for asserting: rather than checking that empty slots are NULL
> (which just verifies our own cleanup), it might be more useful to
> assert that a slot is non-NULL when lookahead_nr says it should be
> populated, i.e. assert on read rather than on empty. But even that
> may be overkill for a 2-element internal buffer.

> > Not the best engineering practice, but I guess it is fine to constrain
> > the logic to _only_ a 2-entry buffer since that's what we'll always
> > deal with anyway.
>
> I did consider making it a proper ring buffer, but it felt like
> overkill (and I could not find any other existing ring buffer to
> piggy-back on in the repo), and the lookahead depth is
> structurally tied to the algorithm - we only ever need two more
> elements.
>
> It also helps that this is entirely internal to graph.c. If the
> buffer were part of a broader API, a less hardcoded approach
> would be more appropriate indeed.

> > We should use ARRAY_SIZE(graph->lookahead) instead of hardcoding
> > the value 2.
>
> Agreed, that is a nice improvement. What do you think Pablo?
>
> Thanks,
> Kristofer