Re: New "make benchmark" target

[Pip Cet via "Emacs development discussions." <emacs-devel@gnu.org>]

"Eli Zaretskii" <eliz@gnu.org> writes:

Top-posted TL;DR: let's call Andrea's code "make elisp-benchmarks" and
include it now?  That would preserve the Git history and importantly (to
me) reserve the name for now.

>> Date: Mon, 30 Dec 2024 15:49:30 +0000
>> From: Pip Cet <pipcet@protonmail.com>
>> Cc: acorallo@gnu.org, stefankangas@gmail.com, mattiase@acm.org, eggert@cs.ucla.edu, emacs-devel@gnu.org, joaotavora@gmail.com
>>
>> >> https://lists.gnu.org/archive/html/emacs-devel/2024-12/msg00595.html
>> >
>> > Thanks, but AFAICT this just says that you intended to use/extend ERT
>> > to run this benchmark suite, but doesn't explain why you think using
>> > ERT would be an advantage worthy of keeping.
>>
>> I think some advantages are stated in that email: the ERT tagging
>> mechanism is more general, works, and can be extended (I describe one
>> such extension).  All that isn't currently true for elisp-benchmarks.
>
> Unlike the rest of the test suite, where we need a way to be able to
> run individual tests, a benchmark suite is much more likely to run as
> a whole, because benchmarking a single kind of jobs in Emacs is much
> less useful than producing a benchmark of a representative sample of
> jobs.  So I'm not sure this particular aspect is such a serious

Not my experience.  Running the entire suite is much more likely not to
produce usable data due to such issues as CPU thermal management (for
example: the first few tests are run at full clock speed and heat up the
system so much that thermal throttling is activated; the next few tests
are run at a reduced rate while the fan is running; eventually we run
out of amperes that we're allowed to drain the battery by and reduce
clock speed even further; this results in reduced temperature, so the
fan speed is reduced, which means we will eventually decide to try a
higher clock speed again, which will work for a while only before
repeating the cycle.  The whole thing will appear regular enough we
won't notice the data is bad, but it will be, until we rerun the test on
the same system in a different room and get wildly different results).
A single-second test run in a loop produces the occasional mid-stream
result which is actually useful (and promptly lost to the averaging
mechanism of elisp-benchmarks).

Benchmarking is hard, and I wouldn't have provided this very verbose
example if I hadn't seen "paradoxical" results that can only be
explained by such mechanisms.  We need to move away from average run
times either way, and that requires code changes.

And I don't usually run ERT tests individually, while I'm trying to get
in the habit of running the (non-expensive) test suite before I push.

> problem, certainly if it makes the job of adding the suite much
> harder.

I don't think time-to-Emacs is very different for the two approaches.
The difference is the post-merge work.

>> The other big difference is resource management, which elisp-benchmarks
>> does via a global variable, reusing one test as data for another.  ERT
>> has a somewhat better mechanism.
>
> This is just an argument for cleaner, more elegant code, right?  If
> so, I think we can live with the issue.  Having a ready-t-run
> benchmark suite is so much more important that I'm prepared to make
> compromises.
>
>> > Why do you think it is wrong to do the (AFAIU) simple change that
>> > Andrea proposed?
>>
>> Because it's a de facto commitment to not doing it in ERT.
>
> Probably, but that in itself is not a catastrophe, surely?

Hmm.  I disagree, but I think the real catastrophe would be wasting the
make target name, not the inclusion of another directory.

My suggestion is a compromise: add "make elisp-benchmarks" now, using
Andrea's code, then consider more complicated ERT-based approaches
without being in any hurry to do so.  But, also, let's agree that the
ERT-based approaches are "allowed" to reuse the elisp-benchmarks code
without providing comparable results or a porting mechanism, and keep
the "make benchmark" name reserved for a while.

My prediction is that it will turn out "make elisp-benchmarks" doesn't
usually provide very useful results, and expansion of the test framework
to produce useful results is easier reusing the ERT framework.

>> It might be relevant that elisp-benchmarks hasn't seen very active
>> development lately.  I think switching to ERT might help there, if only
>> because of the mailing list traffic.
>
> My vote is to get the job done the fastest way possible.  Tests are

Well, the ERT patch is ready "right now" (meaning it needs rebasing).
The elisp-benchmarks code would require, at least, whitespace fixes ;-)

> not Emacs itself, they are means towards a certain end, and so can use
> less clean code and designs, at least IMO.

To be perfectly honest, I was worried about the commit history because
in the Good Old CVS Days, file renames were more of a problem than they
are with git.  With git, I would prefer one "this is elisp-benchmarks"
commit even if the subsequent history modifies and moves those files.
So no reason not to do that now?  Thanks for your patience.

I can prepare a git branch doing that.  As for the git history rewriting
magic code, I've heard from Joao and the other developer involved; he
doesn't have a copyright assignment, but also thinks this is small
enough to be paperwork-exempt, so we can (probably) include a script in
admin/ to migrate elpa packages to core.

My preference would be a top-level directory called "elisp-benchmarks",
but ultimately that's a minor question, so just let me know the
preferred destination.

Pip