Re: [PATCH] fsmonitor: flush pending FSEvents before cookie wait

From: Patrick Steinhardt <ps@pks.im>
Date: Aug 13, 2026, 09:03 UTC
Message-ID: <an2IT78KDS94JqUt@pks.im>
In-Reply-To: <CAJ-ks9kQR77vH-56eS9tT-iXEnih+Z7SPRMs1gD_wTyg_6gZ_w@mail.gmail.com>

On Wed, Aug 12, 2026 at 09:19:08PM -0400, Tamir Duberstein wrote:

> On Tue, Aug 11, 2026 at 12:45 PM Tamir Duberstein <tamird@gmail.com> wrote:

[snip]

Show 14 quoted lines

> > Got it. I agree with you that the flush call feels unnecessary under
> > the interpretation that passing 1ms to FSEventStreamCreate is the
> > equivalent of asking it to flush every 1ms. Empirically, though,
> > that's not the case, as described in the commit message.
> >
> > There's more precedent for this technique (found by agent, sorry):
> > watchman fixed a similar issue here:
> > https://github.com/facebook/watchman/commit/d1795de4ecab33672a89802318fe6f0122462194
> > and the documented it here:
> > https://github.com/facebook/watchman/commit/2f80886991ce81585ac0679c2b019fa0e4d9e9dd
> >
> > I agree this is unsatisfying.
> >
> > Does that help?

Those links definitely help to provide some more context, thanks!

> I did a bunch more digging and I'm withdrawing this patch. I haven't
> succeeded in proving that this fixes the performance issue. I'll
> resend in case this changes.

One major difference I notice there is that your patch uses `FsEventStreamFlushAsync()`, whereas Watchman uses the `Sync()` variant. That could help explain why it works for their use case, as the can now guarantee that the cookie was indeed processed once that call finishes. But with our `Async()` variant that's a guarantee that we cannot uphold, and consequently we're essentially still racing with the timout.

Now we could of course try to use the synchronous variant ourselves. But I'm a bit concerned that this may create new problems that we don't really understand yet. Quite unfortunate indeed :/

Thanks!

Patrick