On Sun, Jul 20, 2025 at 1:07 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Sat, Jul 19, 2025 at 11:23 PM Tomas Vondra <tomas@vondra.me> wrote:
> > Thanks for the link. It seems I came up with an almost the same patch,
> > with three minor differences:
> >
> > 1) There's another place that sets "distance = 0" in
> > read_stream_next_buffer, so maybe this should preserve the distance too?
> >
> > 2) I suspect we need to preserve the distance at the beginning of
> > read_stream_reset, like
> >
> > stream->reset_distance = Max(stream->reset_distance,
> > stream->distance);
> >
> > because what if you call _reset before reaching the end of the stream?
> >
> > 3) Shouldn't it reset the reset_distance to 0 after restoring it?
>
> Probably. Hmm... an earlier version of this code didn't use distance
> == 0 to indicate end-of-stream, but instead had a separate internal
> end_of_stream flag. If we brought that back and didn't clobber
> distance, we wouldn't need this save-and-restore dance. It seemed
> shorter and sweeter without it back then, before _reset() existed in
> its present form, but I wonder if end_of_stream would be nicer than
> having to add this kind of stuff, without measurable downsides.
...
> Good question. Yeah, your flag idea seems like a good way to avoid
> baking opinion into this level. I wonder if it should be a bitmask
> rather than a boolean, in case we think of more things that need to be
> included or not when resetting.
Here's a sketch of the above two ideas for discussion (.txt to stay
off cfbot's radar for this thread). Better than save/restore?
Here also are some alternative experimental patches for preserving
accumulated look-ahead distance better in cases like that. Needs more
exploration... thoughts/ideas welcome...
