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

On Wed, Aug 12, 2026 at 09:19:08PM -0400, Tamir Duberstein wrote:
> On Tue, Aug 11, 2026 at 12:45 PM Tamir Duberstein <[email protected]> wrote:
[snip]
> > 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