On Sat, Apr 6, 2024 at 6:55 AM Melanie Plageman
<melanieplageman@gmail.com> wrote:
> On Fri, Apr 5, 2024 at 12:15 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> > The interesting column is hot. The 200ms->211ms regression is due to
> > the extra bookkeeping in the slow path. The rejiggered fastpath code
> > fixes it for me, or maybe sometimes shows an extra 1ms. Phew. Can
> > you reproduce that?
>
> I am able to reproduce the fast path solving the issue using Heikki's
> example here [1] but in shared buffers (hot).
>
> master: 25 ms
> stream read: 29 ms
> stream read + fast path: 25 ms
Great, thanks.
> I haven't looked into or reviewed the memory prefetching part.
>
> While reviewing 0002, I realized that I don't quite see how
> read_stream_get_block() will be used in the fastpath -- which it
> claims in its comments.
Comments improved.
> Oh and why does READ_STREAM_DISABLE_FAST_PATH macro exist?
Just for testing purposes. Behaviour should be identical to external
observers either way, it's just a hand-rolled specialisation for
certain parameters, and it's useful to be able to verify that and
measure the speedup. I think it's OK to leave a bit of
developer/testing scaffolding in the tree if it's likely to be useful
again and especially if like this case it doesn't create any dead
code. (Perhaps in later work we might find the right way to get the
compiler to do the specialisation? It's so simple though.)
The occasional CI problem I mentioned turned out to be
read_stream_reset() remembering a little too much state across
rescans. Fixed.
Thanks both for the feedback on the ring buffer tweaks. Comments
updated. Here is the full stack of patches I would like to commit
very soon, though I may leave the memory prefetching part out for a
bit longer to see if I can find any downside.