Time to merge scratch/correct-warning-pos into master, perhaps?

Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Emacs.

The branch scratch/correct-warning-pos is working and is almost
finished.  Its intention is to get the byte compiler to report correct
source positions in warnings and errors it emits.  It remains just to
write some documentation about symbols with position and to fix any last
minute bugs which turn up.

There is a minor bug in test/lisp/emacs-lisp/comp-tests.el, which should
be easier to resolve when Andrea is back again.

To use it, just merge it and use it.  You shouldn't notice much
difference.  If there are any byte compiler warning positions which
remain uncorrect, please report them.

Thie branch's predecessor was rejected in 2018 since it slowed down
Emacs too much.  My benchmarks this afternoon show a much smaller
slowdown, in the region of 1%.

In these benchmarks, I used the following function:

    (defun time-scroll-b (&optional arg)
      (condition-case nil
          (while t
            (if arg (scroll-down) (scroll-up))
            (sit-for 0))
        (error nil)))

in conjunction with benchmark-run in native-compiled Emacsen started
with emacs -Q on buffer src/xdisp.c.  Both repositories were brought up
to date yesterday evening, and master merged into the branch.

Before each timing run I typed and deleted the character at the start of
the buffer (to wipe out the fontification) and executed M-x
garbage-collect.

Here are the four timings I did on master:
      o - (20.798601883 460 7.728701306)
      o - (20.947118356 295 7.1172684969999995)
      o - (20.941589929 293 7.144901186)
      o - (20.917180235 293 7.136285445000002)

..  Here are the same four timings on scratch/correct-warning-pos):
      o - (20.854543266 480 7.691123986)
      o - (21.064465459 320 7.189660959000001)
      o - (21.143813105 318 7.287708998000001)
      o - (21.115932422 318 7.266432223999999)

..  It can be seen that the branch is around 1% slower than master.  This
minor slowdown should be worth it to get correct positions in compile
warning messages.

So, I would propose to merge the branch into master now.  What do people
(in particular Eli) say about this?

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sat, 15 Jan 2022 18:15:17 +0000
> From: Alan Mackenzie <acm@muc.de>
> 
> So, I would propose to merge the branch into master now.  What do people
> (in particular Eli) say about this?

Fine with me if Lars agrees.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> So, I would propose to merge the branch into master now.  What do people
>> (in particular Eli) say about this?
>
> Fine with me if Lars agrees.

I'm seeing one compilation warning on the branch:

Compiler-macro error for cl-member: Handler: cl--compiler-macro-member
(excessive-variable-binding)
Eager macro-expansion failure: (excessive-variable-binding)

And a "time make -j8 bootstrap check" takes

real	3m31.160s

while it's

real	3m16.761s

on master.  A 7% slowdown is rather significant.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Po Lu]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> And a "time make -j8 bootstrap check" takes
>
> real	3m31.160s
>
> while it's
>
> real	3m16.761s
>
> on master.  A 7% slowdown is rather significant.

Perhaps correct warning positions could be made a build time option?

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Po.

On Sun, Jan 16, 2022 at 17:51:59 +0800, Po Lu wrote:
> Lars Ingebrigtsen <larsi@gnus.org> writes:

> > And a "time make -j8 bootstrap check" takes

> > real	3m31.160s

> > while it's

> > real	3m16.761s

> > on master.  A 7% slowdown is rather significant.

> Perhaps correct warning positions could be made a build time option?

I would really rather not do that.  It would be a LOT of work, and the
end result would not really be satisfactory.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> And a "time make -j8 bootstrap check" takes

A better benchmark is something like the following, since we're
interested in byte compilation speed and not the rest:

time for i in `seq 1 10`; do rm lisp/gnus/*.elc lisp/org/*.elc; make -j8; done

This is 17% slower on my build machine on the branch compared to master.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Po Lu]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> A better benchmark is something like the following, since we're
> interested in byte compilation speed and not the rest:
>
> time for i in `seq 1 10`; do rm lisp/gnus/*.elc lisp/org/*.elc; make -j8; done
>
> This is 17% slower on my build machine on the branch compared to master.

How much does it affect native compilation?  Background native
compilation is already slow enough to be annoying.  Multiple people have
said that they disabled that feature because it was too slow, so making
it even slower would be a step in the wrong direction, IMHO.

Thanks.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

> How much does it affect native compilation?  Background native
> compilation is already slow enough to be annoying.  Multiple people have
> said that they disabled that feature because it was too slow, so making
> it even slower would be a step in the wrong direction, IMHO.

Looks like a native bootstrap is about 10% slower on the branch than on
the trunk on my machine.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Lars.

On Sun, Jan 16, 2022 at 14:12:10 +0100, Lars Ingebrigtsen wrote:
> Po Lu <luangruo@yahoo.com> writes:

> > How much does it affect native compilation?  Background native
> > compilation is already slow enough to be annoying.  Multiple people have
> > said that they disabled that feature because it was too slow, so making
> > it even slower would be a step in the wrong direction, IMHO.

> Looks like a native bootstrap is about 10% slower on the branch than on
> the trunk on my machine.

Yes.  I measure it variously around 7% or 8%.

> -- 
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Po Lu]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Po Lu <luangruo@yahoo.com> writes:
>
>> How much does it affect native compilation?  Background native
>> compilation is already slow enough to be annoying.  Multiple people have
>> said that they disabled that feature because it was too slow, so making
>> it even slower would be a step in the wrong direction, IMHO.
>
> Looks like a native bootstrap is about 10% slower on the branch than on
> the trunk on my machine.

How much slower is async native compilation?  Will the bootstrap timings
translate cleanly to that?

Thanks in advance.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Alan Mackenzie <acm@muc.de>,  Eli Zaretskii <eliz@gnu.org>,
>  emacs-devel@gnu.org
> Date: Mon, 17 Jan 2022 08:28:35 +0800
> 
> How much slower is async native compilation?

Same as synchronous one, I guess.  It's the same code, basically, it
is just run in a separate executable.

> Will the bootstrap timings translate cleanly to that?

I don't understand: the bootstrap doesn't use async native
compilation.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> Same as synchronous one, I guess.  It's the same code, basically, it
> is just run in a separate executable.

Thanks.

> I don't understand: the bootstrap doesn't use async native
> compilation.

I was asking whether or not the 7-8% slowdown during bootstrap would
reflect the slowdown of async native compilation in the branch as well.

Thanks.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Pl.

On Sun, Jan 16, 2022 at 20:06:43 +0800, Po Lu wrote:
> Lars Ingebrigtsen <larsi@gnus.org> writes:

> > A better benchmark is something like the following, since we're
> > interested in byte compilation speed and not the rest:

> > time for i in `seq 1 10`; do rm lisp/gnus/*.elc lisp/org/*.elc; make -j8; done

> > This is 17% slower on my build machine on the branch compared to master.

> How much does it affect native compilation?  Background native
> compilation is already slow enough to be annoying.  Multiple people have
> said that they disabled that feature because it was too slow, so making
> it even slower would be a step in the wrong direction, IMHO.

Having wrong warning messages from the byte compiler is also not good.
We have had a lot of bug reports about this, even if not for some while.

The thing about byte compilation is that it is done only rarely - for
many users, possibly most, exactly once.  It is only really we
developers who will notice a moderate decrease in compilation speed.

My bootstrap on the branch takes 7m 14s.  On master it's 6m 40s.  That's
only 34s in 400s, which isn't even 10%.  It doesn't even make the
difference between fetching a cup of coffee and not.

> Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Sun, 16 Jan 2022 13:02:34 +0100
> Cc: Alan Mackenzie <acm@muc.de>, emacs-devel@gnu.org
> 
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
> > And a "time make -j8 bootstrap check" takes
> 
> A better benchmark is something like the following, since we're
> interested in byte compilation speed and not the rest:
> 
> time for i in `seq 1 10`; do rm lisp/gnus/*.elc lisp/org/*.elc; make -j8; done
> 
> This is 17% slower on my build machine on the branch compared to master.

Since this is glaringly inconsistent with the timings Alan published,
I think it would be a good idea to measure the difference for each of
the*.el files, because it could be that a couple of outliers skew the
entire picture.  That is, put the 'time' command inside the 'do', and
don't use -j8; then compare the outputs.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Since this is glaringly inconsistent with the timings Alan published,

Did Alan benchmark byte compilation?

> I think it would be a good idea to measure the difference for each of
> the*.el files, because it could be that a couple of outliers skew the
> entire picture.  That is, put the 'time' command inside the 'do', and
> don't use -j8; then compare the outputs.

I'm not sure what that's supposed to show?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: acm@muc.de,  emacs-devel@gnu.org
> Date: Sun, 16 Jan 2022 13:26:52 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Since this is glaringly inconsistent with the timings Alan published,
> 
> Did Alan benchmark byte compilation?

No.  But scrolling through a C file  invokes a lot of Lisp, so such
different results are surprising.

> > I think it would be a good idea to measure the difference for each of
> > the*.el files, because it could be that a couple of outliers skew the
> > entire picture.  That is, put the 'time' command inside the 'do', and
> > don't use -j8; then compare the outputs.
> 
> I'm not sure what that's supposed to show?

Whether the slowdown is uniform or not.  If it is not uniform, then it
is no longer certain that the time to recompile all the *.el files in
the Emacs source tree is a good representative of the effect of this
change on our users.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Eli.

On Sun, Jan 16, 2022 at 14:23:39 +0200, Eli Zaretskii wrote:
> > From: Lars Ingebrigtsen <larsi@gnus.org>
> > Date: Sun, 16 Jan 2022 13:02:34 +0100
> > Cc: Alan Mackenzie <acm@muc.de>, emacs-devel@gnu.org

> > Lars Ingebrigtsen <larsi@gnus.org> writes:

> > > And a "time make -j8 bootstrap check" takes

> > A better benchmark is something like the following, since we're
> > interested in byte compilation speed and not the rest:

> > time for i in `seq 1 10`; do rm lisp/gnus/*.elc lisp/org/*.elc; make -j8; done

> > This is 17% slower on my build machine on the branch compared to master.

> Since this is glaringly inconsistent with the timings Alan published,
> I think it would be a good idea to measure the difference for each of
> the*.el files, because it could be that a couple of outliers skew the
> entire picture.  That is, put the 'time' command inside the 'do', and
> don't use -j8; then compare the outputs.

On the branch, the byte compiler is doing more work than on master.  My
timings from yesterday were about the speed of Emacs in daily use rather
than while compiling.

I'm surprised to see the difference at 17%, though.  My bootstrap on the
branch is just 7% - 8% slower than on master.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Lars Ingebrigtsen]

Alan Mackenzie <acm@muc.de> writes:

> I'm surprised to see the difference at 17%, though.  My bootstrap on the
> branch is just 7% - 8% slower than on master.

17% isn't bootstrap; it's just byte compilation.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sun, 16 Jan 2022 14:06:59 +0000
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > Since this is glaringly inconsistent with the timings Alan published,
> > I think it would be a good idea to measure the difference for each of
> > the*.el files, because it could be that a couple of outliers skew the
> > entire picture.  That is, put the 'time' command inside the 'do', and
> > don't use -j8; then compare the outputs.
> 
> On the branch, the byte compiler is doing more work than on master.  My
> timings from yesterday were about the speed of Emacs in daily use rather
> than while compiling.

Are you saying that the measured slowdown is specific to the
byte-compiler, and will not affect other uses of Lisp, or affect them
much less heavily?  Because the byte-compiler is  just another Lisp
program, it doesn't in general do anything an arbitrary Lisp program
won't do.

Can you point to the places in the byte-compiler that do significantly
more work on the branch?  Can you show a profile where this could be
seen quantitatively?

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Eli.

On Sun, Jan 16, 2022 at 16:21:54 +0200, Eli Zaretskii wrote:
> > Date: Sun, 16 Jan 2022 14:06:59 +0000
> > Cc: Lars Ingebrigtsen <larsi@gnus.org>, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > Since this is glaringly inconsistent with the timings Alan published,
> > > I think it would be a good idea to measure the difference for each of
> > > the*.el files, because it could be that a couple of outliers skew the
> > > entire picture.  That is, put the 'time' command inside the 'do', and
> > > don't use -j8; then compare the outputs.

> > On the branch, the byte compiler is doing more work than on master.  My
> > timings from yesterday were about the speed of Emacs in daily use rather
> > than while compiling.

> Are you saying that the measured slowdown is specific to the
> byte-compiler, and will not affect other uses of Lisp, or affect them
> much less heavily?

Yes.  The byte compile, as expected, is more slowed down than other
software.  On a quick measurement of compiling comp.el, I found a
slowdown of around 12%

> Because the byte-compiler is  just another Lisp program, it doesn't in
> general do anything an arbitrary Lisp program won't do.

It does.  It runs with symbols with position activated, so any operation
involving an EQ which doesn't match is going to be significantly slower.

> Can you point to the places in the byte-compiler that do significantly
> more work on the branch?

Every EQ operation.  With symbols-with-pos-enabled set, the EQ has
appreciably more work to do.  If the first binary comparison does not
match, it needs to check the possibilities of symbols with positions.

With symbols-with-pos-enabled set to nil, an EQ has only a little more
work to do (namely, checking symbols-with-pos-enabled and finding it
unset).

> Can you show a profile where this could be seen quantitatively?

I'm not sure I understand.  The slowdown in the byte compiler is
distributed throughout the Emacs C Code.  It doesn't happen at any
particular isolated place.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sun, 16 Jan 2022 14:45:28 +0000
> Cc: larsi@gnus.org, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > Are you saying that the measured slowdown is specific to the
> > byte-compiler, and will not affect other uses of Lisp, or affect them
> > much less heavily?
> 
> Yes.  The byte compile, as expected, is more slowed down than other
> software.  On a quick measurement of compiling comp.el, I found a
> slowdown of around 12%

That seems to be inconsistent with the 17% slowdown compiling the *.el
files that Lars measured.

> > Because the byte-compiler is  just another Lisp program, it doesn't in
> > general do anything an arbitrary Lisp program won't do.
> 
> It does.  It runs with symbols with position activated, so any operation
> involving an EQ which doesn't match is going to be significantly slower.

But EQ is not specific to the byte-compiler, is it?

> > Can you show a profile where this could be seen quantitatively?
> 
> I'm not sure I understand.  The slowdown in the byte compiler is
> distributed throughout the Emacs C Code.  It doesn't happen at any
> particular isolated place.

I thought the slowdown was in Lisp somewhere.  If it's in EQ, I'm not
sure I understand how come the byte-compiler's slowdown is so much
more significant than in other Lisp code.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Eli.

On Sun, Jan 16, 2022 at 17:04:15 +0200, Eli Zaretskii wrote:
> > Date: Sun, 16 Jan 2022 14:45:28 +0000
> > Cc: larsi@gnus.org, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > Are you saying that the measured slowdown is specific to the
> > > byte-compiler, and will not affect other uses of Lisp, or affect them
> > > much less heavily?

> > Yes.  The byte compile, as expected, is more slowed down than other
> > software.  On a quick measurement of compiling comp.el, I found a
> > slowdown of around 12%

> That seems to be inconsistent with the 17% slowdown compiling the *.el
> files that Lars measured.

Or measurement error.  The coding style is likely to be quite a bit
different between gnus and comp.el.

> > > Because the byte-compiler is  just another Lisp program, it doesn't in
> > > general do anything an arbitrary Lisp program won't do.

> > It does.  It runs with symbols with position activated, so any operation
> > involving an EQ which doesn't match is going to be significantly slower.

> But EQ is not specific to the byte-compiler, is it?

> > > Can you show a profile where this could be seen quantitatively?

> > I'm not sure I understand.  The slowdown in the byte compiler is
> > distributed throughout the Emacs C Code.  It doesn't happen at any
> > particular isolated place.

> I thought the slowdown was in Lisp somewhere.  If it's in EQ, I'm not
> sure I understand how come the byte-compiler's slowdown is so much
> more significant than in other Lisp code.

The specification of the macro lisp_h_EQ in lisp.h L370 starts off:

#define lisp_h_EQ(x, y) ((XLI ((x)) == XLI ((y)))       \
  || (symbols_with_pos_enabled    \
.....

If symbols_with_pos_enabled is currently false, EQ need never execute
more than these first two lines.  If the variable is true, potentially
each of the other three cases (each variable x and y can be with or
without a position, giving four matching possibilities) needs to be
checked individually, which is slow.

Essentially the same code is coded up as emit_EQ inside comp.c.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sun, 16 Jan 2022 15:26:44 +0000
> Cc: larsi@gnus.org, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > I thought the slowdown was in Lisp somewhere.  If it's in EQ, I'm not
> > sure I understand how come the byte-compiler's slowdown is so much
> > more significant than in other Lisp code.
> 
> The specification of the macro lisp_h_EQ in lisp.h L370 starts off:
> 
> #define lisp_h_EQ(x, y) ((XLI ((x)) == XLI ((y)))       \
>   || (symbols_with_pos_enabled    \
> .....
> 
> If symbols_with_pos_enabled is currently false, EQ need never execute
> more than these first two lines.  If the variable is true, potentially
> each of the other three cases (each variable x and y can be with or
> without a position, giving four matching possibilities) needs to be
> checked individually, which is slow.
> 
> Essentially the same code is coded up as emit_EQ inside comp.c.

Yes, but my question was why would this affect the byte-compiler more
than it affects any other arbitrary Lisp.  Are you saying that the
byte-compiler is such a more intensive user of EQ than other Lisp
programs?

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Eli.

On Sun, Jan 16, 2022 at 17:43:19 +0200, Eli Zaretskii wrote:
> > Date: Sun, 16 Jan 2022 15:26:44 +0000
> > Cc: larsi@gnus.org, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > I thought the slowdown was in Lisp somewhere.  If it's in EQ, I'm not
> > > sure I understand how come the byte-compiler's slowdown is so much
> > > more significant than in other Lisp code.

> > The specification of the macro lisp_h_EQ in lisp.h L370 starts off:

> > #define lisp_h_EQ(x, y) ((XLI ((x)) == XLI ((y)))       \
> >   || (symbols_with_pos_enabled    \
> > .....

> > If symbols_with_pos_enabled is currently false, EQ need never execute
> > more than these first two lines.  If the variable is true, potentially
> > each of the other three cases (each variable x and y can be with or
> > without a position, giving four matching possibilities) needs to be
> > checked individually, which is slow.

> > Essentially the same code is coded up as emit_EQ inside comp.c.

> Yes, but my question was why would this affect the byte-compiler more
> than it affects any other arbitrary Lisp.

It is because symbols_with_pos_enabled is bound to true inside the byte
compiler (and the native compiler), but nowhere else.

> Are you saying that the byte-compiler is such a more intensive user of
> EQ than other Lisp programs?

Sort of - it uses all ten lines of lisp_h_EQ, rather than just the first
two lines used by all the other code.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Lars.

On Sun, Jan 16, 2022 at 09:24:31 +0100, Lars Ingebrigtsen wrote:
> Eli Zaretskii <eliz@gnu.org> writes:

> >> So, I would propose to merge the branch into master now.  What do people
> >> (in particular Eli) say about this?

> > Fine with me if Lars agrees.

> I'm seeing one compilation warning on the branch:

> Compiler-macro error for cl-member: Handler: cl--compiler-macro-member
> (excessive-variable-binding)
> Eager macro-expansion failure: (excessive-variable-binding)

I don't see this any more (though I've seen plenty of the like in the
past two or three weeks).  Was that build a bootstrap build?

> And a "time make -j8 bootstrap check" takes

> real	3m31.160s

> while it's

> real	3m16.761s

> on master.  A 7% slowdown is rather significant.

I suspect there is more conditional branching happening in the test
suite than in "normal" Lisp code.

Also, the bootstrap is doing more work - it is keeping track of source
positions for correct warning messages, something master doesn't do.

To keep things in perspective, solving the recent bug #53164 took about
the same amount off the build time.

The way to reduce the bootstrap time is surely to build
lisp/emacs-lisp/comp.el with a byte-compiled compiler rather than the
interpreted source one.  This latter takes about 2½ minutes on my
machine.

> -- 
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Lars Ingebrigtsen]

Alan Mackenzie <acm@muc.de> writes:

>> Compiler-macro error for cl-member: Handler: cl--compiler-macro-member
>> (excessive-variable-binding)
>> Eager macro-expansion failure: (excessive-variable-binding)
>
> I don't see this any more (though I've seen plenty of the like in the
> past two or three weeks).  Was that build a bootstrap build?

Yup.

> The way to reduce the bootstrap time is surely to build
> lisp/emacs-lisp/comp.el with a byte-compiled compiler rather than the
> interpreted source one.  This latter takes about 2½ minutes on my
> machine.

This was a build without nativecomp, which doesn't compile comp.el, I
think?  But that seems like a separate issue -- I'm not all that worried
about the Emacs build time, but compilation is usually a nice benchmark,
because it exercises a lot of the Emacs machinery.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

> This was a build without nativecomp, which doesn't compile comp.el, I
> think?  But that seems like a separate issue -- I'm not all that worried
> about the Emacs build time, but compilation is usually a nice benchmark,
> because it exercises a lot of the Emacs machinery.

But you'd need to run the old compiler with the new code (or vice versa)
in order to compare apples with apples :-(


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

> And a "time make -j8 bootstrap check" takes
>
> real	3m31.160s
>
> while it's
>
> real	3m16.761s
>
> on master.  A 7% slowdown is rather significant.

It is to be expected that in order to improve the compiler's behavior,
the compiler runs more slowly.

The main worry is the speed impact on non-compilation tasks, mostly
because of the impact on EQ (where Alan's benchmark suggest a 1% impact,
tho it would be good to have other measurements to confirm that).


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> It is to be expected that in order to improve the compiler's behavior,
> the compiler runs more slowly.

Yup.  But the performance impact is greater than I had imagined.

> The main worry is the speed impact on non-compilation tasks, mostly
> because of the impact on EQ (where Alan's benchmark suggest a 1% impact,
> tho it would be good to have other measurements to confirm that).

Yes.  I've now done a few more realistic non-micro benchmarks --
(eww-open-file "/tmp/foo.html") -- and I see no measurable performance
impact there at all.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Lars.

On Sun, Jan 16, 2022 at 16:04:25 +0100, Lars Ingebrigtsen wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:

[ .... ]

> Yes.  I've now done a few more realistic non-micro benchmarks --
> (eww-open-file "/tmp/foo.html") -- and I see no measurable performance
> impact there at all.

Thanks!  That's a relief.

> -- 
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Mattias Engdegård]

16 jan. 2022 kl. 16.04 skrev Lars Ingebrigtsen <larsi@gnus.org>:

> Yes.  I've now done a few more realistic non-micro benchmarks --
> (eww-open-file "/tmp/foo.html") -- and I see no measurable performance
> impact there at all.

The time to run Relint on the Emacs tree is up about 3.0 ± 0.2 %. A lot of it is spent in reading files and in the regexp engine, neither of which use `eq` much.
It erases some (but not all) of the progress being made elsewhere.

Micro-benchmarking code using `eq` a lot shows an overhead of about 50 %.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Mattias.

On Sun, Jan 16, 2022 at 16:50:23 +0100, Mattias Engdegård wrote:
> 16 jan. 2022 kl. 16.04 skrev Lars Ingebrigtsen <larsi@gnus.org>:

> > Yes.  I've now done a few more realistic non-micro benchmarks --
> > (eww-open-file "/tmp/foo.html") -- and I see no measurable performance
> > impact there at all.

> The time to run Relint on the Emacs tree is up about 3.0 ± 0.2 %. A
> lot of it is spent in reading files and in the regexp engine, neither
> of which use `eq` much.  It erases some (but not all) of the progress
> being made elsewhere.

> Micro-benchmarking code using `eq` a lot shows an overhead of about 50 %.

Would you clarify, please.  50% of what?  The new branch, except whilst
compiling, sees EQ extended from two machine instructions to four.  In
place of

CMP  ; the two operands.
JE

, we now have

CMP  ; the two operands.
JE
TEST ; symbols-with-pos-enabled.
JNE

..  More or less.  So if the Lisp EQ instruction is going to be matching
half the time, there would be an overhead of 50% on just that
instruction.  Presumably quite a bit less on a whole program.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Mattias Engdegård]

16 jan. 2022 kl. 17.18 skrev Alan Mackenzie <acm@muc.de>:

> Would you clarify, please.  50% of what?

A micro-benchmark for measuring `eq` performance takes 50 % longer to run on scratch/correct-warning-pos than on master.

> So if the Lisp EQ instruction is going to be matching
> half the time, there would be an overhead of 50% on just that
> instruction.  Presumably quite a bit less on a whole program.

The point of the micro-benchmark was to measure `eq` since that is what has become more expensive, and we want to know how much. It uses `memq` as a way to reducing other factors, but obviously `memq` itself is very much of interest as well.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

> The point of the micro-benchmark was to measure `eq` since that is what has
> become more expensive, and we want to know how much. It uses `memq` as a way
> to reducing other factors, but obviously `memq` itself is very much of
> interest as well.

Note also that `memq` will tend to use EQ in ways that usually return
false, IOW in ways that exercise the slower code path more than the
fast path.


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Mattias Engdegård]

16 jan. 2022 kl. 18.13 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> Note also that `memq` will tend to use EQ in ways that usually return
> false, IOW in ways that exercise the slower code path more than the
> fast path.

Yes. Statistically most comparisons for equality fail, to the point that compilers often assume the failure branch as statically more likely.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Lars.

On Sun, Jan 16, 2022 at 16:04:25 +0100, Lars Ingebrigtsen wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > It is to be expected that in order to improve the compiler's behavior,
> > the compiler runs more slowly.

> Yup.  But the performance impact is greater than I had imagined.

> > The main worry is the speed impact on non-compilation tasks, mostly
> > because of the impact on EQ (where Alan's benchmark suggest a 1% impact,
> > tho it would be good to have other measurements to confirm that).

> Yes.  I've now done a few more realistic non-micro benchmarks --
> (eww-open-file "/tmp/foo.html") -- and I see no measurable performance
> impact there at all.

I've written some documentation about Symbols with Position, fixed a bug
reported by Brahimi Saifullah concerning flymake on .el files, and fixed
one or two other miscellaneous things.

I now propose to merge the branch into master.  Do you (or anybody else)
still have any issues with this?

Thanks!

> -- 
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Lars Ingebrigtsen]

Alan Mackenzie <acm@muc.de> writes:

> I now propose to merge the branch into master.  Do you (or anybody else)
> still have any issues with this?

I'd like to see the results from elisp-benchmarks with and without this
patch set, please.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Lars.

On Sat, Jan 22, 2022 at 15:30:43 +0100, Lars Ingebrigtsen wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > I now propose to merge the branch into master.  Do you (or anybody else)
> > still have any issues with this?

> I'd like to see the results from elisp-benchmarks with and without this
> patch set, please.

Certainly!

Each of the following two sets of results was obtained from my machine
starting emacs --no-desktop, and represents the second run of
elisp-benchmarks on each build.  The state of master was merged into the
branch this morning, and both builds were redone this morning, too.

Branch scratch/correct-warning-pos:

  | test               | non-gc avg (s) | gc avg (s) | gcs avg | tot avg (s) | tot avg err (s) |
  |--------------------+----------------+------------+---------+-------------+-----------------|
  | inclist            |           1.33 |       0.00 |       0 |        1.33 |            0.00 |
  | fibn-rec           |           0.00 |       0.00 |       0 |        0.00 |            0.00 |
  | listlen-tc         |           0.13 |       0.00 |       0 |        0.13 |            0.00 |
  | pack-unpack        |           0.14 |       0.12 |       4 |        0.26 |            0.00 |
  | pidigits           |           5.18 |       5.82 |     170 |       11.00 |            0.01 |
  | map-closure        |          10.98 |       0.00 |       0 |       10.98 |            0.01 |
  | nbody              |           1.98 |       0.24 |       1 |        2.22 |            0.01 |
  | dhrystone          |           2.92 |       0.00 |       0 |        2.92 |            0.02 |
  | pack-unpack-old    |           0.21 |       0.28 |       9 |        0.49 |            0.00 |
  | bubble             |           1.51 |       0.13 |       1 |        1.64 |            0.00 |
  | inclist-type-hints |           1.14 |       0.00 |       0 |        1.14 |            0.01 |
  | bubble-no-cons     |           2.42 |       0.03 |       1 |        2.46 |            0.02 |
  | fibn               |           0.00 |       0.00 |       0 |        0.00 |            0.00 |
  | fibn-tc            |           0.01 |       0.00 |       0 |        0.01 |            0.00 |
  | pcase              |           1.97 |       0.00 |       0 |        1.97 |            0.00 |
  | flet               |           4.27 |       0.00 |       0 |        4.27 |            0.00 |
  |--------------------+----------------+------------+---------+-------------+-----------------|
  | total              |          34.19 |       6.62 |     186 |       40.81 |            0.03 |


And master:

  | test               | non-gc avg (s) | gc avg (s) | gcs avg | tot avg (s) | tot avg err (s) |
  |--------------------+----------------+------------+---------+-------------+-----------------|
  | inclist            |           1.35 |       0.00 |       0 |        1.35 |            0.01 |
  | fibn-rec           |           0.00 |       0.00 |       0 |        0.00 |            0.00 |
  | listlen-tc         |           0.13 |       0.00 |       0 |        0.13 |            0.00 |
  | pack-unpack        |           0.14 |       0.14 |       5 |        0.28 |            0.00 |
  | pidigits           |           5.38 |       5.83 |     184 |       11.21 |            0.00 |
  | map-closure        |           9.69 |       0.00 |       0 |        9.69 |            0.00 |
  | nbody              |           1.86 |       0.24 |       1 |        2.09 |            0.01 |
  | dhrystone          |           2.91 |       0.00 |       0 |        2.91 |            0.01 |
  | pack-unpack-old    |           0.20 |       0.28 |      10 |        0.49 |            0.00 |
  | bubble             |           1.58 |       0.12 |       1 |        1.70 |            0.04 |
  | inclist-type-hints |           1.14 |       0.00 |       0 |        1.14 |            0.02 |
  | bubble-no-cons     |           2.34 |       0.03 |       1 |        2.37 |            0.00 |
  | fibn               |           0.00 |       0.00 |       0 |        0.00 |            0.00 |
  | fibn-tc            |           0.01 |       0.00 |       0 |        0.01 |            0.00 |
  | pcase              |           2.08 |       0.00 |       0 |        2.08 |            0.00 |
  | flet               |           4.37 |       0.00 |       0 |        4.37 |            0.01 |
  |--------------------+----------------+------------+---------+-------------+-----------------|
  | total              |          33.17 |       6.64 |     202 |       39.81 |            0.05 |


> -- 
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Lars Ingebrigtsen]

Alan Mackenzie <acm@muc.de> writes:

> Each of the following two sets of results was obtained from my machine
> starting emacs --no-desktop, and represents the second run of
> elisp-benchmarks on each build.  The state of master was merged into the
> branch this morning, and both builds were redone this morning, too.

Looks good to me, then, so unless Eli has any objections, merging the
branch is fine by me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  Eli Zaretskii
>  <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Sat, 22 Jan 2022 16:26:27 +0100
> 
> Alan Mackenzie <acm@muc.de> writes:
> 
> > Each of the following two sets of results was obtained from my machine
> > starting emacs --no-desktop, and represents the second run of
> > elisp-benchmarks on each build.  The state of master was merged into the
> > branch this morning, and both builds were redone this morning, too.
> 
> Looks good to me, then, so unless Eli has any objections, merging the
> branch is fine by me.

No objections.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Eli and Lars.

On Sat, Jan 22, 2022 at 17:36:20 +0200, Eli Zaretskii wrote:
> > From: Lars Ingebrigtsen <larsi@gnus.org>
> > Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  Eli Zaretskii
> >  <eliz@gnu.org>,  emacs-devel@gnu.org
> > Date: Sat, 22 Jan 2022 16:26:27 +0100

> > Alan Mackenzie <acm@muc.de> writes:

> > > Each of the following two sets of results was obtained from my machine
> > > starting emacs --no-desktop, and represents the second run of
> > > elisp-benchmarks on each build.  The state of master was merged into the
> > > branch this morning, and both builds were redone this morning, too.

> > Looks good to me, then, so unless Eli has any objections, merging the
> > branch is fine by me.

> No objections.

Many thanks, both of you.

I've now merged the branch into master, and closed the six merged bugs
about it.

Just a small point, Lars.  The change you made to
byte-compile-from-buffer earlier on today, I had to revert.  That
function now needs to call read-positioning-symbols.  But if
load-read-function gets bound to r-p-s, a function in a subsidiary place
gets symbols with position, and doesn't know what to do with them.  This
breaks the bootstrap.

Anyhow, I'm having a beer this evening!  This is the most difficult bug
I've ever fixed, anywhere.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Lars Ingebrigtsen]

Alan Mackenzie <acm@muc.de> writes:

> Just a small point, Lars.  The change you made to
> byte-compile-from-buffer earlier on today, I had to revert.

(I've now reverted the other parts of my change.)

> That function now needs to call read-positioning-symbols.  But if
> load-read-function gets bound to r-p-s, a function in a subsidiary
> place gets symbols with position, and doesn't know what to do with
> them.  This breaks the bootstrap.

I guess we'd have to introduce a new variable if we want to allow
customising the read function here, then.

> Anyhow, I'm having a beer this evening!  This is the most difficult bug
> I've ever fixed, anywhere.

Cheers!

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Mattias Engdegård]

22 jan. 2022 kl. 15.30 skrev Lars Ingebrigtsen <larsi@gnus.org>:

>> I now propose to merge the branch into master.  Do you (or anybody else)
>> still have any issues with this?
> 
> I'd like to see the results from elisp-benchmarks with and without this
> patch set, please.

In my experience, the elisp-benchmarks are very much not representative of actual elisp usage, and I have done quite some benchmarking for other reasons lately.

Unless Alan has improved scratch/correct-warning-pos significantly in the last few days, it still exhibits a penalty of about 3 % over master on common code that I've tried. Byte-compilation is even slower than that since it enables symbol positions.

It all depends a lot, of course -- obviously code that spends all its time in I/O, in GC or the regexp engine will see little difference.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>> I'd like to see the results from elisp-benchmarks with and without this 
>> patch set, please.
>
> In my experience, the elisp-benchmarks are very much not representative 
> of actual elisp usage, and I have done quite some benchmarking for other 
> reasons lately.
>

I don't know what benchmarks would be representative of actual elisp 
usage, but on my build machine both make bootstrap and make check are 
consistently ~8% slower on correct-warning-pos vs master:

make -j1 bootstrap: 649s vs 596s
make -j4 bootstrap: 257s vs 238s
make -j8 bootstrap: 198s vs 183s

make -j1 check: 178s vs 162s
make -j4 check: 44s vs 40s
make -j8 check: 26s vs 24s

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sat, 22 Jan 2022 17:02:43 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: Alan Mackenzie <acm@muc.de>, Lars Ingebrigtsen <larsi@gnus.org>,
>  emacs-devel@gnu.org, Eli Zaretskii <eliz@gnu.org>,
>  Stefan Monnier <monnier@iro.umontreal.ca>
> 
> 
> I don't know what benchmarks would be representative of actual elisp 
> usage, but on my build machine both make bootstrap and make check are 
> consistently ~8% slower on correct-warning-pos vs master:
> 
> make -j1 bootstrap: 649s vs 596s
> make -j4 bootstrap: 257s vs 238s
> make -j8 bootstrap: 198s vs 183s
> 
> make -j1 check: 178s vs 162s
> make -j4 check: 44s vs 40s
> make -j8 check: 26s vs 24s

That's consistent with what was reported at the beginning of this
thread (~7% slowdown).  Byte compilation is expected to be slightly
slower with this change.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>> I don't know what benchmarks would be representative of actual elisp 
>> usage, but on my build machine both make bootstrap and make check are 
>> consistently ~8% slower on correct-warning-pos vs master:
>>
>> make -j1 bootstrap: 649s vs 596s
>> make -j4 bootstrap: 257s vs 238s
>> make -j8 bootstrap: 198s vs 183s
>>
>> make -j1 check: 178s vs 162s
>> make -j4 check: 44s vs 40s
>> make -j8 check: 26s vs 24s
>
> That's consistent with what was reported at the beginning of this thread 
> (~7% slowdown).  Byte compilation is expected to be slightly slower with 
> this change.
>

But make check does a lot more than byte compilation, or am I missing 
something?

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sat, 22 Jan 2022 18:01:20 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: mattiase@acm.org, acm@muc.de, larsi@gnus.org, emacs-devel@gnu.org, 
>     monnier@iro.umontreal.ca
> 
> > That's consistent with what was reported at the beginning of this thread 
> > (~7% slowdown).  Byte compilation is expected to be slightly slower with 
> > this change.
> 
> But make check does a lot more than byte compilation, or am I missing 
> something?

Not much more, IME.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>>> That's consistent with what was reported at the beginning of this 
>>> thread (~7% slowdown).  Byte compilation is expected to be slightly 
>>> slower with this change.
>>
>> But make check does a lot more than byte compilation, or am I missing 
>> something?
>
> Not much more, IME.
>

That's not what the numbers tell us (again 3b33a14380 vs 7922131bb2):

make -j1 check: 181s (69s byte-compilation, 112s execution) vs 162s (58s byte-compilation, 104s execution)
make -j4 check: 45s (17s byte-compilation, 28s execution) vs 40s (15s byte-compilation, 25s execution)
make -j8 check: 26s (9s byte-compilation, 17s execution) vs 23s (7s byte-compilation, 16s execution)

In short, the compilation time in make check is slower (which is 
expected), but the execution time in make check is also consistently ~7% 
slower.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Gregory.

On Sat, Jan 22, 2022 at 22:36:11 +0000, Gregory Heytings wrote:

> >>> That's consistent with what was reported at the beginning of this 
> >>> thread (~7% slowdown).  Byte compilation is expected to be slightly 
> >>> slower with this change.

> >> But make check does a lot more than byte compilation, or am I missing 
> >> something?

> > Not much more, IME.


> That's not what the numbers tell us (again 3b33a14380 vs 7922131bb2):

> make -j1 check: 181s (69s byte-compilation, 112s execution) vs 162s (58s byte-compilation, 104s execution)
> make -j4 check: 45s (17s byte-compilation, 28s execution) vs 40s (15s byte-compilation, 25s execution)
> make -j8 check: 26s (9s byte-compilation, 17s execution) vs 23s (7s byte-compilation, 16s execution)

> In short, the compilation time in make check is slower (which is 
> expected), but the execution time in make check is also consistently ~7% 
> slower.

Yes.  But bug #22288 and friends are fixed.

I did some comparisons using elisp-benchmark earlier on today.  With the
bug fix, the timings were just 2½% slower.

Anything doing compilation (whether byte- or native-), or doing lots of
tight loops will be slower than that -2½%.

Other programs (such as CC Mode) which spend a lot of time in the regexp
engine, or garbage collection, or I/O, will have lost less than that
2½%.

But the main thing is that longstanding bugs have been fixed.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sat, 22 Jan 2022 22:36:11 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: mattiase@acm.org, larsi@gnus.org, acm@muc.de, monnier@iro.umontreal.ca,
>  emacs-devel@gnu.org
> 
> 
> >>> That's consistent with what was reported at the beginning of this 
> >>> thread (~7% slowdown).  Byte compilation is expected to be slightly 
> >>> slower with this change.
> >>
> >> But make check does a lot more than byte compilation, or am I missing 
> >> something?
> >
> > Not much more, IME.
> >
> 
> That's not what the numbers tell us (again 3b33a14380 vs 7922131bb2):
> 
> make -j1 check: 181s (69s byte-compilation, 112s execution) vs 162s (58s byte-compilation, 104s execution)
> make -j4 check: 45s (17s byte-compilation, 28s execution) vs 40s (15s byte-compilation, 25s execution)
> make -j8 check: 26s (9s byte-compilation, 17s execution) vs 23s (7s byte-compilation, 16s execution)
> 
> In short, the compilation time in make check is slower (which is 
> expected), but the execution time in make check is also consistently ~7% 
> slower.

How did you separate compilation times from execution times, and what
does "execution time" stand for -- is that elapsed time, CPU time,
something else?

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

[-- Attachment #1: Type: text/plain, Size: 815 bytes --]


>> That's not what the numbers tell us (again 3b33a14380 vs 7922131bb2):
>>
>> make -j1 check: 181s (69s byte-compilation, 112s execution) vs 162s (58s byte-compilation, 104s execution)
>> make -j4 check: 45s (17s byte-compilation, 28s execution) vs 40s (15s byte-compilation, 25s execution)
>> make -j8 check: 26s (9s byte-compilation, 17s execution) vs 23s (7s byte-compilation, 16s execution)
>>
>> In short, the compilation time in make check is slower (which is 
>> expected), but the execution time in make check is also consistently 
>> ~7% slower.
>
> How did you separate compilation times from execution times,
>

See the attached patch.

>
> and what does "execution time" stand for -- is that elapsed time, CPU 
> time, something else?
>

The first time printed by "time", that is, the wall clock time.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Add-a-target-to-byte-compile-all-tests-without-runni.patch --]
[-- Type: text/x-diff; name=Add-a-target-to-byte-compile-all-tests-without-runni.patch, Size: 1344 bytes --]

From 7ad5a5eb008315bf40061e5aad8a8d001787f2bf Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sun, 23 Jan 2022 21:45:21 +0000
Subject: [PATCH] Add a target to byte-compile all tests without running them

* test/Makefile.in: Add a check-byte-compile target.
* Makefile.in: Add the check-byte-compile target to CHECK_TARGETS.
---
 Makefile.in      | 2 +-
 test/Makefile.in | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/Makefile.in b/Makefile.in
index 8ac6f52746..e1acf13551 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -1014,7 +1014,7 @@ TAGS tags:
 	$(MAKE) -C doc/lispref tags
 	$(MAKE) -C doc/misc tags
 
-CHECK_TARGETS = check check-maybe check-expensive check-all
+CHECK_TARGETS = check check-maybe check-expensive check-all check-byte-compile
 .PHONY: $(CHECK_TARGETS)
 $(CHECK_TARGETS): all
 	$(MAKE) -C test $@
diff --git a/test/Makefile.in b/test/Makefile.in
index 9ad994e110..85e1e0c5e5 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -327,6 +327,9 @@ .PHONY:
 check-maybe: check-no-automated-subdir
 	@${MAKE} check-doit SELECTOR="${SELECTOR_ACTUAL}"
 
+check-byte-compile:
+	@${MAKE} $(ELFILES:.el=.elc)
+
 ## Run the tests.
 .PHONY: check-doit
 ## We can't put LOGFILES as prerequisites, because that would stop the
-- 
2.34.1

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sun, 23 Jan 2022 21:53:24 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: mattiase@acm.org, larsi@gnus.org, acm@muc.de, monnier@iro.umontreal.ca, 
>     emacs-devel@gnu.org
> 
> >> In short, the compilation time in make check is slower (which is 
> >> expected), but the execution time in make check is also consistently 
> >> ~7% slower.
> >
> > How did you separate compilation times from execution times,
> >
> 
> See the attached patch.
> 
> >
> > and what does "execution time" stand for -- is that elapsed time, CPU 
> > time, something else?
> >
> 
> The first time printed by "time", that is, the wall clock time.

If you are measuring wallclock time, I don't see how the overall
slowdown is a reliable measure of any code performance.  The test
suite includes quite a few tests that deliberately wait for some
seconds for various reasons, and those times aren't affected by the
byte code or the interpreter.

We need to measure CPU time.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>>> and what does "execution time" stand for -- is that elapsed time, CPU 
>>> time, something else?
>>
>> The first time printed by "time", that is, the wall clock time.
>
> If you are measuring wallclock time, I don't see how the overall 
> slowdown is a reliable measure of any code performance.  The test suite 
> includes quite a few tests that deliberately wait for some seconds for 
> various reasons, and those times aren't affected by the byte code or the 
> interpreter.
>

It is reliable, because the slowdown ratio doesn't change between -j1, -j4 
and -j8.

>
> We need to measure CPU time.
>

The results are even worse (again 7922131bb2 vs 3b33a14380):

make -j1 check: 153.7s (68.1s byte-compilation, 85.6s execution) vs 135.8s (58.1s byte-compilation, 77.7s execution)
make -j4 check: 151.0s (67.5s byte-compilation, 83.5s execution) vs 132.3s (57.3s byte-compilation, 75.0s execution)
make -j8 check: 149.2s (65.5s byte-compilation, 83.7s execution) vs 130.3s (55.8s byte-compilation, 74.5s execution)

These numbers are the average of 20 runs in sequence, on an unloaded 
up-to-date Debian bookworm machine.

In short: byte-compilation is ~17.5% slower, execution is ~11% slower. 
Nowhere near the "in the region of 1%" that was announced.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Mon, 24 Jan 2022 15:20:26 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: mattiase@acm.org, larsi@gnus.org, emacs-devel@gnu.org, 
>     monnier@iro.umontreal.ca, acm@muc.de
> 
> > We need to measure CPU time.
> >
> 
> The results are even worse (again 7922131bb2 vs 3b33a14380):

I didn't look for better results, I looked for results I can trust as
being the true measure of the slowdown.

Thanks.

> In short: byte-compilation is ~17.5% slower, execution is ~11% slower. 
> Nowhere near the "in the region of 1%" that was announced.

The 1% figure was with a different benchmark.  Each benchmark
exercises a different part of the byte-code interpreter.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>> In short: byte-compilation is ~17.5% slower, execution is ~11% slower. 
>> Nowhere near the "in the region of 1%" that was announced.
>
> The 1% figure was with a different benchmark.  Each benchmark exercises 
> a different part of the byte-code interpreter.
>

At least the test suite is not a micro-benchmark.  I exercises a large 
subset of Emacs features, and I don't see anything in these ~110K lines of 
Elisp that make them significantly different from other Elisp programs. 
My conclusion is that Emacs is now, on average, about 10% slower.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Mon, 24 Jan 2022 20:41:40 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: mattiase@acm.org, larsi@gnus.org, acm@muc.de, monnier@iro.umontreal.ca, 
>     emacs-devel@gnu.org
> 
> 
> >> In short: byte-compilation is ~17.5% slower, execution is ~11% slower. 
> >> Nowhere near the "in the region of 1%" that was announced.
> >
> > The 1% figure was with a different benchmark.  Each benchmark exercises 
> > a different part of the byte-code interpreter.
> >
> 
> At least the test suite is not a micro-benchmark.  I exercises a large 
> subset of Emacs features, and I don't see anything in these ~110K lines of 
> Elisp that make them significantly different from other Elisp programs. 
> My conclusion is that Emacs is now, on average, about 10% slower.

Your conclusion is premature.  While micro-benchmarks don't represent
a typical Emacs usage, neither are the tests in the test suite.  They
are run in batch mode, and the don't involve important Emacs
subsystems, such as the display engine.

So the actual slowdown in "normal" usage could be quite different from
what the test suite measures.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>> At least the test suite is not a micro-benchmark.  I exercises a large 
>> subset of Emacs features, and I don't see anything in these ~110K lines 
>> of Elisp that make them significantly different from other Elisp 
>> programs.  My conclusion is that Emacs is now, on average, about 10% 
>> slower.
>
> Your conclusion is premature.  While micro-benchmarks don't represent a 
> typical Emacs usage, neither are the tests in the test suite.  They are 
> run in batch mode, and the don't involve important Emacs subsystems, 
> such as the display engine.
>
> So the actual slowdown in "normal" usage could be quite different from 
> what the test suite measures.
>

I think you understood what I meant: bytecode execution in Emacs is now, 
on average, about 10% slower.  Are there other benchmarks (not 
micro-benchmarks) that could be used to reach what you would consider a 
mature conclusion?

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Gregory.

On Tue, Jan 25, 2022 at 08:59:00 +0000, Gregory Heytings wrote:

> >> At least the test suite is not a micro-benchmark.  I exercises a large 
> >> subset of Emacs features, and I don't see anything in these ~110K lines 
> >> of Elisp that make them significantly different from other Elisp 
> >> programs.  My conclusion is that Emacs is now, on average, about 10% 
> >> slower.

> > Your conclusion is premature.  While micro-benchmarks don't represent a 
> > typical Emacs usage, neither are the tests in the test suite.  They are 
> > run in batch mode, and the don't involve important Emacs subsystems, 
> > such as the display engine.

> > So the actual slowdown in "normal" usage could be quite different from 
> > what the test suite measures.


> I think you understood what I meant: bytecode execution in Emacs is now, 
> on average, about 10% slower.

The C level EQ operation takes twice as long when the operands do not
match, and the same time as before when they do match.  When one of the
operands is a literal nil, there is no increase in execution time.

The above doesn't apply to compilation, whether byte or native, where
the slowdown in the EQ operation is more marked.

> Are there other benchmarks (not micro-benchmarks) that could be used
> to reach what you would consider a mature conclusion?

There is Andrea Corallo's elisp-benchmark package.

You could try the following, in conjunction with benchmark-run.  It
scrolls through an entire buffer:

(defun time-scroll-b (&optional arg)
  (condition-case nil
      (while t
        (if arg (scroll-down) (scroll-up))
        (sit-for 0))
    (error nil)))

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

> The C level EQ operation takes twice as long when the operands do not
> match, and the same time as before when they do match.

I highly doubt reality is that simple because CPU performance is a lot
more complex than just counting instructions.

> When one of the operands is a literal nil, there is no increase in
> execution time.

That's an oversimplification as well: it's only when we use the `NILP`
macro that the extra cost is avoided.  Any use of `EQ` is still slowed
down when it happens to receive a nil as one of the arguments.

I do find the slowdowns discussed here rather worrisome.
I thought the original agreement was that it was OK to install this
change if the slowdown could be brought down to about 1% or below (for
the non-compilation case).

More importantly, I wonder how slowing down EQ by a factor of 2 can end
up costing 10% of runtime when running the test suite.  I think this
deserves investigation.


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Stefan.

On Tue, Jan 25, 2022 at 08:27:55 -0500, Stefan Monnier wrote:
> > The C level EQ operation takes twice as long when the operands do not
> > match, and the same time as before when they do match.

> I highly doubt reality is that simple because CPU performance is a lot
> more complex than just counting instructions.

OK, but as a first estimate, it should do as an explanation.

> > When one of the operands is a literal nil, there is no increase in
> > execution time.

> That's an oversimplification as well: it's only when we use the `NILP`
> macro that the extra cost is avoided.  Any use of `EQ` is still slowed
> down when it happens to receive a nil as one of the arguments.

I think, but I'm not quite sure, that a literal nil gets handled by
BASE_EQ rather than EQ, or this might depend on it getting native
compiled.

> I do find the slowdowns discussed here rather worrisome.
> I thought the original agreement was that it was OK to install this
> change if the slowdown could be brought down to about 1% or below (for
> the non-compilation case).

> More importantly, I wonder how slowing down EQ by a factor of 2 can end
> up costing 10% of runtime when running the test suite.  I think this
> deserves investigation.

Maybe it's because a lot of the time spent by make check is spent in
compilation, whether byte or native.  Compilation _is_ slower, by quite
a bit.

When I ran elisp-benchmark on the before and after versions, the change
was 2½% (on a native compiled Emacs).

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

>> That's an oversimplification as well: it's only when we use the `NILP`
>> macro that the extra cost is avoided.  Any use of `EQ` is still slowed
>> down when it happens to receive a nil as one of the arguments.
> I think, but I'm not quite sure, that a literal nil gets handled by
> BASE_EQ rather than EQ, or this might depend on it getting native
> compiled.

I don't think that's the case of C code that says

    EQ (Qnil, ...)

maybe it is the case for ELisp code that says (eq nil ...) and which
gets native compiled.  But I suspect it's a very small proportion of
the uses of EQ/eq, in any case.

Of course there shouldn't be very little code that says `EQ (Qnil, ...)`
because such code should use `NILP` instead (and *that* code indeed
doesn't suffer from the slowdown).

>> I do find the slowdowns discussed here rather worrisome.
>> I thought the original agreement was that it was OK to install this
>> change if the slowdown could be brought down to about 1% or below (for
>> the non-compilation case).
>> More importantly, I wonder how slowing down EQ by a factor of 2 can end
>> up costing 10% of runtime when running the test suite.  I think this
>> deserves investigation.
> Maybe it's because a lot of the time spent by make check is spent in
> compilation, whether byte or native.  Compilation _is_ slower, by quite
> a bit.

No, we are talking about the execution time:

Gregory Heytings wrote:
   In short: byte-compilation is ~17.5% slower, execution is ~11%
   slower. Nowhere near the "in the region of 1%" that was announced.

> When I ran elisp-benchmark on the before and after versions, the change
> was 2½% (on a native compiled Emacs).

So, something changes the cost from 2-3% to 11%.  Maybe it's native
compilation (tho I don't know if Gregory ran these with native
compilation or not), or maybe it's somewhere in the nature of the code
in the test suite, or ...

Mind you, I consider 2½% to be already quite different from "about 1%",
but I think we should first focus on those 11% reports because I don't
think I'm willing to slow down all execution by 10% just to get better
position info in the compilation warnings.


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Stefan.

On Tue, Jan 25, 2022 at 14:26:31 -0500, Stefan Monnier wrote:

[ .... ]

> >> I do find the slowdowns discussed here rather worrisome.
> >> I thought the original agreement was that it was OK to install this
> >> change if the slowdown could be brought down to about 1% or below (for
> >> the non-compilation case).
> >> More importantly, I wonder how slowing down EQ by a factor of 2 can end
> >> up costing 10% of runtime when running the test suite.  I think this
> >> deserves investigation.
> > Maybe it's because a lot of the time spent by make check is spent in
> > compilation, whether byte or native.  Compilation _is_ slower, by quite
> > a bit.

> No, we are talking about the execution time:

I've just tried timing $ make -j17 check on an up to date master, and a
master two - four weeks old, both configured the same, with native
compilation.  Much of the run time was taken by native compilation.

The two times were 42.966s and 48.547s.  That's a difference of just
under 13%.  Not a systematic comparison, since make check may have got
bigger in the last few weeks.

> Gregory Heytings wrote:
>    In short: byte-compilation is ~17.5% slower, execution is ~11%
>    slower. Nowhere near the "in the region of 1%" that was announced.

I don't think Gregory has ever been specific about what precisely he has
timed, and how.  I have.  And the 1% figure was for a specific timing,
namely scrolling through xdisp.c from start to end, fontifying as we go.

> > When I ran elisp-benchmark on the before and after versions, the change
> > was 2½% (on a native compiled Emacs).

> So, something changes the cost from 2-3% to 11%.  Maybe it's native
> compilation (tho I don't know if Gregory ran these with native
> compilation or not), or maybe it's somewhere in the nature of the code
> in the test suite, or ...

.... or all of these things.  I suspect it's mainly the increased cost of
the compilation.

> Mind you, I consider 2½% to be already quite different from "about
> 1%", ...

Really?  You have a batch job which you were expecting to take a minute.
Now, instead, it takes 61 seconds.  What would you have done with that
extra second, which is now no longer in your life?

> but I think we should first focus on those 11% reports because I don't
> think I'm willing to slow down all execution by 10% just to get better
> position info in the compilation warnings.

We're not talking about "better" position info.  We're talking about
correct versus incorrect; functional versus buggy.  Compilation has got
slower because it's no longer skimping on an essential portion of its
task.

You are taking up the emotional element of Gregory's posts.  There is no
"all" in the 10% slow down.  That is a measure of the slowdown of $ make
check, nothing else.  The time to bootstrap is about 7% - 8% slower
after the bug fix.  But nobody else cared enough about the boostrap time
to bother putting in the use of the byte-compiled compiler in early
bootstrap until I did a week or so ago.  Bootstrap has become steadily
slower over the months as features have been added.  Does anybody really
care?  If you scroll through C Mode code fontifying it, the fixed Emacs
is about 1% slower.  If you run the elisp-benchmarks, it's about 2½%
slower.

If we were talking about the output of a chemical factory, a 2½%
reduction in output would probably be serious.  But we're not, we're
talking about a use case where the computer's waiting for the next key
stroke nearly all the time anyway.

But do some comparative timings.  I think you'll find the typical loss
in performance is a good deal less than 11%.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>
> I've just tried timing $ make -j17 check on an up to date master, and a 
> master two - four weeks old, both configured the same, with native 
> compilation.  Much of the run time was taken by native compilation.
>
> The two times were 42.966s and 48.547s.  That's a difference of just 
> under 13%.  Not a systematic comparison, since make check may have got 
> bigger in the last few weeks.
>

Which is consistent with the results I reported: about 40% of the time of 
make check is used for byte-compilation, which is slowed down by ~17.5%, 
and the rest is execution, which is slowed down by ~11%.

>
> I don't think Gregory has ever been specific about what precisely he has 
> timed, and how.
>

Of course I have.  I timed make check, which everyone can do, and which 
you just did.

>
> There is no "all" in the 10% slow down.  That is a measure of the 
> slowdown of $ make check, nothing else.
>

It's the time to execute ~110K lines of Elisp, exercising various parts of 
Emacs.  It's IMO a much more significant number than an ad-hoc 
micro-benchmark.  It's perhaps not the only one to take into account, but 
it cannot be ignored either.

>
> But nobody else cared enough about the boostrap time to bother putting 
> in the use of the byte-compiled compiler in early bootstrap until I did 
> a week or so ago.
>

You may have seen that this optimization has no effect without 
--with-native-compilation.  This may explain that.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Gregory.

On Tue, Jan 25, 2022 at 21:27:25 +0000, Gregory Heytings wrote:

> > I've just tried timing $ make -j17 check on an up to date master, and a 
> > master two - four weeks old, both configured the same, with native 
> > compilation.  Much of the run time was taken by native compilation.

> > The two times were 42.966s and 48.547s.  That's a difference of just 
> > under 13%.  Not a systematic comparison, since make check may have got 
> > bigger in the last few weeks.

> Which is consistent with the results I reported: about 40% of the time of 
> make check is used for byte-compilation, which is slowed down by ~17.5%, 
> and the rest is execution, which is slowed down by ~11%.

What you reported was more educated guesses than results.  Why don't you
actually _measure_ some hopefully typical Emacs use, and tell us exactly
how you got these measurements.  Start off saying how you configured your
build, followed by calling emacs -Q, with all the other detailed steps
needed to duplicate your measurements.

So far as I am aware, there have been just two comparative measurements
done which don't involve compilation and aren't what you call "micro
benchmarks".  One was my timing scrolling through a C buffer.  That gave
a slowdown of ~1%.  The other was Lars's timing, in his own words:

    "Yes.  I've now done a few more realistic non-micro benchmarks --
    (eww-open-file "/tmp/foo.html") -- and I see no measurable
    performance impact there at all."

..  So the only benchmarks showing a > 1% slowdown are those involving
compilation, and the "micro benchmarks".

[ .... ]

> > There is no "all" in the 10% slow down.  That is a measure of the 
> > slowdown of $ make check, nothing else.

> It's the time to execute ~110K lines of Elisp, exercising various parts of 
> Emacs.  It's IMO a much more significant number than an ad-hoc 
> micro-benchmark.  It's perhaps not the only one to take into account, but 
> it cannot be ignored either.

But as pointed out above, it's not a measured time difference, it's an
estimated one, complete with assumptions which you're probably not
totally aware of.  Would you please present us with some _measurements_.
I think we're all agreed we accept the slowdown in compilation as a cost
of actually doing the job right.  So timings involving compilation aren't
of all that much interest.

> > But nobody else cared enough about the boostrap time to bother putting 
> > in the use of the byte-compiled compiler in early bootstrap until I did 
> > a week or so ago.

> You may have seen that this optimization has no effect without 
> --with-native-compilation.  This may explain that.

The bootstrap time without native compilation is much shorter in any
case.  The point I'm making is that people are working themselves up
into a frenzy about these small slowdowns, yet when they could have
calmly done something about slow bootstrapping didn't really bother much.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]


>
> What you reported was more educated guesses than results.
>

No, these are results.

>
> Why don't you actually _measure_ some hopefully typical Emacs use, and 
> tell us exactly how you got these measurements.  Start off saying how 
> you configured your build, followed by calling emacs -Q, with all the 
> other detailed steps needed to duplicate your measurements.
>

This information is all upthread, but in case you missed it, I'll repeat 
it here.  I built Emacs with its standard configuration options (IOW I 
just typed "make" in a fresh clone of the repository), on an up-to-date 
and unloaded Debian bookworm machine.  To split the time of make check 
between compilation and execution, I used the attached patch (which I 
already sent upthread).

>> You may have seen that this optimization has no effect without 
>> --with-native-compilation.  This may explain that.
>
> The bootstrap time without native compilation is much shorter in any 
> case.
>

This is an unrelated question, but no, it is not.  With just "make" (IOW 
without --with-native-compilation) I do not see a significant difference 
between 10083e788f and 808917b3fc.

[-- Attachment #2: Add-a-target-to-byte-compile-all-tests-without-runni.patch --]
[-- Type: text/x-diff, Size: 1344 bytes --]

From 7ad5a5eb008315bf40061e5aad8a8d001787f2bf Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sun, 23 Jan 2022 21:45:21 +0000
Subject: [PATCH] Add a target to byte-compile all tests without running them

* test/Makefile.in: Add a check-byte-compile target.
* Makefile.in: Add the check-byte-compile target to CHECK_TARGETS.
---
 Makefile.in      | 2 +-
 test/Makefile.in | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/Makefile.in b/Makefile.in
index 8ac6f52746..e1acf13551 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -1014,7 +1014,7 @@ TAGS tags:
 	$(MAKE) -C doc/lispref tags
 	$(MAKE) -C doc/misc tags
 
-CHECK_TARGETS = check check-maybe check-expensive check-all
+CHECK_TARGETS = check check-maybe check-expensive check-all check-byte-compile
 .PHONY: $(CHECK_TARGETS)
 $(CHECK_TARGETS): all
 	$(MAKE) -C test $@
diff --git a/test/Makefile.in b/test/Makefile.in
index 9ad994e110..85e1e0c5e5 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -327,6 +327,9 @@ .PHONY:
 check-maybe: check-no-automated-subdir
 	@${MAKE} check-doit SELECTOR="${SELECTOR_ACTUAL}"
 
+check-byte-compile:
+	@${MAKE} $(ELFILES:.el=.elc)
+
 ## Run the tests.
 .PHONY: check-doit
 ## We can't put LOGFILES as prerequisites, because that would stop the
-- 
2.34.1

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Gregory.

On Wed, Jan 26, 2022 at 18:59:39 +0000, Gregory Heytings wrote:


> > What you reported was more educated guesses than results.


> No, these are results.


> > Why don't you actually _measure_ some hopefully typical Emacs use, and 
> > tell us exactly how you got these measurements.  Start off saying how 
> > you configured your build, followed by calling emacs -Q, with all the 
> > other detailed steps needed to duplicate your measurements.


> This information is all upthread, but in case you missed it, I'll repeat 
> it here.  I built Emacs with its standard configuration options (IOW I 
> just typed "make" in a fresh clone of the repository), on an up-to-date 
> and unloaded Debian bookworm machine.  To split the time of make check 
> between compilation and execution, I used the attached patch (which I 
> already sent upthread).

Apologies, I had missed it.

> >> You may have seen that this optimization has no effect without 
> >> --with-native-compilation.  This may explain that.

> > The bootstrap time without native compilation is much shorter in any 
> > case.


> This is an unrelated question, but no, it is not.  With just "make" (IOW 
> without --with-native-compilation) I do not see a significant difference 
> between 10083e788f and 808917b3fc.

Mistake of ambiguity on my part.  What I meant was that the bootstrap
time without NC is much shorter than that with NC.

So, the picture we're seeing is that "real world" timings are showing the
new master ~1% - 3.2% slower than the old, with building and test suites
in the low to mid teens of percent slower.

> From 7ad5a5eb008315bf40061e5aad8a8d001787f2bf Mon Sep 17 00:00:00 2001
> From: Gregory Heytings <gregory@heytings.org>
> Date: Sun, 23 Jan 2022 21:45:21 +0000
> Subject: [PATCH] Add a target to byte-compile all tests without running them

> * test/Makefile.in: Add a check-byte-compile target.
> * Makefile.in: Add the check-byte-compile target to CHECK_TARGETS.
> ---
>  Makefile.in      | 2 +-
>  test/Makefile.in | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)

> diff --git a/Makefile.in b/Makefile.in
> index 8ac6f52746..e1acf13551 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -1014,7 +1014,7 @@ TAGS tags:
>  	$(MAKE) -C doc/lispref tags
>  	$(MAKE) -C doc/misc tags

> -CHECK_TARGETS = check check-maybe check-expensive check-all
> +CHECK_TARGETS = check check-maybe check-expensive check-all check-byte-compile
>  .PHONY: $(CHECK_TARGETS)
>  $(CHECK_TARGETS): all
>  	$(MAKE) -C test $@
> diff --git a/test/Makefile.in b/test/Makefile.in
> index 9ad994e110..85e1e0c5e5 100644
> --- a/test/Makefile.in
> +++ b/test/Makefile.in
> @@ -327,6 +327,9 @@ .PHONY:
>  check-maybe: check-no-automated-subdir
>  	@${MAKE} check-doit SELECTOR="${SELECTOR_ACTUAL}"

> +check-byte-compile:
> +	@${MAKE} $(ELFILES:.el=.elc)
> +
>  ## Run the tests.
>  .PHONY: check-doit
>  ## We can't put LOGFILES as prerequisites, because that would stop the
> -- 
> 2.34.1

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

> I've just tried timing $ make -j17 check on an up to date master, and a
> master two - four weeks old, both configured the same, with native
> compilation.  Much of the run time was taken by native compilation.
>
> The two times were 42.966s and 48.547s.  That's a difference of just
> under 13%.  Not a systematic comparison, since make check may have got
> bigger in the last few weeks.

But that includes compilation time, right?  So that's not the number I'm
worried about.

> And the 1% figure was for a specific timing, namely scrolling through
> xdisp.c from start to end, fontifying as we go.

The 1% I refer to is the goal that Eli set as being acceptable.  I can't
find that email right now, so I'm not sure how specific he was, but to
me it clearly applied to something wider than just that
scrolling benchmark.

>> > When I ran elisp-benchmark on the before and after versions, the change
>> > was 2½% (on a native compiled Emacs).
>
>> So, something changes the cost from 2-3% to 11%.  Maybe it's native
>> compilation (tho I don't know if Gregory ran these with native
>> compilation or not), or maybe it's somewhere in the nature of the code
>> in the test suite, or ...
>
> .... or all of these things.  I suspect it's mainly the increased cost of
> the compilation.

As the person responsible for the patch, I think it's your
responsibility to dig into the report and either "debunk it" or figure
out what is the cause for it, rather than sit back and wave it off as "I
suspect it's <foo>".

>> Mind you, I consider 2½% to be already quite different from "about
>> 1%", ...
> Really?

Speeding up execution by 1% is not super easy, so yes a general slowdown
of 1% is quite different from a general slowdown of 2½%.

>> but I think we should first focus on those 11% reports because I don't
>> think I'm willing to slow down all execution by 10% just to get better
>> position info in the compilation warnings.
> We're not talking about "better" position info.

This is a matter of opinion.  Don't get me wrong, I'm quite happy to
have better positions, but we've lived for many many years with poor
(and even non-existing) position info, and the new info is often
correct, indeed, but (inevitably) not always.

> Compilation has got slower because it's no longer skimping on an
> essential portion of its task.

I am not bothered by the slower compilation.  I'm worried about the
performance impact on execution of code unrelated to compilation.

> You are taking up the emotional element of Gregory's posts.
> There is no "all" in the 10% slow down.

I'd hope you know me better by now.  I do understand that Gregory's
number is just one number, thank you very much.  Please re-read what
I wrote, because AFAIK I have always been very clear about that.
It still remains a worrisome data point.

> But we're not, we're talking about a use case where the computer's
> waiting for the next key stroke nearly all the time anyway.

A lot of Emacs's code falls into this category, yes.  But enough falls
in another category that there's been a lot of enthusiasm over the years
for speeding up execution.


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Óscar Fuentes]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Speeding up execution by 1% is not super easy, so yes a general slowdown
> of 1% is quite different from a general slowdown of 2½%.

I didn't follow this thread so apologies in advance if I'm off-base, but
I'm wondering what would happen if somebody comes with a patch that
makes Emacs run X% faster with the side effect of making dianostics
inaccurate.

Because if such a patch is acceptable, maybe there are some easy ways
for speeding up Emacs ;-)

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Po Lu]

Óscar Fuentes <ofv@wanadoo.es> writes:

> I didn't follow this thread so apologies in advance if I'm off-base, but
> I'm wondering what would happen if somebody comes with a patch that
> makes Emacs run X% faster with the side effect of making dianostics
> inaccurate.

I think bikeshedding is over a claim that the new feature (which finally
makes byte compiler warnings accurate) slows down Emacs by a dozen or so
percent.

Thanks.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[chad]

[-- Attachment #1: Type: text/plain, Size: 4564 bytes --]

Since changing careers a long while back, I don't get much in-depth hacking
done anymore, but in my current situation, I do spend a fair bit of time
collaborating remotely across time zones and specialties (especially
creative,  analytic, and experience-focused people). I think maybe I can
help here; apologies if not.


On Tue, Jan 25, 2022 at 8:09 someone wrote:

> I think bikeshedding is over a claim that the new feature (which finally
> makes byte compiler warnings accurate) slows down Emacs by a dozen or so
> percent.
>

I don't understand at all what this message is trying to say, and I'll
wager that I'm not alone. (I've removed the name, because this isn't about
a person; it's about the discussion.)

There is a proposal being considered that improves emacs by adding a
long-missing capability that _at least_ some people find potentially very
useful, even if the capability isn't 100%. (I understand from conversation
here that it will occasionally be wrong, but nearly always be right or very
close to right.) There is also a performance cost for this capability.

There is also some high-level agreement from the maintainers that the
capability will be seriously considered as long as the cost isn't too high,
and there are some *very rough* guidelines for that acceptable cost. This
is quite good -- it's basically impossible to predict these costs, and it's
difficult to even measure them after the fact, much less beforehand.
Additionally, it's entirely possible that the costs can be mitigated once
they are identified.

I think we all understand that the final decision will be made by the
maintainers, with input from the users and developers (with all of the
possible side-effects like maintaining a branch, fork, etc). Towards that
end, the best way to move forward (from what I can see) is twofold:

1.) identify the costs, including a reasonable level of detail
THEN
2.) determine, for various use-cases (users, situations, platforms, etc.)
whether than cost is too high or not.

Trying to jump to #2 is very common, natural, and in fact difficult for
humans to avoid, but it almost never helps, and it quite frequently hinders
the effort (I think there are clear examples of that here). This is,
unfortunately, even more likely to be true in the current emacs-devel
environment, where I claim there are clear issues of ego, tradition,
process, and personality that are likely (have already) to be become
entangled. This is common in the best of times, and it's very rare that I
talk to another human today who thinks that these are the best of times.

To that end, I suggest that people try to focus on step 1 above: identify
the costs, including when and where (i.e. platforms, circumstance, and
use-cases) where emacs is slower with this capability added.

We know that byte-compiling is slower, so just finding/noting that is
presumably done, but it seems there might be some questions about why (e.g.
the recent sub-thread about EQ(Qnil,...) vs NILp(). I'm sure there are more
measurements that can be done there, but perhaps it would be best to try to
analyze the hot paths in byte-compiling using the new capability, to see if
there are ways to mitigate it?

There is also concern that emacs is slower outside of byte-compiling, but
the reported numbers there are divergent, and it seems that we haven't even
established a vector (or set of vectors?) for comparison. This seems like a
pretty good first step. (Yes, benchmarking emacs is hard, but we don't need
a perfect approach for this. A few "good" metrics should do the job for
this capability.)

There is an outstanding question of whether or not to merge the capability
into the development head, to get more people involved in testing and
refinement. Again, this is a question for the maintainers. I will hazard a
guess that they would like to see a bit more forward motion of how to
measure the costs first, but I could be wrong. In either case, it seems
like there is enough interest that if someone were to put together some
candidate benchmarks, and there were reasonable buy-in that the candidates
were solid, we could probably get people to run some tests from the branch.
That would probably (I'm guessing here) be enough to make a clear call
between "needs more baking" and "we can try this on git master".

If someone does have ideas for such benchmarks, please suggest them (here,
the elpa package, etc.) and I'm sure we'll be able to get people to try
them in various contexts.

I hope that helps! If not, then I hope at least it gets us moving forward.
~Chad

[-- Attachment #2: Type: text/html, Size: 5361 bytes --]

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> From: chad <yandros@mit.edu>
> Date: Wed, 26 Jan 2022 11:56:30 -0500
> 
> There is an outstanding question of whether or not to merge the capability into the development head, to get
> more people involved in testing and refinement. Again, this is a question for the maintainers. I will hazard a
> guess that they would like to see a bit more forward motion of how to measure the costs first, but I could be
> wrong. In either case, it seems like there is enough interest that if someone were to put together some
> candidate benchmarks, and there were reasonable buy-in that the candidates were solid, we could probably
> get people to run some tests from the branch. That would probably (I'm guessing here) be enough to make a
> clear call between "needs more baking" and "we can try this on git master".

You may have missed the fact that the feature was already merged, and
is now on master.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[chad]

[-- Attachment #1: Type: text/plain, Size: 413 bytes --]

On Wed, Jan 26, 2022 at 12:38 PM Eli Zaretskii <eliz@gnu.org> wrote:

> You may have missed the fact that the feature was already merged, and
> is now on master.
>

I did, in fact! Thanks for pointing it out (my build of master is failing
comp-tests-ret-type-spec-55 at the moment). This is good news.

Does anyone have suggestions for good benchmark candidates to try across
platforms/configurations/etc?

~Chad

[-- Attachment #2: Type: text/html, Size: 782 bytes --]

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>
> I think bikeshedding
>

Bikeshedding, really?  "Futile investment of time and energy in discussion 
of marginal technical issues"?  Please read 
https://www.gnu.org/philosophy/kind-communication.en.html .  If Emacs' 
efficiency is not something that is important to you, that's fine, but 
it's not a reason to disregard those who consider that it is important.

>
> is over a claim
>

It's not a claim, it's an observation, which you can easily check (and 
possibly contradict) by yourself.

>
> that the new feature (which finally makes byte compiler warnings 
> accurate)
>

They are not accurate, they are _more_ accurate than earlier.  See 
Stefan's answer above.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

>> that the new feature (which finally makes byte compiler warnings accurate)
> They are not accurate, they are _more_ accurate than earlier.  See Stefan's
> answer above.

Just to clarify: IMO they are significantly more accurate, and
I really appreciate the improvement.  The problem is that most Emacs
users don't spend nearly as much time looking at ELisp's compiler
warnings, so imposing a 10% slowdown for them would be a steep price to
pay for the benefit of those who do look at those warnings.


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Gregory.

On Wed, Jan 26, 2022 at 18:46:40 +0000, Gregory Heytings wrote:


> > I think bikeshedding


> Bikeshedding, really?  "Futile investment of time and energy in discussion 
> of marginal technical issues"?  Please read 
> https://www.gnu.org/philosophy/kind-communication.en.html .  If Emacs' 
> efficiency is not something that is important to you, that's fine, but 
> it's not a reason to disregard those who consider that it is important.


> > is over a claim


> It's not a claim, it's an observation, which you can easily check (and 
> possibly contradict) by yourself.


> > that the new feature (which finally makes byte compiler warnings 
> > accurate)

It doesn't make the warning positions more "accurate".  That thinking is
a category error.  Warning positions are not "accurate to +- 0.2mm" or
anything like that.  They are either wholly correct or they are
incorrect.

> They are not accurate, they are _more_ accurate than earlier.  See 
> Stefan's answer above.

In the current master branch, the warning positions are not accurate, no
(see above), they are correct.

And if any are found to be incorrect, as has already happened, that is a
bug to fix like any other, and one which is now capable of being fixed
like any other, contrary to the previous situation.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>
> (tho I don't know if Gregory ran these with native compilation or not)
>

Without.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>
> More importantly, I wonder how slowing down EQ by a factor of 2 can end 
> up costing 10% of runtime when running the test suite.  I think this 
> deserves investigation.
>

Do you have an idea how this could be investigated?

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

Gregory Heytings [2022-01-25 21:38:24] wrote:
>> More importantly, I wonder how slowing down EQ by a factor of 2 can end up
>> costing 10% of runtime when running the test suite.  I think this
>> deserves investigation.
> Do you have an idea how this could be investigated?

Usually such slowdown don't come from everywhere at the same time.
So you need to "slice" our total time into subelements, and presumably
some of those elements show a higher slowdown, so you can focus on those
and start slicing them further.

As you get closer to the source, the slowdown should become
more marked.


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>>> More importantly, I wonder how slowing down EQ by a factor of 2 can 
>>> end up costing 10% of runtime when running the test suite.  I think 
>>> this deserves investigation.
>>
>> Do you have an idea how this could be investigated?
>
> Usually such slowdown don't come from everywhere at the same time. So 
> you need to "slice" our total time into subelements, and presumably some 
> of those elements show a higher slowdown, so you can focus on those and 
> start slicing them further.
>
> As you get closer to the source, the slowdown should become more marked.
>

Thanks a lot, I'll see what I can do.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

[-- Attachment #1: Type: text/plain, Size: 1775 bytes --]


>>> More importantly, I wonder how slowing down EQ by a factor of 2 can 
>>> end up costing 10% of runtime when running the test suite.  I think 
>>> this deserves investigation.
>>
>> Do you have an idea how this could be investigated?
>
> Usually such slowdown don't come from everywhere at the same time. So 
> you need to "slice" our total time into subelements, and presumably some 
> of those elements show a higher slowdown, so you can focus on those and 
> start slicing them further.
>
> As you get closer to the source, the slowdown should become more marked.
>

You were too optimistic.

I just finished a detailed analysis of the slowdown of the execution of 
Emacs' test suite, and it turns out that the slowdown is indeed spread 
over all tests.

I attach the detailed results for each of the 389 tests.  Each test has 
been executed 2000 (two thousand) times, again on an unloaded up-to-date 
Debian bookworm computer.

If you look closer at the results, you will see that the slowdown is 
actually worse than 10%.  The slowest of all tests (lisp/net/tramp-tests) 
is only 2% slower, because it uses external processes and is therefore not 
as much affected as other tests by the slowdown of bytecode execution 
itself.  If you remove that test from the calculations, you will see that 
the slowdown is actually 17%, that is, the same slowdown as that of byte 
compilation.

(FWIW, I was puzzled by the claim that byte compilation could be slowed 
down markedly, and that at the same time general bytecode execution would 
not.  Byte compilation does not do anything that is fundamentally 
different from general bytecode execution.)

My conclusion is that this merge should be backed out.  Its performance 
impact has not been properly resolved and assessed.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: slowdown.csv --]
[-- Type: text/csv; name=slowdown.csv, Size: 16076 bytes --]

Test,3b33a14380,7922131bb2,Slowdown
lib-src/emacsclient-tests,0.229,0.257,12%
lisp/abbrev-tests,0.086,0.098,14%
lisp/align-tests,0.126,0.135,7%
lisp/allout-tests,0.082,0.096,17%
lisp/allout-widgets-tests,0.084,0.103,23%
lisp/ansi-color-tests,0.080,0.096,20%
lisp/apropos-tests,0.082,0.095,16%
lisp/arc-mode-tests,0.074,0.093,26%
lisp/auth-source-pass-tests,0.117,0.139,19%
lisp/auth-source-tests,0.118,0.145,23%
lisp/autoinsert-tests,0.083,0.096,16%
lisp/autorevert-tests,1.813,3.868,113%
lisp/battery-tests,0.090,0.101,12%
lisp/bookmark-tests,0.234,0.253,8%
lisp/buff-menu-tests,0.072,0.089,24%
lisp/button-tests,0.081,0.094,16%
lisp/calc/calc-tests,0.290,0.295,2%
lisp/calculator-tests,0.080,0.094,18%
lisp/calendar/cal-french-tests,0.087,0.104,20%
lisp/calendar/cal-julian-tests,0.097,0.114,18%
lisp/calendar/icalendar-tests,0.580,0.611,5%
lisp/calendar/iso8601-tests,0.086,0.103,20%
lisp/calendar/lunar-tests,0.095,0.108,14%
lisp/calendar/parse-time-tests,0.076,0.096,26%
lisp/calendar/solar-tests,0.096,0.108,13%
lisp/calendar/time-date-tests,0.084,0.097,15%
lisp/calendar/todo-mode-tests,0.825,0.865,5%
lisp/cedet/cedet-files-tests,0.071,0.090,27%
lisp/cedet/semantic/bovine/gcc-tests,0.103,0.124,20%
lisp/cedet/semantic/format-tests,0.431,0.465,8%
lisp/cedet/semantic/fw-tests,0.090,0.112,24%
lisp/cedet/semantic-utest,0.792,0.850,7%
lisp/cedet/semantic-utest-c,0.102,0.126,24%
lisp/cedet/semantic-utest-ia,2.770,2.860,3%
lisp/cedet/srecode/document-tests,0.301,0.334,11%
lisp/cedet/srecode/fields-tests,0.087,0.106,22%
lisp/cedet/srecode-utest-getset,0.287,0.314,9%
lisp/cedet/srecode-utest-template,0.352,0.380,8%
lisp/char-fold-tests,0.125,0.142,14%
lisp/color-tests,0.083,0.096,16%
lisp/comint-tests,0.092,0.108,17%
lisp/completion-tests,0.081,0.093,15%
lisp/cus-edit-tests,0.142,0.159,12%
lisp/custom-tests,0.153,0.167,9%
lisp/dabbrev-tests,0.081,0.093,15%
lisp/delim-col-tests,0.074,0.094,27%
lisp/descr-text-tests,0.089,0.106,19%
lisp/dired-aux-tests,0.099,0.116,17%
lisp/dired-tests,0.220,0.243,10%
lisp/dired-x-tests,0.091,0.110,21%
lisp/dom-tests,0.081,0.094,16%
lisp/edmacro-tests,0.073,0.092,26%
lisp/electric-tests,1.157,1.207,4%
lisp/elide-head-tests,0.088,0.101,15%
lisp/emacs-lisp/backquote-tests,0.073,0.091,25%
lisp/emacs-lisp/backtrace-tests,0.102,0.117,15%
lisp/emacs-lisp/benchmark-tests,0.090,0.106,18%
lisp/emacs-lisp/bindat-tests,0.215,0.226,5%
lisp/emacs-lisp/bytecomp-tests,0.322,0.352,9%
lisp/emacs-lisp/cconv-tests,0.129,0.142,10%
lisp/emacs-lisp/check-declare-tests,0.087,0.100,15%
lisp/emacs-lisp/checkdoc-tests,0.101,0.118,17%
lisp/emacs-lisp/cl-extra-tests,0.075,0.092,23%
lisp/emacs-lisp/cl-generic-tests,0.095,0.112,18%
lisp/emacs-lisp/cl-lib-tests,0.098,0.114,16%
lisp/emacs-lisp/cl-macs-tests,0.112,0.128,14%
lisp/emacs-lisp/cl-preloaded-tests,0.073,0.091,25%
lisp/emacs-lisp/cl-print-tests,0.085,0.102,20%
lisp/emacs-lisp/cl-seq-tests,0.081,0.095,17%
lisp/emacs-lisp/comp-cstr-tests,0.156,0.182,17%
lisp/emacs-lisp/copyright-tests,0.074,0.093,26%
lisp/emacs-lisp/derived-tests,0.074,0.091,23%
lisp/emacs-lisp/easy-mmode-tests,0.129,0.151,17%
lisp/emacs-lisp/edebug-tests,0.452,0.485,7%
lisp/emacs-lisp/eieio-tests/eieio-test-methodinvoke,0.097,0.114,18%
lisp/emacs-lisp/eieio-tests/eieio-test-persist,0.101,0.117,16%
lisp/emacs-lisp/eieio-tests/eieio-tests,0.113,0.136,20%
lisp/emacs-lisp/ert-tests,2.697,2.716,1%
lisp/emacs-lisp/ert-x-tests,0.086,0.105,22%
lisp/emacs-lisp/faceup-tests/faceup-test-basics,0.098,0.112,14%
lisp/emacs-lisp/faceup-tests/faceup-test-files,0.076,0.095,25%
lisp/emacs-lisp/find-func-tests,0.351,0.392,12%
lisp/emacs-lisp/float-sup-tests,0.072,0.089,24%
lisp/emacs-lisp/generator-tests,0.104,0.126,21%
lisp/emacs-lisp/gv-tests,0.498,0.540,8%
lisp/emacs-lisp/hierarchy-tests,0.092,0.104,13%
lisp/emacs-lisp/let-alist-tests,0.073,0.094,29%
lisp/emacs-lisp/lisp-mnt-tests,0.087,0.100,15%
lisp/emacs-lisp/lisp-mode-tests,0.099,0.117,18%
lisp/emacs-lisp/lisp-tests,0.165,0.190,15%
lisp/emacs-lisp/macroexp-tests,0.274,0.298,9%
lisp/emacs-lisp/map-tests,0.087,0.104,20%
lisp/emacs-lisp/memory-report-tests,0.084,0.102,21%
lisp/emacs-lisp/multisession-tests,0.186,0.209,12%
lisp/emacs-lisp/nadvice-tests,0.101,0.118,17%
lisp/emacs-lisp/package-tests,1.679,1.769,5%
lisp/emacs-lisp/pcase-tests,0.086,0.102,19%
lisp/emacs-lisp/pp-tests,0.099,0.116,17%
lisp/emacs-lisp/range-tests,0.074,0.094,27%
lisp/emacs-lisp/regexp-opt-tests,0.073,0.092,26%
lisp/emacs-lisp/ring-tests,0.082,0.094,15%
lisp/emacs-lisp/rmc-tests,0.074,0.093,26%
lisp/emacs-lisp/rx-tests,0.085,0.097,14%
lisp/emacs-lisp/seq-tests,0.089,0.105,18%
lisp/emacs-lisp/shadow-tests,0.076,0.097,28%
lisp/emacs-lisp/shortdoc-tests,0.269,0.284,6%
lisp/emacs-lisp/subr-x-tests,0.130,0.141,8%
lisp/emacs-lisp/syntax-tests,0.083,0.102,23%
lisp/emacs-lisp/tabulated-list-tests,0.080,0.094,18%
lisp/emacs-lisp/testcover-tests,0.280,0.303,8%
lisp/emacs-lisp/text-property-search-tests,0.083,0.096,16%
lisp/emacs-lisp/thunk-tests,0.083,0.101,22%
lisp/emacs-lisp/timer-tests,0.073,0.092,26%
lisp/emacs-lisp/unsafep-tests,0.074,0.093,26%
lisp/emacs-lisp/warnings-tests,0.073,0.093,27%
lisp/emulation/viper-tests,0.131,0.147,12%
lisp/epg-config-tests,0.072,0.092,28%
lisp/epg-tests,0.086,0.104,21%
lisp/erc/erc-tests,0.134,0.156,16%
lisp/erc/erc-track-tests,0.118,0.139,18%
lisp/eshell/em-hist-tests,0.095,0.111,17%
lisp/eshell/em-ls-tests,0.233,0.257,10%
lisp/eshell/eshell-tests,0.168,0.187,11%
lisp/eshell/esh-opt-tests,0.085,0.097,14%
lisp/faces-tests,0.087,0.101,16%
lisp/ffap-tests,0.123,0.148,20%
lisp/filenotify-tests,0.563,1.094,94%
lisp/files-tests,1.024,1.586,55%
lisp/files-x-tests,0.135,0.158,17%
lisp/find-cmd-tests,0.087,0.104,20%
lisp/font-lock-tests,0.073,0.092,26%
lisp/format-spec-tests,0.083,0.094,13%
lisp/gnus/gnus-group-tests,0.179,0.203,13%
lisp/gnus/gnus-icalendar-tests,0.381,0.447,17%
lisp/gnus/gnus-search-tests,0.244,0.284,16%
lisp/gnus/gnus-test-headers,0.227,0.267,18%
lisp/gnus/gnus-tests,0.250,0.288,15%
lisp/gnus/gnus-util-tests,0.085,0.097,14%
lisp/gnus/message-tests,0.130,0.153,18%
lisp/gnus/mm-decode-tests,0.132,0.154,17%
lisp/gnus/mml-sec-tests,0.227,0.267,18%
lisp/gnus/nnrss-tests,0.147,0.178,21%
lisp/help-fns-tests,0.387,0.412,6%
lisp/help-mode-tests,0.120,0.138,15%
lisp/help-tests,0.098,0.116,18%
lisp/hfy-cmap-tests,0.074,0.094,27%
lisp/hi-lock-tests,0.082,0.094,15%
lisp/htmlfontify-tests,0.173,0.198,14%
lisp/ibuffer-tests,0.283,0.325,15%
lisp/ido-tests,0.084,0.096,14%
lisp/image-dired-tests,0.088,0.106,20%
lisp/image/exif-tests,0.074,0.093,26%
lisp/image-file-tests,0.073,0.093,27%
lisp/image/gravatar-tests,0.100,0.115,15%
lisp/image-tests,0.074,0.094,27%
lisp/imenu-tests,0.084,0.101,20%
lisp/info-tests,0.099,0.115,16%
lisp/info-xref-tests,0.311,0.330,6%
lisp/international/ccl-tests,0.084,0.097,15%
lisp/international/mule-tests,0.075,0.093,24%
lisp/international/mule-util-tests,0.093,0.116,25%
lisp/international/textsec-tests,0.243,0.267,10%
lisp/international/ucs-normalize-tests,0.400,0.422,5%
lisp/isearch-tests,0.073,0.091,25%
lisp/jit-lock-tests,0.074,0.092,24%
lisp/jsonrpc-tests,0.117,0.145,24%
lisp/json-tests,0.097,0.115,19%
lisp/kmacro-tests,0.123,0.140,14%
lisp/loadhist-tests,0.094,0.109,16%
lisp/lpr-tests,0.073,0.092,26%
lisp/ls-lisp-tests,0.144,0.164,14%
lisp/mail/flow-fill-tests,0.074,0.093,26%
lisp/mail/footnote-tests,0.075,0.094,25%
lisp/mail/mail-parse-tests,0.085,0.098,15%
lisp/mail/mail-utils-tests,0.085,0.096,13%
lisp/mail/qp-tests,0.073,0.091,25%
lisp/mail/rfc2045-tests,0.074,0.092,24%
lisp/mail/rfc2047-tests,0.078,0.097,24%
lisp/mail/rfc6068-tests,0.074,0.092,24%
lisp/mail/rfc822-tests,0.082,0.094,15%
lisp/mail/rmailmm-tests,0.148,0.180,22%
lisp/mail/rmail-tests,0.107,0.121,13%
lisp/mail/uudecode-tests,0.085,0.103,21%
lisp/makesum-tests,0.073,0.092,26%
lisp/man-tests,0.100,0.117,17%
lisp/md4-tests,0.073,0.093,27%
lisp/mh-e/mh-limit-tests,0.100,0.113,13%
lisp/mh-e/mh-thread-tests,0.102,0.121,19%
lisp/mh-e/mh-utils-tests,0.105,0.123,17%
lisp/mh-e/mh-xface-tests,0.115,0.138,20%
lisp/minibuffer-tests,0.085,0.096,13%
lisp/misc-tests,0.072,0.091,26%
lisp/mouse-tests,0.086,0.101,17%
lisp/mwheel-tests,0.072,0.092,28%
lisp/net/browse-url-tests,0.101,0.123,22%
lisp/net/dbus-tests,0.106,0.121,14%
lisp/net/dig-tests,0.073,0.093,27%
lisp/net/gnutls-tests,0.386,0.403,4%
lisp/net/hmac-md5-tests,0.074,0.092,24%
lisp/net/mailcap-tests,0.092,0.109,18%
lisp/net/netrc-tests,0.075,0.095,27%
lisp/net/network-stream-tests,0.113,0.129,14%
lisp/net/newsticker-tests,0.105,0.128,22%
lisp/net/nsm-tests,0.084,0.099,18%
lisp/net/ntlm-tests,0.105,0.127,21%
lisp/net/puny-tests,0.143,0.157,10%
lisp/net/rcirc-tests,0.099,0.116,17%
lisp/net/rfc2104-tests,0.073,0.092,26%
lisp/net/sasl-cram-tests,0.074,0.092,24%
lisp/net/sasl-scram-rfc-tests,0.249,0.263,6%
lisp/net/sasl-tests,0.073,0.092,26%
lisp/net/secrets-tests,0.090,0.100,11%
lisp/net/shr-tests,0.129,0.152,18%
lisp/net/socks-tests,0.117,0.139,19%
lisp/net/tramp-archive-tests,0.153,0.175,14%
lisp/net/tramp-tests,12.022,12.223,2%
lisp/net/webjump-tests,0.100,0.123,23%
lisp/newcomment-tests,0.114,0.130,14%
lisp/nxml/nxml-mode-tests,0.110,0.128,16%
lisp/nxml/xsd-regexp-tests,0.074,0.093,26%
lisp/obarray-tests,0.073,0.092,26%
lisp/obsolete/cl-tests,0.074,0.093,26%
lisp/obsolete/inversion-tests,0.073,0.093,27%
lisp/obsolete/rfc2368-tests,0.074,0.091,23%
lisp/org/org-tests,0.073,0.092,26%
lisp/paren-tests,0.073,0.092,26%
lisp/password-cache-tests,0.073,0.092,26%
lisp/pcmpl-linux-tests,0.087,0.102,17%
lisp/play/animate-tests,0.112,0.125,12%
lisp/play/cookie1-tests,0.075,0.095,27%
lisp/play/dissociate-tests,0.073,0.093,27%
lisp/play/fortune-tests,0.074,0.092,24%
lisp/play/life-tests,0.074,0.093,26%
lisp/play/morse-tests,0.073,0.092,26%
lisp/play/studly-tests,0.073,0.092,26%
lisp/progmodes/asm-mode-tests,0.074,0.093,26%
lisp/progmodes/autoconf-tests,0.073,0.093,27%
lisp/progmodes/bat-mode-tests,0.113,0.131,16%
lisp/progmodes/bug-reference-tests,0.074,0.092,24%
lisp/progmodes/cc-mode-tests,0.124,0.140,13%
lisp/progmodes/compile-tests,0.137,0.154,12%
lisp/progmodes/cperl-mode-tests,0.144,0.155,8%
lisp/progmodes/elisp-mode-tests,0.621,0.661,6%
lisp/progmodes/etags-tests,0.153,0.181,18%
lisp/progmodes/executable-tests,0.071,0.090,27%
lisp/progmodes/f90-tests,0.113,0.130,15%
lisp/progmodes/flymake-tests,0.484,0.509,5%
lisp/progmodes/gdb-mi-tests,0.099,0.115,16%
lisp/progmodes/glasses-tests,0.074,0.092,24%
lisp/progmodes/grep-tests,0.114,0.129,13%
lisp/progmodes/js-tests,0.130,0.152,17%
lisp/progmodes/octave-tests,0.086,0.104,21%
lisp/progmodes/opascal-tests,0.082,0.095,16%
lisp/progmodes/pascal-tests,0.073,0.093,27%
lisp/progmodes/perl-mode-tests,0.157,0.182,16%
lisp/progmodes/project-tests,0.126,0.143,13%
lisp/progmodes/ps-mode-tests,0.096,0.115,20%
lisp/progmodes/python-tests,0.307,0.334,9%
lisp/progmodes/ruby-mode-tests,0.130,0.147,13%
lisp/progmodes/scheme-tests,0.073,0.093,27%
lisp/progmodes/sh-script-tests,0.084,0.103,23%
lisp/progmodes/sql-tests,0.111,0.128,15%
lisp/progmodes/subword-tests,0.074,0.092,24%
lisp/progmodes/tcl-tests,0.086,0.103,20%
lisp/progmodes/xref-tests,0.171,0.197,15%
lisp/ps-print-tests,0.080,0.095,19%
lisp/register-tests,0.072,0.091,26%
lisp/repeat-tests,0.081,0.101,25%
lisp/replace-tests,0.098,0.121,23%
lisp/rot13-tests,0.072,0.090,25%
lisp/saveplace-tests,0.093,0.109,17%
lisp/scroll-lock-tests,0.074,0.094,27%
lisp/ses-tests,0.088,0.106,20%
lisp/shadowfile-tests,0.471,0.503,7%
lisp/shell-tests,0.085,0.104,22%
lisp/simple-tests,3.547,3.579,1%
lisp/so-long-tests/autoload-longlines-mode-tests,0.083,0.095,14%
lisp/so-long-tests/autoload-major-mode-tests,0.082,0.094,15%
lisp/so-long-tests/autoload-minor-mode-tests,0.074,0.094,27%
lisp/so-long-tests/so-long-tests,0.253,0.286,13%
lisp/so-long-tests/so-long-tests-helpers,0.074,0.094,27%
lisp/so-long-tests/spelling-tests,0.090,0.101,12%
lisp/sort-tests,0.128,0.146,14%
lisp/soundex-tests,0.073,0.092,26%
lisp/startup-tests,0.073,0.092,26%
lisp/subr-tests,0.119,0.130,9%
lisp/tab-bar-tests,0.082,0.094,15%
lisp/tabify-tests,0.073,0.092,26%
lisp/tar-mode-tests,0.104,0.121,16%
lisp/tempo-tests,0.080,0.093,16%
lisp/term-tests,0.120,0.138,15%
lisp/term/tty-colors-tests,0.071,0.090,27%
lisp/textmodes/bibtex-tests,0.084,0.102,21%
lisp/textmodes/conf-mode-tests,0.082,0.094,15%
lisp/textmodes/css-mode-tests,0.206,0.237,15%
lisp/textmodes/dns-mode-tests,0.074,0.093,26%
lisp/textmodes/fill-tests,0.072,0.091,26%
lisp/textmodes/mhtml-mode-tests,0.206,0.245,19%
lisp/textmodes/page-tests,0.073,0.093,27%
lisp/textmodes/paragraphs-tests,0.099,0.114,15%
lisp/textmodes/po-tests,0.073,0.091,25%
lisp/textmodes/reftex-tests,0.095,0.109,15%
lisp/textmodes/sgml-mode-tests,0.093,0.105,13%
lisp/textmodes/texinfo-tests,0.083,0.096,16%
lisp/textmodes/tildify-tests,0.126,0.149,18%
lisp/textmodes/underline-tests,0.072,0.092,28%
lisp/thingatpt-tests,0.154,0.171,11%
lisp/thread-tests,0.086,0.099,15%
lisp/thumbs-tests,0.085,0.104,22%
lisp/time-stamp-tests,0.128,0.146,14%
lisp/time-tests,0.085,0.097,14%
lisp/timezone-tests,0.074,0.094,27%
lisp/url/url-auth-tests,0.106,0.129,22%
lisp/url/url-domsuf-tests,0.073,0.094,29%
lisp/url/url-expand-tests,0.107,0.130,21%
lisp/url/url-file-tests,0.105,0.128,22%
lisp/url/url-future-tests,0.073,0.092,26%
lisp/url/url-handlers-tests,0.138,0.159,15%
lisp/url/url-misc-tests,0.092,0.115,25%
lisp/url/url-parse-tests,0.097,0.113,16%
lisp/url/url-tramp-tests,0.139,0.160,15%
lisp/url/url-util-tests,0.109,0.131,20%
lisp/vc/add-log-tests,0.085,0.103,21%
lisp/vc/diff-mode-tests,0.160,0.178,11%
lisp/vc/ediff-diff-tests,0.087,0.111,28%
lisp/vc/ediff-ptch-tests,0.117,0.133,14%
lisp/vc/log-edit-tests,0.131,0.153,17%
lisp/vc/smerge-mode-tests,0.085,0.097,14%
lisp/vc/vc-bzr-tests,0.085,0.103,21%
lisp/vc/vc-git-tests,0.083,0.103,24%
lisp/vc/vc-hg-tests,0.086,0.104,21%
lisp/vc/vc-tests,0.297,0.332,12%
lisp/version-tests,0.073,0.092,26%
lisp/wdired-tests,0.129,0.146,13%
lisp/whitespace-tests,0.070,0.090,29%
lisp/wid-edit-tests,0.085,0.103,21%
lisp/xdg-tests,0.075,0.095,27%
lisp/xml-tests,0.092,0.110,20%
lisp/xt-mouse-tests,0.118,0.133,13%
misc/test-custom-deps,0.072,0.092,28%
misc/test-custom-libs,0.072,0.092,28%
misc/test-custom-noloads,0.072,0.091,26%
misc/test-custom-opts,0.074,0.092,24%
src/alloc-tests,0.073,0.092,26%
src/buffer-tests,0.138,0.153,11%
src/callint-tests,0.072,0.091,26%
src/callproc-tests,0.109,0.126,16%
src/casefiddle-tests,0.087,0.102,17%
src/character-tests,0.073,0.092,26%
src/charset-tests,0.072,0.091,26%
src/chartab-tests,0.073,0.090,23%
src/cmds-tests,0.072,0.092,28%
src/coding-tests,0.121,0.130,7%
src/comp-tests,0.251,0.301,20%
src/data-tests,0.100,0.112,12%
src/decompress-tests,0.073,0.092,26%
src/doc-tests,0.074,0.093,26%
src/editfns-tests,0.202,0.216,7%
src/emacs-module-tests,0.371,0.390,5%
src/emacs-tests,0.180,0.197,9%
src/eval-tests,0.246,0.263,7%
src/fileio-tests,0.088,0.099,13%
src/filelock-tests,0.085,0.103,21%
src/floatfns-tests,0.084,0.102,21%
src/fns-tests,0.109,0.126,16%
src/font-tests,0.085,0.104,22%
src/image-tests,0.083,0.095,14%
src/indent-tests,0.073,0.092,26%
src/inotify-tests,0.085,0.101,19%
src/json-tests,0.083,0.096,16%
src/keyboard-tests,0.080,0.095,19%
src/keymap-tests,0.213,0.229,8%
src/lcms-tests,0.074,0.092,24%
src/lread-tests,0.197,0.211,7%
src/marker-tests,0.156,0.167,7%
src/minibuf-tests,0.084,0.102,21%
src/print-tests,0.168,0.186,11%
src/process-tests,1.723,1.747,1%
src/regex-emacs-tests,0.160,0.176,10%
src/search-tests,0.072,0.091,26%
src/sqlite-tests,0.074,0.093,26%
src/syntax-tests,0.144,0.163,13%
src/textprop-tests,0.071,0.093,31%
src/thread-tests,0.103,0.107,4%
src/timefns-tests,0.208,0.218,5%
src/undo-tests,0.654,0.700,7%
src/xdisp-tests,0.076,0.096,26%
src/xfaces-tests,0.073,0.091,25%
src/xml-tests,0.073,0.092,26%

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Gregory.

On Fri, Feb 04, 2022 at 00:11:46 +0000, Gregory Heytings wrote:

> >>> More importantly, I wonder how slowing down EQ by a factor of 2 can 
> >>> end up costing 10% of runtime when running the test suite.  I think 
> >>> this deserves investigation.

> >> Do you have an idea how this could be investigated?

> > Usually such slowdown don't come from everywhere at the same time. So 
> > you need to "slice" our total time into subelements, and presumably some 
> > of those elements show a higher slowdown, so you can focus on those and 
> > start slicing them further.

> > As you get closer to the source, the slowdown should become more marked.


> You were too optimistic.

> I just finished a detailed analysis of the slowdown of the execution of 
> Emacs' test suite, and it turns out that the slowdown is indeed spread 
> over all tests.

> I attach the detailed results for each of the 389 tests.  Each test has 
> been executed 2000 (two thousand) times, again on an unloaded up-to-date 
> Debian bookworm computer.

Just for completion's sake, that was presumably without native
compilation.

> If you look closer at the results, you will see that the slowdown is 
> actually worse than 10%.  The slowest of all tests (lisp/net/tramp-tests) 
> is only 2% slower, because it uses external processes and is therefore not 
> as much affected as other tests by the slowdown of bytecode execution 
> itself.  If you remove that test from the calculations, you will see that 
> the slowdown is actually 17%, that is, the same slowdown as that of byte 
> compilation.

> (FWIW, I was puzzled by the claim that byte compilation could be slowed 
> down markedly, and that at the same time general bytecode execution would 
> not.  Byte compilation does not do anything that is fundamentally 
> different from general bytecode execution.)

Compilation _does_ do things differently.  It runs with
symbols-with-pos-enabled bound to t, which makes EQ slower than when
that variable is nil.  To see this, have a look at the defition of
lisp_h_EQ in src/lisp.h, around line 372.

> My conclusion is that this merge should be backed out.  Its performance 
> impact has not been properly resolved and assessed.

These commits fixed a long standing bug, namely the outputting of wrong
position information in compiler warning messages.  Perhaps you could
propose a better way of fixing that bug.  For the record, I don't
believe there is a better way which is practical.

Also, there is something strange about the speed measurements on the
test suite that gives such marked slowdowns.  On measurements of "real"
Emacs activities which don't involve compilation, the slowdowns measured
were "no measurable difference", 1% and 3%.

Perhaps you could hypothesize why there is such a large speed difference
in those tests, but not in real Emacs use.  Such an understanding could
be useful.  My guess is that there is an unusually large number of `eq's
in the test suite, and that it does very little garbage collection or
I/O.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Fri, 4 Feb 2022 11:57:21 +0000
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, mattiase@acm.org,
>   Eli Zaretskii <eliz@gnu.org>, larsi@gnus.org, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> Compilation _does_ do things differently.  It runs with
> symbols-with-pos-enabled bound to t, which makes EQ slower than when
> that variable is nil.  To see this, have a look at the defition of
> lisp_h_EQ in src/lisp.h, around line 372.

Can you explain why we need to complicate EQ so much to account for
symbols-with-pos?  Specific questions:

  . comparison of a symbol and symbol-with-pos should only compare the
    symbol part, and disregard the "pos" part, right?
  . if so, couldn't we implement symbol-with-pos as a struct that has
    the symbol at its beginning, and then use memcmp to compare only
    that part?

Or what am I missing?

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Eli.

On Fri, Feb 04, 2022 at 14:06:39 +0200, Eli Zaretskii wrote:
> > Date: Fri, 4 Feb 2022 11:57:21 +0000
> > Cc: Stefan Monnier <monnier@iro.umontreal.ca>, mattiase@acm.org,
> >   Eli Zaretskii <eliz@gnu.org>, larsi@gnus.org, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > Compilation _does_ do things differently.  It runs with
> > symbols-with-pos-enabled bound to t, which makes EQ slower than when
> > that variable is nil.  To see this, have a look at the defition of
> > lisp_h_EQ in src/lisp.h, around line 372.

> Can you explain why we need to complicate EQ so much to account for
> symbols-with-pos?

There are four possible ways the two Lisp objects might match - each
object can be either a bare symbol, or a symbol with pos, giving four
possibilities.  It's worth noting that the last three possibilities are
cut off completely by the check on symbols-with-pos-enabled, except
during compilation.

Specific questions:

>   . comparison of a symbol and symbol-with-pos should only compare the
>     symbol part, and disregard the "pos" part, right?
>   . if so, couldn't we implement symbol-with-pos as a struct that has
>     the symbol at its beginning, and then use memcmp to compare only
>     that part?

> Or what am I missing?

I don't think we can get anywhere that way.  What gets compared in EQ
are the Lisp_Object's, which are typically already in processor
registers.  I don't think we typically have a pointer to memory
containing the symbol with position.  Don't forget that the position
attaches to the symbol OCCURRENCE, not the symbol itself.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Fri, 4 Feb 2022 18:31:01 +0000
> Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
>   larsi@gnus.org, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > Can you explain why we need to complicate EQ so much to account for
> > symbols-with-pos?
> 
> There are four possible ways the two Lisp objects might match - each
> object can be either a bare symbol, or a symbol with pos, giving four
> possibilities.  It's worth noting that the last three possibilities are
> cut off completely by the check on symbols-with-pos-enabled, except
> during compilation.

I don't see how the number of combinations is relevant.  If all you
need to compare is the symbol part, then whether there's something
after it in memory is not interesting and doesn't affect comparison.

> Specific questions:
> 
> >   . comparison of a symbol and symbol-with-pos should only compare the
> >     symbol part, and disregard the "pos" part, right?
> >   . if so, couldn't we implement symbol-with-pos as a struct that has
> >     the symbol at its beginning, and then use memcmp to compare only
> >     that part?
> 
> > Or what am I missing?
> 
> I don't think we can get anywhere that way.  What gets compared in EQ
> are the Lisp_Object's, which are typically already in processor
> registers.  I don't think we typically have a pointer to memory
> containing the symbol with position.

Sorry, I don't understand how this answers my questions.

> Don't forget that the position attaches to the symbol OCCURRENCE,
> not the symbol itself.

I don't understand what I need not to forget, please elaborate.

What is there in a symbol-with-pos except the symbol and the position?

(And please be more cooperative and forthcoming, since my questions
are meant to help make this feature less expensive, otherwise there's
a real chance of the feature to be removed, something I hope you don't
want.)

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Eli.

On Fri, Feb 04, 2022 at 20:54:20 +0200, Eli Zaretskii wrote:
> > Date: Fri, 4 Feb 2022 18:31:01 +0000
> > Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
> >   larsi@gnus.org, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > Can you explain why we need to complicate EQ so much to account for
> > > symbols-with-pos?

> > There are four possible ways the two Lisp objects might match - each
> > object can be either a bare symbol, or a symbol with pos, giving four
> > possibilities.  It's worth noting that the last three possibilities are
> > cut off completely by the check on symbols-with-pos-enabled, except
> > during compilation.

> I don't see how the number of combinations is relevant.  If all you
> need to compare is the symbol part, then whether there's something
> after it in memory is not interesting and doesn't affect comparison.

Yes, that's true.

> > Specific questions:

> > >   . comparison of a symbol and symbol-with-pos should only compare the
> > >     symbol part, and disregard the "pos" part, right?

Yes.

> > >   . if so, couldn't we implement symbol-with-pos as a struct that has
> > >     the symbol at its beginning, and then use memcmp to compare only
> > >     that part?

> > > Or what am I missing?

> > I don't think we can get anywhere that way.  What gets compared in EQ
> > are the Lisp_Object's, which are typically already in processor
> > registers.  I don't think we typically have a pointer to memory
> > containing the symbol with position.

> Sorry, I don't understand how this answers my questions.

OK, I don't think we can improve the performance by the use of a struct
containing the symbol followed by the position.  I'm not sure what more I
can add.

> > Don't forget that the position attaches to the symbol OCCURRENCE,
> > not the symbol itself.

> I don't understand what I need not to forget, please elaborate.

In the moderate past, that thing, the (lack of) possibility of attaching
the position to the symbol, caused me a lack of clarity.  I forgot many
times that we need to deal with symbol occurrences, not symbols.

We cannot extend struct Lisp_Symbol with the position, since a single
symbol typically occurs several times in different positions.  Rather, I
think we would need to extend the Lisp_Object in this fashion.

> What is there in a symbol-with-pos except the symbol and the position?

There is the symbol, the position, and a pseudovector header.

> (And please be more cooperative and forthcoming, since my questions
> are meant to help make this feature less expensive, otherwise there's
> a real chance of the feature to be removed, something I hope you don't
> want.)

Sorry, I was doing my best to answer what I thought you were asking.  I
wasn't trying to be evasive.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Fri, 4 Feb 2022 19:33:28 +0000
> Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
>   larsi@gnus.org, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > > >   . comparison of a symbol and symbol-with-pos should only compare the
> > > >     symbol part, and disregard the "pos" part, right?
> 
> Yes.
> 
> > > >   . if so, couldn't we implement symbol-with-pos as a struct that has
> > > >     the symbol at its beginning, and then use memcmp to compare only
> > > >     that part?
> 
> > > > Or what am I missing?
> 
> > > I don't think we can get anywhere that way.  What gets compared in EQ
> > > are the Lisp_Object's, which are typically already in processor
> > > registers.  I don't think we typically have a pointer to memory
> > > containing the symbol with position.
> 
> > Sorry, I don't understand how this answers my questions.
> 
> OK, I don't think we can improve the performance by the use of a struct
> containing the symbol followed by the position.  I'm not sure what more I
> can add.

Then let me ask guiding questions.

First, are symbols-with-pos supposed to happen in bytecode that
doesn't deal with byte compilation?  If yes, why/when would such
objects appear in GP bytecode?

> We cannot extend struct Lisp_Symbol with the position, since a single
> symbol typically occurs several times in different positions.

So?  That would just mean there could be several different Lisp
objects whose "symbol parts" are identical, but the position parts are
different.  Right?

> > What is there in a symbol-with-pos except the symbol and the position?
> 
> There is the symbol, the position, and a pseudovector header.

The pseudovector part is not needed if we just extend Lisp_Symbol to
have an additional field 'position'.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Eli.

On Fri, Feb 04, 2022 at 21:46:06 +0200, Eli Zaretskii wrote:
> > Date: Fri, 4 Feb 2022 19:33:28 +0000
> > Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
> >   larsi@gnus.org, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > > >   . comparison of a symbol and symbol-with-pos should only compare the
> > > > >     symbol part, and disregard the "pos" part, right?

> > Yes.

> > > > >   . if so, couldn't we implement symbol-with-pos as a struct that has
> > > > >     the symbol at its beginning, and then use memcmp to compare only
> > > > >     that part?

> > > > > Or what am I missing?

> > > > I don't think we can get anywhere that way.  What gets compared in EQ
> > > > are the Lisp_Object's, which are typically already in processor
> > > > registers.  I don't think we typically have a pointer to memory
> > > > containing the symbol with position.

> > > Sorry, I don't understand how this answers my questions.

> > OK, I don't think we can improve the performance by the use of a struct
> > containing the symbol followed by the position.  I'm not sure what more I
> > can add.

> Then let me ask guiding questions.

> First, are symbols-with-pos supposed to happen in bytecode that
> doesn't deal with byte compilation?

Symbols with pos are intended to be used only in compilation, native- as
well as byte-.  They mustn't be output to .elc files.

> If yes, why/when would such objects appear in GP bytecode?

What does "GP" mean here, please?

> > We cannot extend struct Lisp_Symbol with the position, since a single
> > symbol typically occurs several times in different positions.

> So?  That would just mean there could be several different Lisp
> objects whose "symbol parts" are identical, but the position parts are
> different.  Right?

I suppose so, yes.

> > > What is there in a symbol-with-pos except the symbol and the position?

> > There is the symbol, the position, and a pseudovector header.

> The pseudovector part is not needed if we just extend Lisp_Symbol to
> have an additional field 'position'.

Yes.  I'm not sure we can do this, though.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

>> First, are symbols-with-pos supposed to happen in bytecode that
>> doesn't deal with byte compilation?
>
> Symbols with pos are intended to be used only in compilation, native- as
> well as byte-.  They mustn't be output to .elc files.
>
>> If yes, why/when would such objects appear in GP bytecode?
>
> What does "GP" mean here, please?

I think he meant "general purpose".  So the answer is no,
symbol-with-pos should not occur in general purpose code.  And indeed
that's why you have that boolean var controlling `eq` to choose between
the slow version (used in the compiler) and the fast version (used
everywhere else).  The fast version is exactly the same as the
previous `eq` but it still makes the new `eq` slower because of the
extra choice between the slow version and the fast one.

>> The pseudovector part is not needed if we just extend Lisp_Symbol to
>> have an additional field 'position'.
>
> Yes.  I'm not sure we can do this, though.

I can't see why we couldn't but I can't see why it would help either.
We'd still have the problem that two different objects may need to be
considered `eq`, so we need `eq` to be slower.


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

> I think he meant "general purpose".  So the answer is no,
> symbol-with-pos should not occur in general purpose code.  And indeed
> that's why you have that boolean var controlling `EQ` to choose between
> the slow version (used in the compiler) and the fast version (used
> everywhere else).  The fast version is exactly the same as the
> previous `eq` but it still makes the new `EQ` slower because of the
> extra choice between the slow version and the fast one.

BTW, currently, it's not just `EQ` that's slower but also `XSYMBOL` and
`SYMBOLP`.  I think we could make those faster (especially we could
probably make `SYMBOLP` as fast as it was), but I don't know if those
have a noticeable performance impact.

> I can't see why we couldn't but I can't see why it would help either.
> We'd still have the problem that two different objects may need to be
> considered `eq`, so we need `eq` to be slower.

One way out of this is to go through all the compiler code and all the
macros and arrange for it to avoid using `eq` (and use `equal` instead)
when comparing symbols.

Then we could revert `EQ` to be the fast equality check that it was.

It's no small task, tho,


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  gregory@heytings.org,  mattiase@acm.org,
>  larsi@gnus.org,  emacs-devel@gnu.org
> Date: Fri, 04 Feb 2022 17:30:09 -0500
> 
> BTW, currently, it's not just `EQ` that's slower but also `XSYMBOL` and
> `SYMBOLP`.

Where's the data which says what functions/macros became slower with
this change?

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

>> BTW, currently, it's not just `EQ` that's slower but also `XSYMBOL` and
>> `SYMBOLP`.
> Where's the data which says what functions/macros became slower with
> this change?

Nowhere.  It's just that its code has gotten slightly more complex (an
additional test or two).  As I said, I don't know if it ever has any
measurable impact.  I just mentioned it for completeness's sake.


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: acm@muc.de,  gregory@heytings.org,  mattiase@acm.org,  larsi@gnus.org,
>   emacs-devel@gnu.org
> Date: Sat, 05 Feb 2022 13:04:36 -0500
> 
> >> BTW, currently, it's not just `EQ` that's slower but also `XSYMBOL` and
> >> `SYMBOLP`.
> > Where's the data which says what functions/macros became slower with
> > this change?
> 
> Nowhere.  It's just that its code has gotten slightly more complex (an
> additional test or two).  As I said, I don't know if it ever has any
> measurable impact.  I just mentioned it for completeness's sake.

I think it's high time we actually measured the impact by using perf.
Otherwise this discussion is just a lot of hand-waving and "I said, he
said" kind of argument.  We will never be able to establish whether
the slowdown is real in actual Emacs usage or just in
micro-benchmarks.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Fri, 4 Feb 2022 21:24:56 +0000
> Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
>   larsi@gnus.org, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > First, are symbols-with-pos supposed to happen in bytecode that
> > doesn't deal with byte compilation?
> 
> Symbols with pos are intended to be used only in compilation, native- as
> well as byte-.  They mustn't be output to .elc files.

So that would mean the slowdown of EQ is due to the cases where the
objects are not-EQ?  Where are the numbers that show how much slower
is the current EQ, for the case of EQ and not-EQ objects?

> > If yes, why/when would such objects appear in GP bytecode?
> 
> What does "GP" mean here, please?

General Purpose.

> > > > What is there in a symbol-with-pos except the symbol and the position?
> 
> > > There is the symbol, the position, and a pseudovector header.
> 
> > The pseudovector part is not needed if we just extend Lisp_Symbol to
> > have an additional field 'position'.
> 
> Yes.  I'm not sure we can do this, though.

Let's revisit that once we understand the slowdown of EQ better and
more quantitatively.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Eli.

On Sat, Feb 05, 2022 at 10:17:36 +0200, Eli Zaretskii wrote:
> > Date: Fri, 4 Feb 2022 21:24:56 +0000
> > Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
> >   larsi@gnus.org, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > First, are symbols-with-pos supposed to happen in bytecode that
> > > doesn't deal with byte compilation?

> > Symbols with pos are intended to be used only in compilation, native- as
> > well as byte-.  They mustn't be output to .elc files.

> So that would mean the slowdown of EQ is due to the cases where the
> objects are not-EQ?

Yes.

> Where are the numbers that show how much slower is the current EQ, for
> the case of EQ and not-EQ objects?

We don't have any such numbers.  Is it even possible to measure this?  On
earlier processors, we could have just counted up processor cycles used
for each instruction, but not any more.

> > > If yes, why/when would such objects appear in GP bytecode?

> > What does "GP" mean here, please?

> General Purpose.

Thanks.  Symbols with position shouldn't appear at all in general purpose
bytecode.

> > > > > What is there in a symbol-with-pos except the symbol and the position?

> > > > There is the symbol, the position, and a pseudovector header.

> > > The pseudovector part is not needed if we just extend Lisp_Symbol to
> > > have an additional field 'position'.

> > Yes.  I'm not sure we can do this, though.

> Let's revisit that once we understand the slowdown of EQ better and
> more quantitatively.

OK.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sun, 6 Feb 2022 11:50:02 +0000
> Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
>   larsi@gnus.org, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > So that would mean the slowdown of EQ is due to the cases where the
> > objects are not-EQ?
> 
> Yes.
> 
> > Where are the numbers that show how much slower is the current EQ, for
> > the case of EQ and not-EQ objects?
> 
> We don't have any such numbers.  Is it even possible to measure this?  On
> earlier processors, we could have just counted up processor cycles used
> for each instruction, but not any more.

Yes, it's possible: use perf.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Eli.

On Sun, Feb 06, 2022 at 13:56:34 +0200, Eli Zaretskii wrote:
> > Date: Sun, 6 Feb 2022 11:50:02 +0000
> > Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
> >   larsi@gnus.org, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > So that would mean the slowdown of EQ is due to the cases where the
> > > objects are not-EQ?

> > Yes.

> > > Where are the numbers that show how much slower is the current EQ, for
> > > the case of EQ and not-EQ objects?

> > We don't have any such numbers.  Is it even possible to measure this?  On
> > earlier processors, we could have just counted up processor cycles used
> > for each instruction, but not any more.

> Yes, it's possible: use perf.

I've installed the user side part of perf on my machine.  On reading the
tutorial (which is very difficult), it seems perf acts like a super
accurate benchmarking program, which measures program runs.

Maybe I misunderstand what you meant - but I can't see how perf is able
to report the cycles, etc., taken by a single execution of an instance of
EQ.

What exactly do we want to measure, here?

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sun, 6 Feb 2022 18:09:30 +0000
> Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
>   larsi@gnus.org, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > > > Where are the numbers that show how much slower is the current EQ, for
> > > > the case of EQ and not-EQ objects?
> 
> > > We don't have any such numbers.  Is it even possible to measure this?  On
> > > earlier processors, we could have just counted up processor cycles used
> > > for each instruction, but not any more.
> 
> > Yes, it's possible: use perf.
> 
> I've installed the user side part of perf on my machine.  On reading the
> tutorial (which is very difficult), it seems perf acts like a super
> accurate benchmarking program, which measures program runs.
> 
> Maybe I misunderstand what you meant - but I can't see how perf is able
> to report the cycles, etc., taken by a single execution of an instance of
> EQ.
> 
> What exactly do we want to measure, here?

Perf is capable of showing profiles at a source line granularity.
Since the relation of parts of EQ to equal and non-equal objects is
quite clear, the profile produced by perf can be easily interpreted as
pertaining to either of these two possibilities.  By comparing the
profiles with the old and the new definitions of EQ you should be able
to measure the slowdown we are interested in.

Btw, unlike what you said up-thread, I don't recommend comparing the
version of Emacs before the merge of the branch with today's
codebase.  Instead, I suggest to compile two versions of the same
Emacs codebase, one with the old and simple definition of EQ, the
other with the definition we use now on master.  This should make the
comparison easier, since the only difference will be how EQ is
defined.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Eli.

On Sun, Feb 06, 2022 at 20:39:14 +0200, Eli Zaretskii wrote:
> > Date: Sun, 6 Feb 2022 18:09:30 +0000
> > Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
> >   larsi@gnus.org, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > > > Where are the numbers that show how much slower is the current EQ, for
> > > > > the case of EQ and not-EQ objects?

> > > > We don't have any such numbers.  Is it even possible to measure this?  On
> > > > earlier processors, we could have just counted up processor cycles used
> > > > for each instruction, but not any more.

> > > Yes, it's possible: use perf.

> > I've installed the user side part of perf on my machine.  On reading the
> > tutorial (which is very difficult), it seems perf acts like a super
> > accurate benchmarking program, which measures program runs.

> > Maybe I misunderstand what you meant - but I can't see how perf is able
> > to report the cycles, etc., taken by a single execution of an instance of
> > EQ.

> > What exactly do we want to measure, here?

> Perf is capable of showing profiles at a source line granularity.
> Since the relation of parts of EQ to equal and non-equal objects is
> quite clear, the profile produced by perf can be easily interpreted as
> pertaining to either of these two possibilities.  By comparing the
> profiles with the old and the new definitions of EQ you should be able
> to measure the slowdown we are interested in.

After building each version with CFLAGS='-O2 -g3' and
--with-native-compilation, I've run

    $ perf record -e cpu-clock make check

followed by

    $ perf report -i perf.data --tui

on both old and new EQ versions of Emacs.

On the old version, there is no sign of EQ.  I searched with `/' at the
top level of perf report, and found only other functions with "EQ" in the
name in two non-Emacs libraries.

On the new version, EQ appears just twice when I type `/' EQ RET.  The
total CPU usage appears to be around 0.01%.  In fact the screen looks
like this:

Samples: 576K of event 'cpu-clock:u', Event count (approx.): 144023250000
Overhead  Comma  Shared Object        Symbol
   0.01%  emacs  emacs                [.] EQ
   0.00%  hg     libpython3.9.so.1.0  [.] _PyUnicode_EQ
   0.00%  emacs  emacs                [.] EQ

Typing RET on one of the two EQ lines, then RET again, displays
percentages taken by the various instructions in that particular
expansion of EQ.

I think part of the problem is that EQ isn't in any particular place in
Emacs.  In the C source it occurs over 2,700 times.  Is there any way to
get perf to amalgamate the different occurrences of EQ?

I haven't got any useful information out of the exercise, so far.  I
can't help feeling that I'm missing something.  Is there anything I ought
to be doing that I've not yet done?

> Btw, unlike what you said up-thread, I don't recommend comparing the
> version of Emacs before the merge of the branch with today's
> codebase.  Instead, I suggest to compile two versions of the same
> Emacs codebase, one with the old and simple definition of EQ, the
> other with the definition we use now on master.  This should make the
> comparison easier, since the only difference will be how EQ is
> defined.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sat, 19 Feb 2022 16:42:07 +0000
> Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
>   larsi@gnus.org, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> I haven't got any useful information out of the exercise, so far.  I
> can't help feeling that I'm missing something.  Is there anything I ought
> to be doing that I've not yet done?

Maybe you should make EQ real function, with an attribute that would
preclude its inlining.

I have no other ideas.  Maybe someone else does.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[David Engster]

>> Date: Sat, 19 Feb 2022 16:42:07 +0000
>> Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
>>   larsi@gnus.org, emacs-devel@gnu.org
>> From: Alan Mackenzie <acm@muc.de>
>> 
>> I haven't got any useful information out of the exercise, so far.  I
>> can't help feeling that I'm missing something.  Is there anything I ought
>> to be doing that I've not yet done?
>
> Maybe you should make EQ real function, with an attribute that would
> preclude its inlining.
>
> I have no other ideas.  Maybe someone else does.

Making EQ a non-inlined function would be the first step. But since perf
is a sampling profiler, it is not easy to measure something that's
already very quick - it's simply quite unlikely that perf would hit it,
unless it is called extremely often. The default sampling frequency of
perf is 1kHz, so the first thing you can try is to increase that, so
that it becomes more likely to hit EQ (see the '-F' switch for 'perf
record').

However, it might simply be that a tracing profiler would be the better
choice here, so maybe look into good old gprof or valgrind. Or maybe
even instrument EQ yourself and count cycles between start and end.

-David

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, David.

On Sat, Feb 19, 2022 at 18:43:34 +0100, David Engster wrote:
> >> Date: Sat, 19 Feb 2022 16:42:07 +0000
> >> Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
> >>   larsi@gnus.org, emacs-devel@gnu.org
> >> From: Alan Mackenzie <acm@muc.de>

> >> I haven't got any useful information out of the exercise, so far.  I
> >> can't help feeling that I'm missing something.  Is there anything I ought
> >> to be doing that I've not yet done?

> > Maybe you should make EQ real function, with an attribute that would
> > preclude its inlining.

> > I have no other ideas.  Maybe someone else does.

> Making EQ a non-inlined function would be the first step. But since perf
> is a sampling profiler, it is not easy to measure something that's
> already very quick - it's simply quite unlikely that perf would hit it,
> unless it is called extremely often.

It is called extremely often.  I managed to de-inline EQ.

> The default sampling frequency of perf is 1kHz, so the first thing you
> can try is to increase that, so that it becomes more likely to hit EQ
> (see the '-F' switch for 'perf record').

I seem to have a default setting of 4kHz.  This was enough to get several
thousand hits in each build ("old" and "new") running the test suite.  As
I wrote in my post just now to Eli, the results came up contrary to what
was expected - the "old" build, running the simple EQ spent more time in
it than the "new" build running the complicated EQ.  Possibly I've made
some silly mistake in the measurements.

> However, it might simply be that a tracing profiler would be the better
> choice here, so maybe look into good old gprof or valgrind. Or maybe
> even instrument EQ yourself and count cycles between start and end.

Time to sleep on it, I think.  Thanks for the post.

> -David

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[David Engster]

> I seem to have a default setting of 4kHz.  This was enough to get several
> thousand hits in each build ("old" and "new") running the test suite.  As
> I wrote in my post just now to Eli, the results came up contrary to what
> was expected - the "old" build, running the simple EQ spent more time in
> it than the "new" build running the complicated EQ.  Possibly I've made
> some silly mistake in the measurements.

Hi Alan,

Are you comparing builds directly before/after the merge of the
correct-warnings branch? The first thing to check would be if EQ is
actually called the same amount of times in both builds. You can do this
with 'perf' by using user-space probes ('uprobes'). Just as an example,
with './emacs' being your emacs binary,

  perf probe -x ./emacs redisplay

would insert a uprobe at redisplay, and with

  perf record -e probe_emacs:redisplay ./emacs

you can save the events. Just to show how flexible this system is:

  perf stat -e probe_emacs:redisplay -a -I 1000

would continuously show you the number of times redisplay is called each
second.

Anyway, measuring things around ~1% with perf can be quite noisy. For
doing exact timings, you can also insert 'uretprobes' which are called
when a function returns, and then you can calculate how long each call
took from the timestamps. In the above example,

   perf probe -x ./emacs 'redisplay%return'

will insert a return probe for 'redisplay', then use

   perf record -e 'probe_emacs:*' ./emacs

and use 'perf script' to display the timestamps. I don't think there's
something built-in to display durations between uprobe/uretprobe, so a
little bit of coding is needed to calculate that from the output. Of
course, these timings will usually be increased by the probe
instrumentations, but for relative comparisons, they should be fine.

-David

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, David.

On Sun, Feb 20, 2022 at 06:35:25 +0100, David Engster wrote:
> > I seem to have a default setting of 4kHz.  This was enough to get several
> > thousand hits in each build ("old" and "new") running the test suite.  As
> > I wrote in my post just now to Eli, the results came up contrary to what
> > was expected - the "old" build, running the simple EQ spent more time in
> > it than the "new" build running the complicated EQ.  Possibly I've made
> > some silly mistake in the measurements.

> Hi Alan,

> Are you comparing builds directly before/after the merge of the
> correct-warnings branch?

Yes.

> The first thing to check would be if EQ is actually called the same
> amount of times in both builds. You can do this with 'perf' by using
> user-space probes ('uprobes'). Just as an example, with './emacs'
> being your emacs binary,

>   perf probe -x ./emacs redisplay

> would insert a uprobe at redisplay, and with

>   perf record -e probe_emacs:redisplay ./emacs

> you can save the events. Just to show how flexible this system is:

>   perf stat -e probe_emacs:redisplay -a -I 1000

> would continuously show you the number of times redisplay is called each
> second.

I tried that with EQ on the new build.  I aborted the run when perf.data
had reached 29 GB.  ;-).  I then tried again with just making
test/lisp/calc/calc-tests.log.  That gave a file around 1 GB, which gave
me the number of times EQ was called.

Then I temporarily lost interest.  I'll try the same on the old build
soon.

> Anyway, measuring things around ~1% with perf can be quite noisy. For
> doing exact timings, you can also insert 'uretprobes' which are called
> when a function returns, and then you can calculate how long each call
> took from the timestamps. In the above example,

>    perf probe -x ./emacs 'redisplay%return'

> will insert a return probe for 'redisplay', then use

>    perf record -e 'probe_emacs:*' ./emacs

> and use 'perf script' to display the timestamps.

Maybe I'll try that later.

> I don't think there's something built-in to display durations between
> uprobe/uretprobe, so a little bit of coding is needed to calculate
> that from the output. Of course, these timings will usually be
> increased by the probe instrumentations, but for relative comparisons,
> they should be fine.

Thanks!

> -David

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

> I seem to have a default setting of 4kHz.  This was enough to get several
> thousand hits in each build ("old" and "new") running the test suite.  As
> I wrote in my post just now to Eli, the results came up contrary to what
> was expected - the "old" build, running the simple EQ spent more time in
> it than the "new" build running the complicated EQ.  Possibly I've made
> some silly mistake in the measurements.

Maybe it's because EQ was used for NILP before but isn't used for it
any more?

In any case, in my experience measuring such things is wickedly
difficult.  It's even hard to define what it is we really want to
measure because the impact will depend so heavily of how the code gets
inlined&scheduled by the compiler and then how it gets scheduled in the
CPU, and how branch prediction interacts with it, etc...


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Stefan.

On Sun, Feb 20, 2022 at 15:37:55 -0500, Stefan Monnier wrote:
> > I seem to have a default setting of 4kHz.  This was enough to get several
> > thousand hits in each build ("old" and "new") running the test suite.  As
> > I wrote in my post just now to Eli, the results came up contrary to what
> > was expected - the "old" build, running the simple EQ spent more time in
> > it than the "new" build running the complicated EQ.  Possibly I've made
> > some silly mistake in the measurements.

> Maybe it's because EQ was used for NILP before but isn't used for it
> any more?

Bingo!  That's surely the reason.  In the new build, NILP wasn't using
EQ.  I'll sort that out and compare again.  Just not tonight.

Thank you very much!

> In any case, in my experience measuring such things is wickedly
> difficult.  It's even hard to define what it is we really want to
> measure because the impact will depend so heavily of how the code gets
> inlined&scheduled by the compiler and then how it gets scheduled in the
> CPU, and how branch prediction interacts with it, etc...

I think I'm just trying to get some sort of handle on how much the code
has slowed down because of EQ.  Obviously, making EQ into a function
(rather than an inline function) will have distorted things, but it
seems the easiest way of measuring something at the moment.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

> I think I'm just trying to get some sort of handle on how much the code
> has slowed down because of EQ.  Obviously, making EQ into a function
> (rather than an inline function) will have distorted things, but it
> seems the easiest way of measuring something at the moment.

I understand, but it's still going to be hard to figure out what the
result means.  Let's say you find that the new non-inlined EQ is 20%
slower than the old non-inlined EQ, what does it tell us about the
inlined versions?

It's quite possible that more than 50% of the time spent in the
non-inlined EQ is spent in the "function call overhead" (the jumps
themselves, the forced placement into specific registers, the need to
reify a condition code into a boolean stored in a register, the
impossibility to move code around the call to EQ because the compiler
doesn't know it's a pure function, ...)?


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Óscar Fuentes]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I think I'm just trying to get some sort of handle on how much the code
>> has slowed down because of EQ.  Obviously, making EQ into a function
>> (rather than an inline function) will have distorted things, but it
>> seems the easiest way of measuring something at the moment.
>
> I understand, but it's still going to be hard to figure out what the
> result means.  Let's say you find that the new non-inlined EQ is 20%
> slower than the old non-inlined EQ, what does it tell us about the
> inlined versions?
>
> It's quite possible that more than 50% of the time spent in the
> non-inlined EQ is spent in the "function call overhead" (the jumps
> themselves, the forced placement into specific registers, the need to
> reify a condition code into a boolean stored in a register, the
> impossibility to move code around the call to EQ because the compiler
> doesn't know it's a pure function, ...)?

For those and more reasons it is useless to directly measure changes to
small utility functions in complex applications, even to non-inline
ones. The results can be highly misleading.

If you want to know the impact of such a change, the correct method is
to measure representative instances of real-world cases.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> From: Óscar Fuentes <ofv@wanadoo.es>
> Date: Mon, 21 Feb 2022 01:22:08 +0100
> 
> If you want to know the impact of such a change, the correct method is
> to measure representative instances of real-world cases.

We already did that.  We now want to understand _why_ the new version
is slower, and then try making it faster.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Óscar Fuentes]

Eli Zaretskii <eliz@gnu.org> writes:

>> If you want to know the impact of such a change, the correct method is
>> to measure representative instances of real-world cases.
>
> We already did that.  We now want to understand _why_ the new version
> is slower, and then try making it faster.

Then I suggest to use Valgrind for collecting performance data at the
machine instruction level, on a release build (with debug info, of
course.) Knowledge of Assembler and processor architecture required. And
patience, lots of patience.

Although if the difference is also perceptible in a build with -Od, you
could try this before. Its analysis will be much more approachable than
-O2. Then test hypothesis in -O2.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> From: Óscar Fuentes <ofv@wanadoo.es>
> Date: Mon, 21 Feb 2022 05:04:57 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> If you want to know the impact of such a change, the correct method is
> >> to measure representative instances of real-world cases.
> >
> > We already did that.  We now want to understand _why_ the new version
> > is slower, and then try making it faster.
> 
> Then I suggest to use Valgrind for collecting performance data at the
> machine instruction level

AFAIK, Perf is fully capable of collecting performance data on this
level.  So what will Valgrind gain us, except the need to battle the
known problems when running Emacs under Valgrind?

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Óscar Fuentes]

Eli Zaretskii <eliz@gnu.org> writes:

>> >> If you want to know the impact of such a change, the correct method is
>> >> to measure representative instances of real-world cases.
>> >
>> > We already did that.  We now want to understand _why_ the new version
>> > is slower, and then try making it faster.
>> 
>> Then I suggest to use Valgrind for collecting performance data at the
>> machine instruction level
>
> AFAIK, Perf is fully capable of collecting performance data on this
> level.

Perf is a sampling profiler and that makes very tricky to get reliable
data for a tiny piece of code within a large application.

> So what will Valgrind gain us, except the need to battle the known
> problems when running Emacs under Valgrind?

Valgrind emulates each instruction and its context (L1/L2 cache misses,
etc.) That's why Valgrind is two orders of magnitude slower than Perf
(and it accounts for a good chunk of the "patience" requirement I
mentioned on my previous message.)

Profiling Emacs with Valgrind should not pose problems, AFAIK.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: David Engster <deng@randomsample.de>,  Eli Zaretskii <eliz@gnu.org>,
>  larsi@gnus.org,  mattiase@acm.org,  gregory@heytings.org,
>  emacs-devel@gnu.org
> Date: Sun, 20 Feb 2022 18:02:01 -0500
> 
> > I think I'm just trying to get some sort of handle on how much the code
> > has slowed down because of EQ.  Obviously, making EQ into a function
> > (rather than an inline function) will have distorted things, but it
> > seems the easiest way of measuring something at the moment.
> 
> I understand, but it's still going to be hard to figure out what the
> result means.  Let's say you find that the new non-inlined EQ is 20%
> slower than the old non-inlined EQ, what does it tell us about the
> inlined versions?

That they are ~20% slower.  Moreover, it hopefully will tell us which
part(s) of EQ are more expensive, and thus allow to try making it
faster.

> It's quite possible that more than 50% of the time spent in the
> non-inlined EQ is spent in the "function call overhead" (the jumps
> themselves, the forced placement into specific registers, the need to
> reify a condition code into a boolean stored in a register, the
> impossibility to move code around the call to EQ because the compiler
> doesn't know it's a pure function, ...)?

That time should be the same in the old and the new EQ.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

> Maybe you should make EQ real function, with an attribute that would
> preclude its inlining.

But that's going to slow it down, probably even more so that the
slowdown we're trying to measure.


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Alan Mackenzie <acm@muc.de>,  gregory@heytings.org,  mattiase@acm.org,
>   larsi@gnus.org,  emacs-devel@gnu.org
> Date: Sat, 19 Feb 2022 14:01:27 -0500
> 
> > Maybe you should make EQ real function, with an attribute that would
> > preclude its inlining.
> 
> But that's going to slow it down, probably even more so that the
> slowdown we're trying to measure.

Yes, but what we are mainly interested in is to see how much slower
the new EQ is wrt the old one.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Eli.

On Sat, Feb 19, 2022 at 19:02:22 +0200, Eli Zaretskii wrote:
> > Date: Sat, 19 Feb 2022 16:42:07 +0000
> > Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
> >   larsi@gnus.org, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > I haven't got any useful information out of the exercise, so far.  I
> > can't help feeling that I'm missing something.  Is there anything I ought
> > to be doing that I've not yet done?

> Maybe you should make EQ real function, with an attribute that would
> preclude its inlining.

That's an excellent idea!  Thanks!  I managed to get that working.

I reconfigured old and new without native compilation (in which it would
be more work to de-inline EQ) and rebuilt them.  I ran make check on
each of them to have as many .elc files built as possible.  On running

    $ perf record -e cpu-clock make check
    $ mv perf.data <name>
    $ perf report -i <name> --tui

I found EQ taking 1.48% on the old build and 0.84% on the new build.
Yes, I haven't got them the wrong way round.  I saw the same on two
consecutive runs for both builds, and the number of samples involved
(several thousand) was enough to eliminate random wrongness.

I don't understand what I'm seeing.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Eli.

On Sat, Feb 19, 2022 at 19:02:22 +0200, Eli Zaretskii wrote:
> > Date: Sat, 19 Feb 2022 16:42:07 +0000
> > Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
> >   larsi@gnus.org, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>
> > 
> > I haven't got any useful information out of the exercise, so far.  I
> > can't help feeling that I'm missing something.  Is there anything I ought
> > to be doing that I've not yet done?

> Maybe you should make EQ real function, with an attribute that would
> preclude its inlining.

> I have no other ideas.  Maybe someone else does.

I now have some numbers.

I've compared the versions of the master branch just before and just
after the merge of scratch/correct-warning-pos with this difference in
the new version:
(i) A binding of load-read-function was removed (this should be
  irrelevant).

, and this difference in the old version:
(i) The macro/function BASE_EQ was copied from the new version and NILP
  amended to use it.  This excludes NILP from contributing to the EQ
  measurements in the old version, just as it is in the new.

In both versions, the inlining was removed from EQ, which was then
inserted into xdisp.c as a normal function which invokes the macro
lisp_h_EQ.  This macro differs between the old and new versions.

These two versions were configured the same, without native-compilation,
and built.  A make check was run to compile (most of) the test-foo.elc
files.  Then in each the following were run:

    $ perf record -e cpu-clock make check
    $ perf report -i perf.data --tui

..  The proportions of the profiler samples in EQ were:
   (old): 0.48%
   (new): 0.86%

..  This is fairly close to the guessed factor of 2 difference.  However,
it doesn't, by itself, account for the difference in total run time
between the two versions.  The total number of events counted for these
make check runs was
  (old): 372k
  (new): 419k

..  The breakdown of samples on individual instructions in the old and new
versions of EQ is thus:

(old):
EQ  /home/acm/emacs/emacs.git/sub-master-b/src/emacs [Percent: local period]
Samples│
       │
       │
       │   Disassembly of section .text:
       │
       │   0000000000071550 <EQ>:
       │   EQ():
       │
       │   /* STOUGH, 2022-02-19 */
       │   bool
       │   EQ (Lisp_Object x, Lisp_Object y)
       │   {
       │   return lisp_h_EQ (x, y);
   946 │     cmp  %rdi,%rsi
   172 │     sete %al
       │   }
   675 │   ← ret

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(new):
EQ  /home/acm/emacs/emacs.git/sub-master-2/src/emacs [Percent: local period]
Samples│
       │
       │
       │    Disassembly of section .text:
       │
       │    000000000006d150 <EQ>:
       │    EQ():
       │    no-ops.  */
       │
       │    INLINE EMACS_INT
       │    (XLI) (Lisp_Object o)
       │    {
       │    return lisp_h_XLI (o);
  1230 │      mov    $0x1,%eax
       │
       │    /* STOUGH, 2022-02-19 */
       │    bool
       │    EQ (Lisp_Object x, Lisp_Object y)
       │    {
       │    return lisp_h_EQ (x, y);
   311 │      cmp    %rsi,%rdi
   617 │    ↓ je     76
       │      movzbl globals+0xfee,%eax
  1340 │      test   %al,%al
    81 │    ↓ je     76
       │    TAGGEDP():
       │    Equivalent to XTYPE (a) == TAG, but often faster.  */
       │
       │    INLINE bool
       │    (TAGGEDP) (Lisp_Object a, enum Lisp_Type tag)
       │    {
       │    return lisp_h_TAGGEDP (a, tag);
       │      lea    -0x5(%rdi),%eax
       │    PSEUDOVECTORP():
       │    #define MOST_NEGATIVE_FIXNUM (-1 - MOST_POSITIVE_FIXNUM)
       │
       │    INLINE bool
       │    PSEUDOVECTORP (Lisp_Object a, int code)
       │    {
       │    return lisp_h_PSEUDOVECTORP (a, code);
     5 │      test   $0x7,%al
     9 │    ↓ jne    50
       │      movabs $0x400000003f000000,%rdx
       │      mov    -0x5(%rdi),%rcx
     1 │      movabs $0x4000000006000000,%rax
       │      and    %rdx,%rcx
       │      cmp    %rax,%rcx
       │    ↓ jne    50
       │    EQ():
       │      test   $0x7,%sil
       │    ↓ jne    90
       │      cmp    %rsi,0x3(%rdi)
       │      sete   %al
       │    ← ret
       │      nop
       │    TAGGEDP():
       │    return lisp_h_TAGGEDP (a, tag);
       │50:   lea    -0x5(%rsi),%eax
       │    PSEUDOVECTORP():
       │    return lisp_h_PSEUDOVECTORP (a, code);
     4 │      test   $0x7,%al
     5 │    ↓ jne    74
       │      movabs $0x400000003f000000,%rax
       │      and    -0x5(%rsi),%rax
     1 │      movabs $0x4000000006000000,%rdx
       │      cmp    %rdx,%rax
       │    ↓ je     80
       │74:   xor    %eax,%eax
       │    EQ():
       │    }
     1 │76: ← ret
       │      nop
       │    return lisp_h_EQ (x, y);
       │80:   test   $0x7,%dil
       │    ↑ jne    74
       │      cmp    %rdi,0x3(%rsi)
       │      sete   %al
       │    ← ret
       │      xchg   %ax,%ax
       │    TAGGEDP():
       │    return lisp_h_TAGGEDP (a, tag);
       │90:   lea    -0x5(%rsi),%r8d
       │    PSEUDOVECTORP():
       │      xor    %eax,%eax
       │    return lisp_h_PSEUDOVECTORP (a, code);
       │      and    $0x7,%r8d
       │    ↑ jne    76
       │      and    -0x5(%rsi),%rdx
       │      cmp    %rcx,%rdx
       │    ↑ jne    76
       │    EQ():
       │      mov    0x3(%rdi),%rax
       │      cmp    %rax,0x3(%rsi)
       │      sete   %al
       │    ← ret
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

As yet, I don't know quite what to make of these numbers.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sun, 6 Feb 2022 18:09:30 +0000
> Cc: gregory@heytings.org, monnier@iro.umontreal.ca, mattiase@acm.org,
>   larsi@gnus.org, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> On reading the tutorial (which is very difficult)

Oh, one other thing: the Internet is full with very detailed tutorials
for how to use perf in different scenarios, so I suggest to look those
up if the tutorial that comes with the installation is not good
enough.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sun, 06 Feb 2022 20:40:41 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: larsi@gnus.org, mattiase@acm.org, gregory@heytings.org,
>  monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> Oh, one other thing: the Internet is full with very detailed tutorials
> for how to use perf in different scenarios, so I suggest to look those
> up if the tutorial that comes with the installation is not good
> enough.

Here's one I found useful at the time:

   http://sandsoftwaresound.net/perf/perf-tutorial-hot-spots/

It has a couple of examples that are relevant to what we want to do.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sun, 06 Feb 2022 20:40:41 +0200
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: larsi@gnus.org, mattiase@acm.org, gregory@heytings.org,
>>  monnier@iro.umontreal.ca, emacs-devel@gnu.org
>> 
>> Oh, one other thing: the Internet is full with very detailed tutorials
>> for how to use perf in different scenarios, so I suggest to look those
>> up if the tutorial that comes with the installation is not good
>> enough.
>
> Here's one I found useful at the time:
>
>    http://sandsoftwaresound.net/perf/perf-tutorial-hot-spots/
>
> It has a couple of examples that are relevant to what we want to do.

Hi all,

just to add: Alan if you are going to compare two builds with perf I'd
suggest to use two native compiled instances, with that using perf
you'll get information also on which specific Lisp function is taking
time.  I guess it might help the investigation.

Otherwise with bytecode for Lisp execution all time for perf is spent in
'exec_byte_code'.

Hope it helps.

Best Regards

  Andrea

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

> I attach the detailed results for each of the 389 tests.  Each test
> has been executed 2000 (two thousand) times, again on an unloaded
> up-to-date Debian bookworm computer.

[...]

> If you remove that test from the calculations, you will see that the
> slowdown is actually 17%, that is, the same slowdown as that of byte
> compilation.

Thanks, that's interesting, but it doesn't really answer the question of
why it's so hard to see these performance regressions in actual use.  I
tried to benchmark Alan's patches before they went in by doing things
like measuring shr DOM rendering, and saw essentially no measurable
difference.  And 17% vs "essentially nothing" is a big gap.

So it's still not clear what's being measured.  Is ert doing something
that's triggering these slowdowns?  Is it only measurable in "emacs
-batch"?  Is there something else that makes the test suites so much
slower while we're not seeing that in real usage?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Lars.

On Sat, Feb 05, 2022 at 07:08:16 +0100, Lars Ingebrigtsen wrote:
> Gregory Heytings <gregory@heytings.org> writes:

> > I attach the detailed results for each of the 389 tests.  Each test
> > has been executed 2000 (two thousand) times, again on an unloaded
> > up-to-date Debian bookworm computer.

> [...]

> > If you remove that test from the calculations, you will see that the
> > slowdown is actually 17%, that is, the same slowdown as that of byte
> > compilation.

> Thanks, that's interesting, but it doesn't really answer the question of
> why it's so hard to see these performance regressions in actual use.  I
> tried to benchmark Alan's patches before they went in by doing things
> like measuring shr DOM rendering, and saw essentially no measurable
> difference.  And 17% vs "essentially nothing" is a big gap.

> So it's still not clear what's being measured.  Is ert doing something
> that's triggering these slowdowns?  Is it only measurable in "emacs
> -batch"?  Is there something else that makes the test suites so much
> slower while we're not seeing that in real usage?

I just tried to disassemble an ert-form for (ert-deftest
parse-partial-sexp-paren-comments () ...), in tests/src/syntax-tests.el.
I haven't yet managed it, but just dumping (get
'parse-partial-sexp-paren-comments 'ert--test) onto the *Messages*
buffer, it is apparent that symbols with position have somehow got into
the compiled form.  They appear like this:

\312\313^B\301\"D\262^A\244\240\210\314\303\242!\207" [V0 V1 V2 V3 (should (#<symbol = at 3586> (#<symbol nth at 3589> \

..  This might have something to do with the slowdown, it might not.  But
there is definitely a bug somewhere to fix.  I don't know yet whether
these dumped forms actually get executed, or whether they're just there
to be displayed on an error.  If they do get executed, the SWP there
might explain the slowdown in the tests.

> -- 
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Lars Ingebrigtsen]

Alan Mackenzie <acm@muc.de> writes:

> ..  This might have something to do with the slowdown, it might not.  But
> there is definitely a bug somewhere to fix.  I don't know yet whether
> these dumped forms actually get executed, or whether they're just there
> to be displayed on an error.  If they do get executed, the SWP there
> might explain the slowdown in the tests.

Ah, interesting.  As you say, this may not explain the slowdown, but
Gregory's found a bug here, at the very least.  (ert does a lot of odd
things.)

But this reminds me of something I meant to ask, but forgot: Would it
make sense to introduce a configuration option to en/disable the SWP
stuff?  Probably not as a long term thing, but it would make
benchmarking things a lot easier if we could en/disable it on the
current trunk simply.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Gregory Heytings <gregory@heytings.org>,  Stefan Monnier
>  <monnier@iro.umontreal.ca>,  mattiase@acm.org,  Eli Zaretskii
>  <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Sat, 05 Feb 2022 22:31:12 +0100
> 
> But this reminds me of something I meant to ask, but forgot: Would it
> make sense to introduce a configuration option to en/disable the SWP
> stuff?

This question was already asked, and you then said it will be very
complicated.  Is it?

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Lars.

On Sat, Feb 05, 2022 at 22:31:12 +0100, Lars Ingebrigtsen wrote:
> Alan Mackenzie <acm@muc.de> writes:

[ .... ]

> But this reminds me of something I meant to ask, but forgot: Would it
> make sense to introduce a configuration option to en/disable the SWP
> stuff?  Probably not as a long term thing, but it would make
> benchmarking things a lot easier if we could en/disable it on the
> current trunk simply.

By doing something like calling read rather than read-positioning-symbols
from the byte compiler, and putting conditional compilation into
lisp_h_EQ, etc., in src/lisp.h?  This would leave the compiler not fully
functional, of course.  It might work.

I get the feeling it's one of those things which sounds simple, but there
might be unforeseen problems waiting.  One of those problems might be the
option somehow surviving long beyond its intended lifetime, with pressure
to maintain it coming from somewhere.

I would more favour the slightly more laborious strategy: benchmark on
the master branch in two repositories, one just before the big merge, the
other just after.

> -- 
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Lars Ingebrigtsen]

Alan Mackenzie <acm@muc.de> writes:

> By doing something like calling read rather than read-positioning-symbols
> from the byte compiler, and putting conditional compilation into
> lisp_h_EQ, etc., in src/lisp.h?  This would leave the compiler not fully
> functional, of course.  It might work.

Or just have conditional compilation of lisp_h_EQ and have
read-positioning-symbols be an alias for read (when this is disabled).
Or something like that.  So the code impact of the configuration option
could be very localised.

> I would more favour the slightly more laborious strategy: benchmark on
> the master branch in two repositories, one just before the big merge, the
> other just after.

People don't like to skip back in git history much, because other code
has also moved on since that time, and you end up with various obscure
issues very fast.  Configuration options are just more handy.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>> Are there other benchmarks (not micro-benchmarks) that could be used to 
>> reach what you would consider a mature conclusion?
>
> There is Andrea Corallo's elisp-benchmark package.
>

It's a micro-benchmark, and moreover it is difficult to interpret, its 
results on the same (unloaded) machine with the same version of Emacs vary 
by +/- 5%.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Andrea Corallo]

Gregory Heytings <gregory@heytings.org> writes:

>>> Are there other benchmarks (not micro-benchmarks) that could be
>>> used to reach what you would consider a mature conclusion?
>>
>> There is Andrea Corallo's elisp-benchmark package.
>>
>
> It's a micro-benchmark, and moreover it is difficult to interpret, its
> results on the same (unloaded) machine with the same version of Emacs
> vary by +/- 5%.

Hi Gregory,

if your system is noisy you can increase `elb-runs' to an higher value,
in the results an estimation of the error anyway is given (probably more
reliable with ~ elb-runs >= 5?).

That said yes, is a suite of micro benchmarks but it's better than
nothing.  Actually it would be *very* interesting to add to
elisp-benchmark a micro benchmark capable of stressing exactly this
supposed performance regression.  It would really help understanding the
(observed?) slowdown.

Initially when the branch was proposed I tried quickly to do that but I
failed, maybe I was rushing due to lack of time and overlooked something
or maybe except for the compilation phase the slowdown is really
minimal, dunno.  I encourage you to try my same experiment, if you find
an interesting case we'll add it to the suite.

Thanks

  Andrea

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>
> if your system is noisy
>

It's on an unloaded build machine, with just one sshd and one bash 
process.

>
> you can increase `elb-runs' to an higher value, in the results an 
> estimation of the error anyway is given (probably more reliable with ~ 
> elb-runs >= 5?).
>

Thanks, I'll keep that in mind!

>
> That said yes, is a suite of micro benchmarks but it's better than 
> nothing.
>

Of course; my remark was certainly not meant to be derogatory.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Andrea Corallo]

Gregory Heytings <gregory@heytings.org> writes:

>>
>> if your system is noisy
>>
>
> It's on an unloaded build machine, with just one sshd and one bash
> process.
>
>>
>> you can increase `elb-runs' to an higher value, in the results an
>> estimation of the error anyway is given (probably more reliable with
>> ~ elb-runs >= 5?).
>>
>
> Thanks, I'll keep that in mind!
>
>>
>> That said yes, is a suite of micro benchmarks but it's better than
>> nothing.
>>
>
> Of course; my remark was certainly not meant to be derogatory.

No worries, it was not how I got it :)

Last words just to repeat that if there's a slowdown in such a defined
area of code, a micro benchmark is the right tool to prove and
investigate that.  We must be able to clearly exercise this supposed
slowdown if exists.

Thanks

  Andrea

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>>> Are there other benchmarks (not micro-benchmarks) that could be used 
>>> to reach what you would consider a mature conclusion?
>> 
>> There is Andrea Corallo's elisp-benchmark package.
>
> It's a micro-benchmark, and moreover it is difficult to interpret, its 
> results on the same (unloaded) machine with the same version of Emacs 
> vary by +/- 5%.
>

(And as Mattias said upthread it is not representative of real Elisp 
usage.)

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Tue, 25 Jan 2022 08:59:00 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: mattiase@acm.org, larsi@gnus.org, emacs-devel@gnu.org, 
>     monnier@iro.umontreal.ca, acm@muc.de
> 
> I think you understood what I meant: bytecode execution in Emacs is now, 
> on average, about 10% slower.  Are there other benchmarks (not 
> micro-benchmarks) that could be used to reach what you would consider a 
> mature conclusion?

I think we need more measurements in scenarios closer to actual Emacs
usage.  One is intensive display operation -- I think Alan posted
something to that effect.  Another could be starting Gnus to read a
large newsgroup.  Yet another could be reindent a large piece of C or
Python or Lisp code.  Perhaps also "M-x occur" through a large
buffer.  Stuff like that -- any command that tends to be used
frequently and is known to take a tangible amount of time.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>
> I think we need more measurements in scenarios closer to actual Emacs 
> usage.  One is intensive display operation -- I think Alan posted 
> something to that effect.  Another could be starting Gnus to read a 
> large newsgroup.  Yet another could be reindent a large piece of C or 
> Python or Lisp code.  Perhaps also "M-x occur" through a large buffer. 
> Stuff like that -- any command that tends to be used frequently and is 
> known to take a tangible amount of time.
>

The problem is that, IME, such benchmarks do not show anything when the 
slowdown is not significant enough (> 10%), because the measurements vary 
a lot depending on external factors.  I wonder how Alan could conclude 
that there is a slowdown of "less than 1%", when I run his benchmark on my 
(otherwise unloaded) computer, the running times I get are anywhere 
between 17s and 20s.  Taking the average of such measures does not really 
make sense.

However, I found one measurement that seems to give reasonably stable 
results.  On my Debian bookworm computer, with a standard build, on 
src/xdisp.c, (benchmark-run 100 (occur "a.b")) needs on average (with 10 
runs) 7.42 seconds (standard deviation 0.04) on 3b33a14380 and 7.66 
seconds (standard deviation 0.05) on 7922131bb2.  That's a >3% slowdown.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Wed, 26 Jan 2022 18:41:57 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: mattiase@acm.org, larsi@gnus.org, emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca, acm@muc.de
> 
> > I think we need more measurements in scenarios closer to actual Emacs 
> > usage.  One is intensive display operation -- I think Alan posted 
> > something to that effect.  Another could be starting Gnus to read a 
> > large newsgroup.  Yet another could be reindent a large piece of C or 
> > Python or Lisp code.  Perhaps also "M-x occur" through a large buffer. 
> > Stuff like that -- any command that tends to be used frequently and is 
> > known to take a tangible amount of time.
> >
> 
> The problem is that, IME, such benchmarks do not show anything when the 
> slowdown is not significant enough (> 10%), because the measurements vary 
> a lot depending on external factors.

Which external factors are those?

> I wonder how Alan could conclude that there is a slowdown of "less
> than 1%", when I run his benchmark on my (otherwise unloaded)
> computer, the running times I get are anywhere between 17s and 20s.
> Taking the average of such measures does not really make sense.

That is not my experience: my measurements of these and other
benchmarks are remarkably stable, as long as no heavy job (such as
building some large package) is running in parallel.

> However, I found one measurement that seems to give reasonably stable 
> results.  On my Debian bookworm computer, with a standard build, on 
> src/xdisp.c, (benchmark-run 100 (occur "a.b")) needs on average (with 10 
> runs) 7.42 seconds (standard deviation 0.04) on 3b33a14380 and 7.66 
> seconds (standard deviation 0.05) on 7922131bb2.  That's a >3% slowdown.

OK, thanks.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

>> The problem is that, IME, such benchmarks do not show anything when the 
>> slowdown is not significant enough (> 10%), because the measurements vary 
>> a lot depending on external factors.
> Which external factors are those?

On the machines I use the dynamic CPU frequency changes are an important
such factor.


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Gregory Heytings <gregory@heytings.org>,  mattiase@acm.org,
>   larsi@gnus.org,  emacs-devel@gnu.org,  acm@muc.de
> Date: Wed, 26 Jan 2022 14:14:26 -0500
> 
> >> The problem is that, IME, such benchmarks do not show anything when the 
> >> slowdown is not significant enough (> 10%), because the measurements vary 
> >> a lot depending on external factors.
> > Which external factors are those?
> 
> On the machines I use the dynamic CPU frequency changes are an important
> such factor.

Then either disable that when benchmarking, or acknowledge that your
machine is useless for benchmarking.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Gregory.

On Wed, Jan 26, 2022 at 18:41:57 +0000, Gregory Heytings wrote:


> > I think we need more measurements in scenarios closer to actual Emacs 
> > usage.  One is intensive display operation -- I think Alan posted 
> > something to that effect.  Another could be starting Gnus to read a 
> > large newsgroup.  Yet another could be reindent a large piece of C or 
> > Python or Lisp code.  Perhaps also "M-x occur" through a large buffer. 
> > Stuff like that -- any command that tends to be used frequently and is 
> > known to take a tangible amount of time.

> The problem is that, IME, such benchmarks do not show anything when the 
> slowdown is not significant enough (> 10%), because the measurements vary 
> a lot depending on external factors.  I wonder how Alan could conclude 
> that there is a slowdown of "less than 1%", when I run his benchmark on my 
> (otherwise unloaded) computer, the running times I get are anywhere 
> between 17s and 20s.  Taking the average of such measures does not really 
> make sense.

No, but the first run in an Emacs session tends to be shorter than
subsequent runs.  I don't know why.  I take it you also saw a very small
difference in the timings for this benchmark between the two versions.
Otherwise you would have said, I think.

The four measurements I got from my benchmark in master (from
benchmark-run) were:
      o - (20.798601883 460 7.728701306)
      o - (20.947118356 295 7.1172684969999995)
      o - (20.941589929 293 7.144901186)
      o - (20.917180235 293 7.136285445000002)
The corresponding four from the (then) branch were
      o - (20.854543266 480 7.691123986)
      o - (21.064465459 320 7.189660959000001)
      o - (21.143813105 318 7.287708998000001)
      o - (21.115932422 318 7.266432223999999)
It is fairly clear the second set are ~1% more than the first set, give
or take unimportant measurement errors.

That's precisely what I meant when I said that the branch was about 1%
slower ON THAT BENCHMARK.  Please note what I said, going back to my
original post if needed.  I did not say "less than" 1%.  I was not
referring to the speed in general.  I was very specific about what I
wrote.

> However, I found one measurement that seems to give reasonably stable 
> results.  On my Debian bookworm computer, with a standard build, on 
> src/xdisp.c, (benchmark-run 100 (occur "a.b")) needs on average (with 10 
> runs) 7.42 seconds (standard deviation 0.04) on 3b33a14380 and 7.66 
> seconds (standard deviation 0.05) on 7922131bb2.  That's a >3% slowdown.

Yes, that's around 3.2%.  Scarcely something you'd notice in an
interactive session, where the difference is around the time it takes to
depress a key on the keyboard.

So we now have a real life measurement which is more than 1%.  It's
nowhere near 10%, though.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Gregory.

On Sat, Jan 22, 2022 at 17:02:43 +0000, Gregory Heytings wrote:

> >> I'd like to see the results from elisp-benchmarks with and without this 
> >> patch set, please.

> > In my experience, the elisp-benchmarks are very much not representative 
> > of actual elisp usage, and I have done quite some benchmarking for other 
> > reasons lately.


> I don't know what benchmarks would be representative of actual elisp 
> usage, but on my build machine both make bootstrap and make check are 
> consistently ~8% slower on correct-warning-pos vs master:

> make -j1 bootstrap: 649s vs 596s
> make -j4 bootstrap: 257s vs 238s
> make -j8 bootstrap: 198s vs 183s

> make -j1 check: 178s vs 162s
> make -j4 check: 44s vs 40s
> make -j8 check: 26s vs 24s

This is true, and expected.  However the bootstrap is still faster than
it was this time last week, due to commit
10083e788f7349fa363d100687dc3d94bea88f57 on Thursday.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>
> However the bootstrap is still faster than it was this time last week, 
> due to commit 10083e788f7349fa363d100687dc3d94bea88f57 on Thursday.
>

That is only true for those who use --with-native-compilation.  I don't.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sat, 22 Jan 2022 18:45:03 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Mattias Engdegård <mattiase@acm.org>, 
>     Lars Ingebrigtsen <larsi@gnus.org>, Eli Zaretskii <eliz@gnu.org>, 
>     Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel@gnu.org
> 
> > However the bootstrap is still faster than it was this time last week, 
> > due to commit 10083e788f7349fa363d100687dc3d94bea88f57 on Thursday.
> 
> That is only true for those who use --with-native-compilation.  I don't.

It's supposed to be faster even without native compilation, albeit
perhaps not by the same amount.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>>> However the bootstrap is still faster than it was this time last week, 
>>> due to commit 10083e788f7349fa363d100687dc3d94bea88f57 on Thursday.
>>
>> That is only true for those who use --with-native-compilation.  I 
>> don't.
>
> It's supposed to be faster even without native compilation, albeit 
> perhaps not by the same amount.
>

I don't know, but I just compared 7922131bb2 vs 3b33a14380, and in most 
cases 7922131bb2 is even a bit slower than correct-warning-pos:

make -j1 bootstrap: 654s vs 596s
make -j4 bootstrap: 256s vs 238s
make -j8 bootstrap: 201s vs 183s

make -j1 check: 181s vs 162s
make -j4 check: 45s vs 40s
make -j8 check: 26s vs 23s

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Eli Zaretskii]

> Date: Sat, 22 Jan 2022 20:07:34 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: acm@muc.de, mattiase@acm.org, larsi@gnus.org, monnier@iro.umontreal.ca, 
>     emacs-devel@gnu.org
> 
> 
> >>> However the bootstrap is still faster than it was this time last week, 
> >>> due to commit 10083e788f7349fa363d100687dc3d94bea88f57 on Thursday.
> >>
> >> That is only true for those who use --with-native-compilation.  I 
> >> don't.
> >
> > It's supposed to be faster even without native compilation, albeit 
> > perhaps not by the same amount.
> >
> 
> I don't know, but I just compared 7922131bb2 vs 3b33a14380, and in most 
> cases 7922131bb2 is even a bit slower than correct-warning-pos:
> 
> make -j1 bootstrap: 654s vs 596s
> make -j4 bootstrap: 256s vs 238s
> make -j8 bootstrap: 201s vs 183s
> 
> make -j1 check: 181s vs 162s
> make -j4 check: 45s vs 40s
> make -j8 check: 26s vs 23s

Is that an optimized or an unoptimized build of Emacs?

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Gregory Heytings]

>> I don't know, but I just compared 7922131bb2 vs 3b33a14380, and in most 
>> cases 7922131bb2 is even a bit slower than correct-warning-pos:
>>
>> make -j1 bootstrap: 654s vs 596s
>> make -j4 bootstrap: 256s vs 238s
>> make -j8 bootstrap: 201s vs 183s
>>
>> make -j1 check: 181s vs 162s
>> make -j4 check: 45s vs 40s
>> make -j8 check: 26s vs 23s
>
> Is that an optimized or an unoptimized build of Emacs?
>

It's a standard build, with the default build options, on Debian bookworm. 
The commands above are exactly what I typed in a fresh clone of the 
repository.  IOW, CFLAGS='-g3 -O2'.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Stefan Monnier]

> In these benchmarks, I used the following function:
>
>     (defun time-scroll-b (&optional arg)
>       (condition-case nil
>           (while t
>             (if arg (scroll-down) (scroll-up))
>             (sit-for 0))
>         (error nil)))

Any chance you could add this benchmark to `elisp-benchmarks`?

Has someone tried other benchmarks than this one?

Could you say something about the impact of this patch outside of the
obvious things in lread.c and bytecomp.el (other than performance), both
in terms of what code is changed and what potential changes one might
expect to see (and could affect for example external packages) in
corner cases?


        Stefan

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Brahimi Saifullah]

Hello Alan.

Flymake is not working as of the latest commit (3023e7c).
The offending function is `elisp-flymake--byte-compile-done',
I've narrowed down the issue to the following section:
[...]
         (cl-loop with data =
                  (with-current-buffer output-buffer
                    (goto-char (point-min))
                    (search-forward ":elisp-flymake-output-start")
                    (read (point-marker)))
                  for (string pos _fill level) in data
                  do (goto-char pos)
                                ^^^
[...]
With DATA being:

    (("Invalid read syntax: \")\", 8, 4" nil nil :error)
     ("reference to free variable `x'" nil t :warning))

POS here, the second element of the list, is nil, as you can see.

For comparison, this is what DATA looks like on Emacs 28

    (("Invalid read syntax: \")\", 8, 4" 196 nil :error)
     ("reference to free variable `x'" 195 t :warning))

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Brahimi.

On Sat, Jan 15, 2022 at 21:27:04 -0300, Brahimi Saifullah wrote:
> Hello Alan.

> Flymake is not working as of the latest commit (3023e7c).

Thanks indeed for the report.

> The offending function is `elisp-flymake--byte-compile-done',
> I've narrowed down the issue to the following section:
> [...]
>          (cl-loop with data =
>                   (with-current-buffer output-buffer
>                     (goto-char (point-min))
>                     (search-forward ":elisp-flymake-output-start")
>                     (read (point-marker)))
>                   for (string pos _fill level) in data
>                   do (goto-char pos)
>                                 ^^^
> [...]
> With DATA being:

>     (("Invalid read syntax: \")\", 8, 4" nil nil :error)
>      ("reference to free variable `x'" nil t :warning))

> POS here, the second element of the list, is nil, as you can see.

> For comparison, this is what DATA looks like on Emacs 28

>     (("Invalid read syntax: \")\", 8, 4" 196 nil :error)
>      ("reference to free variable `x'" 195 t :warning))

That looks like some minor glitch interfering.

I don't actually use Flymake mode.  Could you possibly give me a recipe
to reproduce the bug, then either João or I can fix it.

Thanks again!

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Brahimi Saifullah]

Alan Mackenzie <acm@muc.de> writes:

> I don't actually use Flymake mode.  Could you possibly give me a recipe
> to reproduce the bug, then either João or I can fix it.

Sure.

emacs -Q
M-x flymake-mode
Type anything.

I've also further tracked down the issue to `byte-compile-log-warning':

    (defun byte-compile-log-warning (string &optional fill level)
      "Log a byte-compilation warning.
    STRING, FILL and LEVEL are as described in
    `byte-compile-log-warning-function', which see."
      (funcall byte-compile-log-warning-function
               string nil
               fill   ^^^
               level))

The highlighted part used to be `byte-compile-last-position',
but it was replaced with nil. The default byte compile log warning
function  doesn't make use of that value (which is why you wouldn't
ordinarily notice it's amiss), but Flymake's function does.

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Brahimi.

Sorry it's taken time to get back to you.  I got distracted from this
matter by another project (speeding up the bootstrap build).

On Sun, Jan 16, 2022 at 13:45:25 -0300, Brahimi Saifullah wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > I don't actually use Flymake mode.  Could you possibly give me a recipe
> > to reproduce the bug, then either João or I can fix it.

> Sure.

> emacs -Q
> M-x flymake-mode
> Type anything.

Thanks.  That was helpful.

> I've also further tracked down the issue to `byte-compile-log-warning':

>     (defun byte-compile-log-warning (string &optional fill level)
>       "Log a byte-compilation warning.
>     STRING, FILL and LEVEL are as described in
>     `byte-compile-log-warning-function', which see."
>       (funcall byte-compile-log-warning-function
>                string nil
>                fill   ^^^
>                level))

Thanks!  Hey, why aren't you on the team?  ;-)

> The highlighted part used to be `byte-compile-last-position',
> but it was replaced with nil. The default byte compile log warning
> function  doesn't make use of that value (which is why you wouldn't
> ordinarily notice it's amiss), but Flymake's function does.

I've amended the function such that POSITION is no longer an &optional
parameter, to help other people avoid falling into the same trap.  In
place of that nil, I now give a proper position (I hope).

With the current state of the git branch, flymake on a .el file now
seems to be working for me.

I've committed the fix (along with a few other amendments) and I'd be
grateful indeed if you could take another look at it.  I'm hoping to
merge it into master very soon indeed.

Again, many thanks for the help.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Brahimi Saifullah]

I'm very sorry for the noise, it seems I replied to the wrong Alan :(

https://lists.gnu.org/archive/html/emacs-devel/2022-01/msg01454.html
Brahimi Saifullah <brahimi.saifullah@gmail.com> writes:

> Alan Mackenzie <acm@muc.de> writes:
>
>> With the current state of the git branch, flymake on a .el file now
>> seems to be working for me.
>
> Yes, Flymake is working perfectly now.
>
> I did notice an issue with the warning for unused forms:
>
>     value returned from (#<symbol + at 2> 1 1) is unused
>     (from byte-compiling a file that consists of "(+ 1 1)")
>
> I expect that the symbol should be used, not the symbol with
> positions.  But it looks like this just got fixed a few hours ago
> in master while my build was from before the merge.
>
>
> While further testing this I also found a couple problems relating to
> unused function arguments -- I'm unsure if they have anything to do with
> the latest round of changes or not.
>
> First, a bug:
>
>     (defun f (a b c)
>   ; ^^^ Warning: Unused lexical argument `a'
>   ;      Warning: Unused lexical argument `b'
>
>     "A B C."
>     (+ 1 1))
>   ; ^^^ Warning: Unused lexical argument `c'
>
> You should be able to reproduce it by copying the above function
> somewhere and byte compiling (sans the).
>
> If the function consists of a single form, the position returned for
> the last "unused lexical argument" will wrongly point to it.  In this
> example, the warning about C being unused will instead point to `+'.
>
> The second issue is that, even when the unused lexical argument warnings
> work correctly, they always point to the "defun," and not to the actual
> argument in question.  Not the end of the world of course, but it would
> be nice for them to be more accurate.
>
>
>> Again, many thanks for the help.
>
> And thanks for the work you've done, faulty warning
> positions had always been an annoyance of mine :)

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Alan Mackenzie]

Hello, Brahimi.

On Sat, Jan 22, 2022 at 20:16:43 -0300, Brahimi Saifullah wrote:
> I'm very sorry for the noise, it seems I replied to the wrong Alan :(

:-)  I know the feeling.  I'm massaging your post a bit and replying to
it.

> Alan Mackenzie <acm@muc.de> writes:

>> With the current state of the git branch, flymake on a .el file now
>> seems to be working for me.

> Yes, Flymake is working perfectly now.

> I did notice an issue with the warning for unused forms:

>     value returned from (#<symbol + at 2> 1 1) is unused
>     (from byte-compiling a file that consists of "(+ 1 1)")

I'll look at that later today, thanks.

> I expect that the symbol should be used, not the symbol with
> positions.  But it looks like this just got fixed a few hours ago
> in master while my build was from before the merge.


> While further testing this I also found a couple problems relating to
> unused function arguments -- I'm unsure if they have anything to do with
> the latest round of changes or not.

> First, a bug:

>     (defun f (a b c)
>   ; ^^^ Warning: Unused lexical argument `a'
>   ;      Warning: Unused lexical argument `b'

>     "A B C."
>     (+ 1 1))
>   ; ^^^ Warning: Unused lexical argument `c'

> You should be able to reproduce it by copying the above function
> somewhere and byte compiling (sans the).

Yes, no problem.  That was a one-line correction to some hairy code which
went to great lengths to get a correct (or close) warning position in the
previous mechanism.  Sorry about the mistake.

I've committed the fix for this to master.

> If the function consists of a single form, the position returned for
> the last "unused lexical argument" will wrongly point to it.  In this
> example, the warning about C being unused will instead point to `+'.

> The second issue is that, even when the unused lexical argument warnings
> work correctly, they always point to the "defun," and not to the actual
> argument in question.  Not the end of the world of course, but it would
> be nice for them to be more accurate.

I think it's fixed now.  Thank you indeed for the bug reports.

>> Again, many thanks for the help.

> And thanks for the work you've done, faulty warning
> positions had always been an annoyance of mine :)

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Time to merge scratch/correct-warning-pos into master, perhaps?

[Andrea Corallo]

Alan Mackenzie <acm@muc.de> writes:

> Hello, Emacs.
>
> The branch scratch/correct-warning-pos is working and is almost
> finished.  Its intention is to get the byte compiler to report correct
> source positions in warnings and errors it emits.  It remains just to
> write some documentation about symbols with position and to fix any last
> minute bugs which turn up.
>
> There is a minor bug in test/lisp/emacs-lisp/comp-tests.el, which should
> be easier to resolve when Andrea is back again.
>
> To use it, just merge it and use it.  You shouldn't notice much
> difference.  If there are any byte compiler warning positions which
> remain uncorrect, please report them.
>
> Thie branch's predecessor was rejected in 2018 since it slowed down
> Emacs too much.  My benchmarks this afternoon show a much smaller
> slowdown, in the region of 1%.
>
> In these benchmarks, I used the following function:
>
>     (defun time-scroll-b (&optional arg)
>       (condition-case nil
>           (while t
>             (if arg (scroll-down) (scroll-up))
>             (sit-for 0))
>         (error nil)))
>
> in conjunction with benchmark-run in native-compiled Emacsen started
> with emacs -Q on buffer src/xdisp.c.  Both repositories were brought up
> to date yesterday evening, and master merged into the branch.
>
> Before each timing run I typed and deleted the character at the start of
> the buffer (to wipe out the fontification) and executed M-x
> garbage-collect.
>
> Here are the four timings I did on master:
>       o - (20.798601883 460 7.728701306)
>       o - (20.947118356 295 7.1172684969999995)
>       o - (20.941589929 293 7.144901186)
>       o - (20.917180235 293 7.136285445000002)
>
> ..  Here are the same four timings on scratch/correct-warning-pos):
>       o - (20.854543266 480 7.691123986)
>       o - (21.064465459 320 7.189660959000001)
>       o - (21.143813105 318 7.287708998000001)
>       o - (21.115932422 318 7.266432223999999)
>
> ..  It can be seen that the branch is around 1% slower than master.  This
> minor slowdown should be worth it to get correct positions in compile
> warning messages.
>
> So, I would propose to merge the branch into master now.  What do people
> (in particular Eli) say about this?

Hi Alan,

have you tried running elisp-benchmarks [1]?  I think this should give a
closer feedback on the perf impact of the execution engine.

Thanks

  Andrea

[1] <https://elpa.gnu.org/packages/elisp-benchmarks.html>