bug#75358: [scratch/elisp-benchmarks] repeated runs don't apply selector

bug#75358: [scratch/elisp-benchmarks] repeated runs don't apply selector

[Pip Cet via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On scratch/elisp-benchmarks, the following emacs -Q invocation:

./src/emacs --batch -Q -l ./elisp-benchmarks/elisp-benchmarks.el --eval '(progn (elisp-benchmarks-run) (delete-other-windows) (elisp-benchmarks-run "bubble"))'

is equivalent to

./src/emacs --batch -Q -l ./elisp-benchmarks/elisp-benchmarks.el --eval '(progn (elisp-benchmarks-run) (delete-other-windows) (elisp-benchmarks-run))'

but

./src/emacs --batch -Q -l ./elisp-benchmarks/elisp-benchmarks.el --eval '(progn (elisp-benchmarks-run "bubble") (delete-other-windows) (elisp-benchmarks-run))'

is not.

(The (delete-other-windows) is necessary because of other issues).

The expected behavior is for the SELECTOR to determine the tests to be
executed, as described in the documentation; in reality, it merely
determines which tests are compiled, then all symbols matching the
regexp produced below are executed.

In the first example, the second SELECTOR argument has no effect,
because the tests to be run are determined as:

	           (mapatoms (lambda (s)
	                      (let ((name (symbol-name s)))
	                        (when (and (fboundp s)
	                                   (string-match
	                                    "\\`elb-\\(.*\\)-entry\\'" name))
	                          (push (match-string 1 name) names)))))

without respecting the SELECTOR argument in any fashion: any elisp
function that happens to match the symbol format is run.

"Fixing" this by applying the selector as a regular expression to the
"name" value seems simple enough, as a stop-gap.  But: IMHO, reserving
chunks of the symbol namespace for special uses is unsatisfactory and
should be avoided.  This is true in both interactive and
non-interactive emacs runs.  Running benchmarks in a live Emacs
session can be useful; running functions based on their symbol name
alone is dangerous.

Of course, the selector is currently applied to file names, not test
names, so we'd have to decide what (elisp-benchmarks-run "font-lock"),
for example, is supposed to do.

IMHO, the proper fix is to use a working test registration mechanism,
ideally ERT.

bug#75358: [scratch/elisp-benchmarks] repeated runs don't apply selector

[Andrea Corallo]

Pip Cet via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs@gnu.org> writes:

> On scratch/elisp-benchmarks, the following emacs -Q invocation:
>
> ./src/emacs --batch -Q -l ./elisp-benchmarks/elisp-benchmarks.el --eval '(progn (elisp-benchmarks-run) (delete-other-windows) (elisp-benchmarks-run "bubble"))'
>
> is equivalent to
>
> ./src/emacs --batch -Q -l ./elisp-benchmarks/elisp-benchmarks.el --eval '(progn (elisp-benchmarks-run) (delete-other-windows) (elisp-benchmarks-run))'
>
> but
>
> ./src/emacs --batch -Q -l ./elisp-benchmarks/elisp-benchmarks.el --eval '(progn (elisp-benchmarks-run "bubble") (delete-other-windows) (elisp-benchmarks-run))'
>
> is not.
>
> (The (delete-other-windows) is necessary because of other issues).
>
> The expected behavior is for the SELECTOR to determine the tests to be
> executed, as described in the documentation; in reality, it merely
> determines which tests are compiled, then all symbols matching the
> regexp produced below are executed.
>
> In the first example, the second SELECTOR argument has no effect,
> because the tests to be run are determined as:
>
> 	           (mapatoms (lambda (s)
> 	                      (let ((name (symbol-name s)))
> 	                        (when (and (fboundp s)
> 	                                   (string-match
> 	                                    "\\`elb-\\(.*\\)-entry\\'" name))
> 	                          (push (match-string 1 name) names)))))
>
> without respecting the SELECTOR argument in any fashion: any elisp
> function that happens to match the symbol format is run.
>
> "Fixing" this by applying the selector as a regular expression to the
> "name" value seems simple enough, as a stop-gap.

Yep, as mentioned elisp-benachmarks was made (in order to give
meaningful results) to run from a known state, running two times will
not give meaningful results.  Anyway applying the selector to the runner
as well seems a satisfactory fix to me.

> But: IMHO, reserving
> chunks of the symbol namespace for special uses is unsatisfactory and
> should be avoided.  This is true in both interactive and
> non-interactive emacs runs.  Running benchmarks in a live Emacs
> session can be useful; running functions based on their symbol name
> alone is dangerous.
>
> Of course, the selector is currently applied to file names, not test
> names, so we'd have to decide what (elisp-benchmarks-run "font-lock"),
> for example, is supposed to do.

We can also decide to apply it only to test names to disambiguate if we
prefer.

bug#75358: [scratch/elisp-benchmarks] repeated runs don't apply selector

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> The expected behavior is for the SELECTOR to determine the tests to be
>> executed, as described in the documentation; in reality, it merely
>> determines which tests are compiled, then all symbols matching the
>> regexp produced below are executed.

Originally, there was only one benchmark per file, so SELECTOR
selects the files.  Maybe we should simply always load all the files and
then use SELECTOR to select the `elb-*-entry` functions?


        Stefan

bug#75358: [scratch/elisp-benchmarks] repeated runs don't apply selector

[Andrea Corallo]

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

>>> The expected behavior is for the SELECTOR to determine the tests to be
>>> executed, as described in the documentation; in reality, it merely
>>> determines which tests are compiled, then all symbols matching the
>>> regexp produced below are executed.
>
> Originally, there was only one benchmark per file, so SELECTOR
> selects the files.  Maybe we should simply always load all the files and
> then use SELECTOR to select the `elb-*-entry` functions?

+1