GNU bug report logs - #65700
time when gcs-done is updated needs to be clarified

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 65700 in the body.
You can then email your comments to 65700 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Shynur Xie <one.last.kiss <at> outlook.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: time when gcs-done is updated needs to be clarified
Date: Sat, 2 Sep 2023 13:00:59 +0000

I wrote a function which runs each time Emacs collects garbage, so I
added it to `post-gc-hook'.

That function cares about the value of `gcs-done' and the value of
`gc-elapsed'.
But it seems that `gcs-done' is updated after `post-gc-hook' is
executed?

Is this an intended behavior?  If so, I think the docstring of these
variables (such as `gcs-done', `gc-elapsed', ...) need to clarify when
the variable is updated.


v29.1

Message #10 received at 65700 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Shynur Xie <one.last.kiss <at> outlook.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 65700 <at> debbugs.gnu.org
Subject: Re: bug#65700: time when gcs-done is updated needs to be clarified
Date: Thu, 07 Sep 2023 12:14:44 +0300

> From: Shynur Xie <one.last.kiss <at> outlook.com>
> Date: Sat, 2 Sep 2023 13:00:59 +0000
> msip_labels: 
> 
> I wrote a function which runs each time Emacs collects garbage, so I
> added it to `post-gc-hook'.
> 
> That function cares about the value of `gcs-done' and the value of
> `gc-elapsed'.
> But it seems that `gcs-done' is updated after `post-gc-hook' is
> executed?
> 
> Is this an intended behavior?  If so, I think the docstring of these
> variables (such as `gcs-done', `gc-elapsed', ...) need to clarify when
> the variable is updated.

Stefan, any comments?

Message #13 received at 65700 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Shynur Xie <one.last.kiss <at> outlook.com>, 65700 <at> debbugs.gnu.org
Subject: Re: bug#65700: time when gcs-done is updated needs to be clarified
Date: Thu, 07 Sep 2023 09:38:35 -0400

>> I wrote a function which runs each time Emacs collects garbage, so I
>> added it to `post-gc-hook'.
>> 
>> That function cares about the value of `gcs-done' and the value of
>> `gc-elapsed'.  But it seems that `gcs-done' is updated after
>> `post-gc-hook' is executed?

Indeed, from what I see in the code, `gc-elapsed` and `gcs-done` are
updated *after* running `post-gc-hook`.
This means for example that the time to run `post-gc-hook` is included
in the `gc-elapsed` time.

>> Is this an intended behavior?

Good question.  AFAICT this was added by the following commit:

    commit 2c5bd60800ce4f2702f263eda2145be342208ffe
    Author: Dave Love <fx <at> gnu.org>
    Date:   Thu Jan 30 14:15:58 2003 +0000
    
        (Vgc_elapsed, gcs_done): New variables.
        (Fgarbage_collect): Use them.
        (init_alloc, syms_of_alloc): Set them up.
    
    diff --git a/src/alloc.c b/src/alloc.c
    --- a/src/alloc.c
    +++ b/src/alloc.c
    @@ -4367,7 +4373,15 @@
       if (!NILP (Vpost_gc_hook))
         {
           int count = inhibit_garbage_collection ();
           safe_run_hooks (Qpost_gc_hook);
           unbind_to (count, Qnil);
         }
    -  
    +
    +  /* Accumulate statistics.  */
    +  EMACS_GET_TIME (t2);
    +  EMACS_SUB_TIME (t3, t2, t1);
    +  if (FLOATP (Vgc_elapsed))
    +    XSETFLOAT (Vgc_elapsed, make_float (XFLOAT_DATA (Vgc_elapsed) +
    +					EMACS_SECS (t3) +
    +					EMACS_USECS (t3) * 1.0e-6));
    +  gcs_done++;

The only related commit I see around that time is the associated NEWS
message:

    ** New variables `gc-elapsed' and `gcs-done' provide extra information
    on garbage collection.

So I strongly suspect that it was largely accidental.  "Philosophically",
both choices make sense (either consider `post-gc-hook` as being part of
the GC or consider it as external to the GC).

I'd be curious to know how it affects your code.


        Stefan

Message #16 received at 65700 <at> debbugs.gnu.org (full text, mbox):

From: Shynur Xie <one.last.kiss <at> outlook.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>
Cc: "65700 <at> debbugs.gnu.org" <65700 <at> debbugs.gnu.org>
Subject: Re: bug#65700: time when gcs-done is updated needs to be clarified
Date: Thu, 7 Sep 2023 14:35:32 +0000

> Stefan:
> I'd be curious to know how it affects your code.

My Emacs displays its runtime information like this:
  <github.com/shynur/.emacs.d/issues/1#issuecomment-1671815270>
Since it's mainly related to GC, I have it updated after each GC.

I believe I'm not the first person to do this; why others didn't
report the bug may be that they don't care whether the number of GCs
is exactly the same as they expect or one less, so they didn't really
check when `gcs-done' is updated.
IOW, if the update time of `gcs-done' is changed, I think most people
won't notice this change at all.

> Stefan:
> "Philosophically", both choices make sense (either consider
> `post-gc-hook` as being part of the GC or consider it as external to
> the GC).

I'm not a native English speaker.  Does the POST in `post-gc-hook'
mean the functions will run when GC is DONE?

> shynur:
> Is this an intended behavior?  If so, I think the docstring of these
> variables (such as `gcs-done', `gc-elapsed', ...) need to clarify
> when the variable is updated.

And `garbage-collection-messages'.

Message #19 received at 65700 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Shynur Xie <one.last.kiss <at> outlook.com>
Cc: monnier <at> iro.umontreal.ca, 65700 <at> debbugs.gnu.org
Subject: Re: bug#65700: time when gcs-done is updated needs to be clarified
Date: Thu, 07 Sep 2023 17:47:18 +0300

> From: Shynur Xie <one.last.kiss <at> outlook.com>
> CC: "65700 <at> debbugs.gnu.org" <65700 <at> debbugs.gnu.org>
> Date: Thu, 7 Sep 2023 14:35:32 +0000
> msip_labels:
> 
> > Stefan:
> > I'd be curious to know how it affects your code.
> 
> My Emacs displays its runtime information like this:
>   <github.com/shynur/.emacs.d/issues/1#issuecomment-1671815270>
> Since it's mainly related to GC, I have it updated after each GC.

That's just causes off-by-one count of GC cycles, no?  Easy enough to
fix.

> > Stefan:
> > "Philosophically", both choices make sense (either consider
> > `post-gc-hook` as being part of the GC or consider it as external to
> > the GC).
> 
> I'm not a native English speaker.  Does the POST in `post-gc-hook'
> mean the functions will run when GC is DONE?

Its being "done" doesn't necessarily have to include updating the
count of GCs.  The main job of GC is already done by the time the hook
is called.

This is what Stefan meant when he said that philosophically it is not
clear-cut whether updating the count should be done before or after
calling the hook.

> > shynur:
> > Is this an intended behavior?  If so, I think the docstring of these
> > variables (such as `gcs-done', `gc-elapsed', ...) need to clarify
> > when the variable is updated.
> 
> And `garbage-collection-messages'.

What about it?

Message #22 received at 65700 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Shynur Xie <one.last.kiss <at> outlook.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "65700 <at> debbugs.gnu.org" <65700 <at> debbugs.gnu.org>
Subject: Re: bug#65700: time when gcs-done is updated needs to be clarified
Date: Thu, 07 Sep 2023 10:58:46 -0400

>> I'd be curious to know how it affects your code.
> My Emacs displays its runtime information like this:
>   <github.com/shynur/.emacs.d/issues/1#issuecomment-1671815270>
> Since it's mainly related to GC, I have it updated after each GC.

Hmm... that doesn't really explain in which way this affects your code.
If you only ever run that from `post-gc-hook` the only difference is
that `gcs-done` will be always smaller by 1 and since it doesn't start
at 0 anyway it's not really relevant: it will still increase by 1 every
time `post-gc-hook` is run.

FWIW, I tend to agree that it would be more useful to run `post-gc-hook`
after those values have been updated.  So that `post-gc-hook` can know
the exact duration of the GC that we just finished, rather than the one
before that.

Any objection to the patch below?


        Stefan


diff --git a/src/alloc.c b/src/alloc.c
index c77bdc6372d..cee43783387 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -6595,13 +6595,6 @@ garbage_collect (void)
   image_prune_animation_caches (false);
 #endif
 
-  if (!NILP (Vpost_gc_hook))
-    {
-      specpdl_ref gc_count = inhibit_garbage_collection ();
-      safe_run_hooks (Qpost_gc_hook);
-      unbind_to (gc_count, Qnil);
-    }
-
   /* Accumulate statistics.  */
   if (FLOATP (Vgc_elapsed))
     {
@@ -6620,6 +6613,13 @@ garbage_collect (void)
       if (tot_after < tot_before)
 	malloc_probe (min (tot_before - tot_after, SIZE_MAX));
     }
+
+  if (!NILP (Vpost_gc_hook))
+    {
+      specpdl_ref gc_count = inhibit_garbage_collection ();
+      safe_run_hooks (Qpost_gc_hook);
+      unbind_to (gc_count, Qnil);
+    }
 }
 
 DEFUN ("garbage-collect", Fgarbage_collect, Sgarbage_collect, 0, 0, "",

Message #25 received at 65700 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: one.last.kiss <at> outlook.com, 65700 <at> debbugs.gnu.org
Subject: Re: bug#65700: time when gcs-done is updated needs to be clarified
Date: Thu, 07 Sep 2023 18:08:56 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  "65700 <at> debbugs.gnu.org"
>  <65700 <at> debbugs.gnu.org>
> Date: Thu, 07 Sep 2023 10:58:46 -0400
> 
> FWIW, I tend to agree that it would be more useful to run `post-gc-hook`
> after those values have been updated.  So that `post-gc-hook` can know
> the exact duration of the GC that we just finished, rather than the one
> before that.
> 
> Any objection to the patch below?

None here, but please call out this incompatible change in NEWS.

Message #28 received at 65700 <at> debbugs.gnu.org (full text, mbox):

From: Shynur Xie <one.last.kiss <at> outlook.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: "65700 <at> debbugs.gnu.org" <65700 <at> debbugs.gnu.org>
Subject: Re: bug#65700: time when gcs-done is updated needs to be clarified
Date: Thu, 7 Sep 2023 15:23:08 +0000

> Eli:
>
> > I'm not a native English speaker.  Does the POST in `post-gc-hook'
> > mean the functions will run when GC is DONE?
>
> Its being "done" doesn't necessarily have to include updating the
> count of GCs.  The main job of GC is already done by the time the
> hook is called.

Thanks for explaining.  Yes, "Its being 'done' doesn't necessarily
have to include updating the count of GCs."

My point is, IF "the functions will run when GC is DONE" THEN when
these functions are being executed, the `gcs-DONE' should have been
updated.

> Eli:
>
> > > Is this an intended behavior?  If so, I think the docstring of
> > > these variables (such as `gcs-done', `gc-elapsed', ...) need to
> > > clarify when the variable is updated.
> 
> > And `garbage-collection-messages'.
>
> What about it?

Mentioning it here is just for reference.  Maybe you want clarify when
the variable/message is updated/generated in the docstring.


> Stefan:
> Hmm... that doesn't really explain in which way this affects your
> code.

It does.

> `gcs-done` will be always smaller by 1

That's it.

Message #31 received at 65700 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Shynur Xie <one.last.kiss <at> outlook.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "65700 <at> debbugs.gnu.org" <65700 <at> debbugs.gnu.org>
Subject: Re: bug#65700: time when gcs-done is updated needs to be clarified
Date: Thu, 07 Sep 2023 12:52:50 -0400

>> Hmm... that doesn't really explain in which way this affects your
>> code.
> It does.

At least not in a way that I understand, then :-(

>> `gcs-done` will be always smaller by 1
> That's it.

Why do you care if it starts at 4 rather than at 5?
Maybe we should make it start at some random number instead, to make it
clear that 0 is meaningless :-)


        Stefan

Message #34 received at 65700 <at> debbugs.gnu.org (full text, mbox):

From: Shynur Xie <one.last.kiss <at> outlook.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: "65700 <at> debbugs.gnu.org" <65700 <at> debbugs.gnu.org>
Subject: Re: bug#65700: time when gcs-done is updated needs to be clarified
Date: Thu, 7 Sep 2023 17:19:48 +0000

> Stefan:
> Why do you care if it starts at 4 rather than at 5?

To be honest, I don't really care; it's quite easy for me to fix this
in my function (although I'm a little freaked out that the behavior
doesn't match the docstring: "accumulated number of garbage
collections done.").  Even if I just leave it there, it's only one
less than it actually is.

But at least I should let Emacs developers know this.  If this is not
an intended behavior, they can fix it; if it is, they may make it
clear in docstring.

So the question is:  do you care?  or, do you think users care?

If the answer is no, then there's no need to change it, just clarify
this behavior in the docstring of `post-gc-hook'.

Message #37 received at 65700 <at> debbugs.gnu.org (full text, mbox):

From: Shynur Xie <one.last.kiss <at> outlook.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: "65700 <at> debbugs.gnu.org" <65700 <at> debbugs.gnu.org>
Subject: Re: bug#65700: time when gcs-done is updated needs to be clarified
Date: Tue, 12 Sep 2023 10:14:16 +0000

> shynur:
> My Emacs displays its runtime information like this:
>   <github.com/shynur/.emacs.d/issues/1#issuecomment-1671815270>
> Since it's mainly related to GC, I have it updated after each GC.

> Eli:
> That's just causes off-by-one count of GC cycles, no?  Easy enough
> to fix.

> Stefan:
> the only difference is that `gcs-done` will be always smaller by 1

We forgot the `gc-elapsed'.  It's a little hard to fix (dirty work is
needed).

Message #40 received at 65700 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Shynur Xie <one.last.kiss <at> outlook.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "65700 <at> debbugs.gnu.org" <65700 <at> debbugs.gnu.org>
Subject: Re: bug#65700: time when gcs-done is updated needs to be clarified
Date: Tue, 12 Sep 2023 08:53:43 -0400

>> My Emacs displays its runtime information like this:
>>   <github.com/shynur/.emacs.d/issues/1#issuecomment-1671815270>
>> Since it's mainly related to GC, I have it updated after each GC.
>
>> Eli:
>> That's just causes off-by-one count of GC cycles, no?  Easy enough
>> to fix.
>
>> Stefan:
>> the only difference is that `gcs-done` will be always smaller by 1
>
> We forgot the `gc-elapsed'.  It's a little hard to fix (dirty work is
> needed).

Yup, for `gcs-done`, it doesn't matter very much, but with the current
code `post-gc-hook` simply can't know how much time the last GC took,
which can be a useful information there.
I'll push my patch to `master` soon, I just need to clean it up first.


        Stefan

Message #43 received at 65700 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Shynur Xie <one.last.kiss <at> outlook.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "65700 <at> debbugs.gnu.org" <65700 <at> debbugs.gnu.org>
Subject: Re: bug#65700: time when gcs-done is updated needs to be clarified
Date: Tue, 12 Sep 2023 14:07:19 -0400

> I'll push my patch to `master` soon, I just need to clean it up first.

Done, closing,


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.