From: Junio C Hamano <gitster@pobox.com>
To: Koji Nakamaru <koji.nakamaru@gree.net>
Cc: Tamir Duberstein <tamird@gmail.com>,
git@vger.kernel.org, Jeff Hostetler <jeffhost@microsoft.com>,
Paul Tarjan <github@paulisageek.com>,
Patrick Steinhardt <ps@pks.im>, Jeff King <peff@peff.net>,
Taylor Blau <me@ttaylorr.com>,
Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH] fsmonitor: flush pending FSEvents before cookie wait
Date: Fri, 24 Jul 2026 13:38:52 -0700 [thread overview]
Message-ID: <xmqqh5lo5dib.fsf@gitster.g> (raw)
In-Reply-To: <CAOTNsDy4pKbPHdK1T688Ax6Mgz15K-qfZR-8fAvTk48z3E43Rg@mail.gmail.com> (Koji Nakamaru's message of "Fri, 24 Jul 2026 11:41:08 +0900")
Koji Nakamaru <koji.nakamaru@gree.net> writes:
> On Wed, Jul 22, 2026 at 6:05 AM Tamir Duberstein <tamird@gmail.com> wrote:
>>
>> 56cef9cb1a (fsmonitor: use pthread_cond_timedwait for cookie wait,
>> 2026-04-15) limits the cookie wait to one second so that a filesystem
>> which never delivers events cannot hang fsmonitor clients. A client that
>> times out receives a trivial response and scans the entire index.
>>
>> FSEvents can defer delivery while it batches notifications and does not
>> guarantee that its queue is drained in one latency interval. A loaded
>> macOS system can therefore time out even though the event stream is
>> working.
>>
>> On an Apple M4 Max (16 cores, 128 GiB RAM) running macOS 26.5.2, two
>> worktrees with a 1,001,178-entry index timed out 484 of 545 and 297 of
>> 365 fsmonitor requests. One status call performed 934,519 lstat() calls
>> during a 47-second preload and took 52 seconds overall.
>>
>> Ask FSEvents to flush pending notifications after creating the cookie
>> and before starting the timed wait. Use the asynchronous form because
>> the client handler holds main_lock, which the listener callback also
>> acquires. Keep the timeout and the behavior of the other backends
>> unchanged.
>>
>> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>> ---
>>...
> This patch is carefully designed to minimize any risks. To drain events,
> we could also call FSEventStreamFlushSync before acquiring main_lock in
> do_handle_client(), but this patch should be sufficient if it mitigates
> the issue. The commit message would be much more convincing if you also
> included benchmark results showing how many timeouts were reduced.
Thanks for a review.
next prev parent reply other threads:[~2026-07-24 20:38 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-21 21:04 [PATCH] fsmonitor: flush pending FSEvents before cookie wait Tamir Duberstein
2026-07-24 2:41 ` Koji Nakamaru
2026-07-24 20:38 ` Junio C Hamano [this message]
2026-08-04 22:13 ` Junio C Hamano
2026-08-05 7:59 ` Patrick Steinhardt
2026-08-11 15:22 ` Tamir Duberstein
2026-08-11 16:23 ` Patrick Steinhardt
2026-08-11 16:45 ` Tamir Duberstein
2026-08-11 17:35 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqh5lo5dib.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=github@paulisageek.com \
--cc=jeffhost@microsoft.com \
--cc=koji.nakamaru@gree.net \
--cc=me@ttaylorr.com \
--cc=peff@peff.net \
--cc=ps@pks.im \
--cc=tamird@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.