All of lore.kernel.org
 help / color / mirror / Atom feed
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.

  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.