All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>,
	"Nicolas Pitre" <nico@fluxnic.net>,
	"Carlo Marcelo Arenas Belón via GitGitGadget"
	<gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 0/2] progress: replace setitimer() with alarm()
Date: Sun, 24 Aug 2025 09:11:46 -0700	[thread overview]
Message-ID: <xmqqsehgu2bh.fsf@gitster.g> (raw)
In-Reply-To: <08f405a6-fd2e-40d7-850a-574356b4009e@kdbg.org> (Johannes Sixt's message of "Sun, 24 Aug 2025 00:03:29 +0200")

Johannes Sixt <j6t@kdbg.org> writes:

>> Operating system folks may have worked hard to minimize the cost of
>> system calls to gettimeofday() in order to help applications that do
>> so, but I somehow feel even dirtier to hear proposal to do so to
>> replace a signal that we set and forget, to be reminded once every
>> second.
>
> I think that ship has sailed already. Look at display_throughput(). One
> of the first things it does is to look at the wallclock a.k.a.
> getnanotime().

It can be fixed if we wanted to, though, no?  Instead of doing all
the computation for the latest lap, and then decide not to show by
looking at the progress_update flag (set by the interrupt), we can
accumulate the total in the progress->throughput struct until we see
the progress_update flag, at which time we can look at the wallclock
time, compute the time difference, perform clever division, etc.

> That said, I am not very happy about the new calls introduced in
> display_progress(), either. I'll see whether I can produce some
> performance measurements.
>
> I observe a behavior change with delayed progress indicators that I have
> to understand and fix it before I can submit the cleaned up patches.

Thanks.

      parent reply	other threads:[~2025-08-24 16:11 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-23 13:22 [PATCH 0/2] progress: replace setitimer() with alarm() Carlo Marcelo Arenas Belón via GitGitGadget
2025-08-23 13:22 ` [PATCH 1/2] " Carlo Marcelo Arenas Belón via GitGitGadget
2025-08-23 13:22 ` [PATCH 2/2] progress: add a shutting down state to the SIGALRM handler Carlo Marcelo Arenas Belón via GitGitGadget
2025-08-23 16:24 ` [PATCH 0/2] progress: replace setitimer() with alarm() Johannes Sixt
2025-08-23 19:38   ` Carlo Marcelo Arenas Belón
2025-08-23 19:55     ` Johannes Sixt
2025-08-23 21:33   ` Junio C Hamano
2025-08-23 21:47     ` Junio C Hamano
2025-08-23 22:03     ` Johannes Sixt
2025-08-24 15:31       ` [PATCH] progress: pay attention to (customized) delay time Johannes Sixt
2025-08-25 17:00         ` Junio C Hamano
2025-08-25 18:11           ` Carlo Marcelo Arenas Belón
2025-08-25 18:50             ` Junio C Hamano
2025-08-25 19:16               ` [PATCH v2] " Johannes Sixt
2025-08-25 22:52                 ` Junio C Hamano
2025-08-24 16:11       ` Junio C Hamano [this message]

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=xmqqsehgu2bh.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=j6t@kdbg.org \
    --cc=nico@fluxnic.net \
    /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.