Re: [PATCH 0/7] Cleanups and tests for DEFVAR_PER_BUFFER variables

[Eli Zaretskii <eliz@gnu.org>]

> From: Spencer Baugh <sbaugh@catern.com>
> Cc: emacs-devel@gnu.org, monnier@iro.umontreal.ca
> Date: Thu, 01 Apr 2021 14:13:50 -0400
> 
> OK, I ran some basic benchmarking. Specifically, I ran
> 
> (defun shr-benchmark ()
>   (let ((gc-cons-threshold 800000000))
>     (message "shr-benchmark result: %s"
>              (benchmark-run 100
>                (dolist (file (directory-files (ert-resource-directory) nil "\\.html\\'"))
>                  (shr-test (replace-regexp-in-string "\\.html\\'" "" file)))))))
> 
> out of shr-tests.el.
> 
> My results (each from running emacs -f shr-benchmark 10 times and
> discarding the first 2 results):
> 
> | Unmodified Emacs                  | 2.265s |
> | With original changes             | 2.295s |
> | Avoid unnecessary Qunbound checks | 2.266s |

Thanks, that's encouraging.  But we need a few more benchmarks, I
think.  One of them should be for redisplay, as it's a
performance-critical part of Emacs, and it accesses buffer-local
variables quite a lot.  So something like a benchmark of scrolling
through xdisp.c one line at a time in an interactive session would
probably give us an idea.

Another benchmark we frequently use is to remove all *.elc files and
then time "make" which byte-compiles them all.

> |            | Var with default             | Var without default      |
> |------------+------------------------------+--------------------------|
> | Status quo | BVAR (buf, field)            | BVAR (buf, field)        |
> | Option 1   | BVAR (buf, field)            | buf->field               |
> | Option 2   | BVAR (buf, field)            | BVAR_DIRECT (buf, field) |
> | Option 3   | BVAR_OR_DEFAULT (buf, field) | BVAR (buf, field)        |
> 
> (The specific names for BVAR_DIRECT or BVAR_OR_DEFAULT don't matter,
> we can make them shorter if we want).
> 
> What do people prefer?

Either 2 or 3 (perhaps with some slightly different names: maybe none
of the new macros should inherit the old name?).

> In any case we should ensure that we can't accidentally, for example,
> use BVAR_DIRECT on a field that actually has a default.  This should
> be easy enough by just naming the fields slightly differently.

Not sure what you mean by naming differently, but yes, we need to
detect incorrect usage at compile time, at least in the build with
"--enable-checking".

Thanks.