Subject: bug#48264: [PATCH v4 00/15] Speeding up setting the default for DEFVAR_PER_BUFFER vars

From: Spencer Baugh <[email-masked]>
Date: Fri, 7 May 2021 22:08:51 -0400

This patch series fixes bug#48264 by speeding up changes to the
default value for DEFVAR_PER_BUFFER vars, whether by let or
set-default.  Such changes are now constant time, and run as fast as
changes to non-default values.

This change optimizes setting the default in exchange for a small
slowdown on every access to a DEFVAR_PER_BUFFER var that has a
default.  I've benchmarked this change (results posted in other
threads on emacs-devel) and found minimal slowdown in pure Lisp code,
and a 1-2% slowdown in the display engine.

=== Background on DEFVAR_PER_BUFFER variables ===

DEFVAR_PER_BUFFER is a C macro which sets up a correspondence between a
Lisp symbol and a field of type Lisp_Object in the C `struct buffer'
struct.  There are a finite number of such fields, and DEFVAR_PER_BUFFER
is called for a subset of them.

Each DEFVAR_PER_BUFFER variable appears to Lisp code as a buffer-local
variable, and should behave like pure Lisp buffer-local variables.  Each
DEFVAR_PER_BUFFER variable is either permanently buffer-local, or has a
default value which is used by buffers that don't currently have a
buffer-local binding.

If a buffer has a buffer-local binding for one of these variables, then
the per-buffer value is stored in the corresponding field in that
buffer's `struct buffer'.

Default values for these variables are stored in a global `struct
buffer' C variable, `buffer_defaults'.  This struct does not correspond
to a real buffer, and its non-DEFVAR_PER_BUFFER fields are unused.  When
a variable's default value is changed, the corresponding field is
changed in (at least) buffer_defaults.  When `default-value' is used to
read a DEFVAR_PER_BUFFER variable's default value, it's read from
buffer_defaults.

The underlying fields in `struct buffer' used with DEFVAR_PER_BUFFER are
also read and written directly from C, through the BVAR macro.  The BVAR
macro takes a pointer to a `struct buffer' and the name of a field in
`struct buffer', and evaluates to the value for that field.

The variables must behave the same both when used through the symbol in
Lisp, and used through BVAR in C.  For example, if BVAR reads a field
from a buffer that does not have a buffer-local binding for that field,
it should evaluate to the default value for that field.

=== Old implementation ===

In the old implementation, both the permanently buffer-local
DEFVAR_PER_BUFFER variables and the variables with defaults values
were accessed in the same way: Through the BVAR macro.

The BVAR macro is essentially a no-op.  It turns into a simple field
dereference, essentially:

  #define BVAR(buf, field) (buf)->field

We simply read the field directly out of the specific `struct buffer'
we're working with.  Neither BVAR nor surrounding C code checks whether
a buffer has a buffer-local binding for a variable before using this
field, and at no point does most code check what value is in
buffer_defaults.

This is fine for permanently buffer-local DEFVAR_PER_BUFFER variables,
which have no default value.

For variables which are not permanently buffer-local, though, this means
we need to ensure that the C Lisp_Object field always contains the
"correct" value for the variable, whether that's a currently
buffer-local value, or the global default value.

If there is a buffer-local binding, then the field contains the
per-buffer value, and all is well.

If there is no buffer-local binding, then we need to ensure that the
field contains the global default value.

To do this, whenever the global default value changes, we iterate over
all buffers, and if there's no buffer-local binding, set the field to
the new default value. This is O(n) in the number of buffers open in
Emacs - quite slow.

= Old implementation: local_flags and buffer_local_flags =

Also, we frequently need to know whether a variable has a buffer-local
binding.  We maintain this information with the `local_flags' field in
`struct buffer', which is a char array with an entry for each
DEFVAR_PER_BUFFER variable.

When we create or kill a buffer-local binding for a DEFVAR_PER_BUFFER
variable, we update local_flags.

To support local_flags, we need even further metadata;
buffer_local_flags is a global, initialized at startup and constant
after that, which maps the offsets of the DEFVAR_PER_BUFFER C fields to
indices in local_flags.  We perform a lookup in buffer_local_flags
whenever we need to change local_flags for a variable.

=== New implementation ===

In the new implementation, we use separate macros for permanently
buffer-local variables and for variables with defaults.

Permanently buffer-local variables use BVAR, which stays the same.

Variables with defaults now use BVAR_OR_DEFAULT, which does a bit more
work.
Simplifying a bit:

  #define BVAR_OR_DEFAULT(buf, field) \
    EQ ((buf)->field, Qunbound) ? buffer_defaults.field : (buf)->field

We now use Qunbound as a sentinel to tell us whether the buffer has a
buffer-local binding for this field or not.  If the field contains
Qunbound, we should use the default value out of buffer_defaults; if
not, there's a buffer-local binding, and we should use the per-buffer
value.

We no longer need to iterate over all buffers whenever we change the
default value.  So setting the default value is now fast.

= New implementation: No more local_flags and buffer_local_flags =

Both of these are now useless and can be deleted.

When we create a buffer-local binding for a DEFVAR_PER_BUFFER variable,
we simply set the field.  When we kill a buffer-local binding, we set
the field to Qunbound.  There's no additional metadata.

=== Individual commits ===

See the individual commit messages for more.

  Stop checking the constant default for enable_multibyte_characters

This removes a defunct default that still existed for
enable_multibyte_characters, making it work like all the other
permanently buffer-local variables, which simplifies the rest of our
changes.

  Take offset not idx in PER_BUFFER_VALUE_P
  Add and use BVAR_HAS_DEFAULT_VALUE_P
  Combine unnecessarily separate loops in buffer.c
  Add and use KILL_PER_BUFFER_VALUE
  Rearrange set_internal for buffer forwarded symbols

These change or add abstractions for accessing buffer Lisp_Object
fields, to remove dependencies on implementation details which will
change.

  Use BVAR_OR_DEFAULT for per-buffer vars with defaults

This is the heart of the patch series; it add BVAR_OR_DEFAULT as
described above.

  Remove unnecessary Qunbound check
  Get rid of buffer_permanent_local_flags array
  Delete SET_PER_BUFFER_VALUE_P and buffer local_flags field
  Set buffer_defaults fields without a default to Qunbound
  Assert that PER_BUFFER_IDX for Lisp variables is not 0
  Remove PER_BUFFER_IDX and buffer_local_flags

These remove various forms of metadata maintained in buffer.c which
are no longer necessary after our change.

  Add and use BVAR_FIELD macros

This enforces statically that BVAR_OR_DEFAULT is only used for fields
with defaults, and that BVAR is only used for fields without defaults.

Changes from v3:
- Fixed formatting to correctly use tabs, not spaces
- Renamed BUFFER_DEFAULT_VALUE_P to BVAR_HAS_DEFAULT_VALUE_P.
- Removed special case for {syntax,category}_table in init_buffer_once
- Miscellaneous formatting fixes

Spencer Baugh (14):
  Stop checking the constant default for enable_multibyte_characters
  Take offset not idx in PER_BUFFER_VALUE_P
  Add and use BVAR_HAS_DEFAULT_VALUE_P
  Combine unnecessarily separate loops in buffer.c
  Add and use KILL_PER_BUFFER_VALUE
  Rearrange set_internal for buffer forwarded symbols
  Use BVAR_OR_DEFAULT for per-buffer vars with defaults
  Remove unnecessary Qunbound check
  Get rid of buffer_permanent_local_flags array
  Delete SET_PER_BUFFER_VALUE_P and buffer local_flags field
  Set buffer_defaults fields without a default to Qunbound
  Assert that PER_BUFFER_IDX for Lisp variables is not 0
  Remove PER_BUFFER_IDX and buffer_local_flags
  Add and use BVAR_FIELD macros

 lisp/bindings.el       |   3 +-
 src/alloc.c            |   3 +-
 src/bidi.c             |  19 +-
 src/buffer.c           | 443 ++++++++++++-----------------------------
 src/buffer.h           | 302 +++++++++++++---------------
 src/category.c         |  12 +-
 src/category.h         |   2 +-
 src/cmds.c             |   6 +-
 src/data.c             |  82 ++------
 src/editfns.c          |   4 +-
 src/emacs.c            |   4 +-
 src/fileio.c           |   9 +-
 src/fns.c              |   8 +-
 src/fringe.c           |  21 +-
 src/hbfont.c           |   2 +-
 src/indent.c           |  34 ++--
 src/msdos.c            |   6 +-
 src/pdumper.c          |   3 -
 src/print.c            |   6 +-
 src/process.c          |  15 +-
 src/search.c           |  21 +-
 src/syntax.c           |  13 +-
 src/syntax.h           |   7 +-
 src/window.c           |  29 +--
 src/xdisp.c            | 130 ++++++------
 test/src/data-tests.el |   1 -
 26 files changed, 455 insertions(+), 730 deletions(-)

-- 
2.31.1