unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Spencer Baugh <sbaugh@catern.com>
To: 48264@debbugs.gnu.org
Cc: Spencer Baugh <sbaugh@catern.com>
Subject: bug#48264: [PATCH v4 00/15] Speeding up setting the default for DEFVAR_PER_BUFFER vars
Date: Fri,  7 May 2021 22:08:51 -0400	[thread overview]
Message-ID: <20210508020905.13583-2-sbaugh@catern.com> (raw)
In-Reply-To: <20210508020905.13583-1-sbaugh@catern.com>

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






  reply	other threads:[~2021-05-08  2:08 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-06 20:24 bug#48264: 28.0.50; Changing the default for DEFVAR_PER_BUFFER variables takes O(#buffers) time Spencer Baugh
2021-05-06 21:33 ` bug#48264: [PATCH v3 00/15] Speeding up setting the default for DEFVAR_PER_BUFFER vars Spencer Baugh
2021-05-07 11:05   ` Eli Zaretskii
2021-05-08  2:08   ` bug#48264: [PATCH v4 " Spencer Baugh
2021-05-08  2:08     ` Spencer Baugh [this message]
2021-05-08  2:08     ` bug#48264: [PATCH v4 01/14] Stop checking the constant default for enable_multibyte_characters Spencer Baugh
2021-05-08  2:08     ` bug#48264: [PATCH v4 02/14] Take offset not idx in PER_BUFFER_VALUE_P Spencer Baugh
2021-05-08  2:08     ` bug#48264: [PATCH v4 03/14] Add and use BVAR_HAS_DEFAULT_VALUE_P Spencer Baugh
2021-05-08  2:08     ` bug#48264: [PATCH v4 04/14] Combine unnecessarily separate loops in buffer.c Spencer Baugh
2021-05-08  2:08     ` bug#48264: [PATCH v4 05/14] Add and use KILL_PER_BUFFER_VALUE Spencer Baugh
2021-05-08  2:08     ` bug#48264: [PATCH v4 06/14] Rearrange set_internal for buffer forwarded symbols Spencer Baugh
2021-05-08  2:08     ` bug#48264: [PATCH v4 07/14] Use BVAR_OR_DEFAULT for per-buffer vars with defaults Spencer Baugh
2021-05-08  2:08     ` bug#48264: [PATCH v4 08/14] Remove unnecessary Qunbound check Spencer Baugh
2021-05-08  2:09     ` bug#48264: [PATCH v4 09/14] Get rid of buffer_permanent_local_flags array Spencer Baugh
2021-05-08  2:09     ` bug#48264: [PATCH v4 10/14] Delete SET_PER_BUFFER_VALUE_P and buffer local_flags field Spencer Baugh
2021-05-08  2:09     ` bug#48264: [PATCH v4 11/14] Set buffer_defaults fields without a default to Qunbound Spencer Baugh
2021-05-08  2:09     ` bug#48264: [PATCH v4 12/14] Assert that PER_BUFFER_IDX for Lisp variables is not 0 Spencer Baugh
2021-05-08  2:09     ` bug#48264: [PATCH v4 13/14] Remove PER_BUFFER_IDX and buffer_local_flags Spencer Baugh
2021-05-08  2:09     ` bug#48264: [PATCH v4 14/14] Add and use BVAR_FIELD macros Spencer Baugh
2021-05-08 18:06     ` bug#48264: [PATCH v4 00/15] Speeding up setting the default for DEFVAR_PER_BUFFER vars Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-05-06 21:33 ` bug#48264: [PATCH v3 01/15] Stop checking the constant default for enable_multibyte_characters Spencer Baugh
2021-05-06 21:33 ` bug#48264: [PATCH v3 02/15] Take offset not idx in PER_BUFFER_VALUE_P Spencer Baugh
2021-05-07  7:27   ` Eli Zaretskii
2021-05-07 12:45     ` Spencer Baugh
2021-05-07 12:54       ` Eli Zaretskii
2021-05-06 21:33 ` bug#48264: [PATCH v3 03/15] Add and use BUFFER_DEFAULT_VALUE_P Spencer Baugh
2021-05-07  7:29   ` Eli Zaretskii
2021-05-07 12:49     ` Spencer Baugh
2021-05-07 12:58       ` Eli Zaretskii
2021-05-07 13:38         ` Spencer Baugh
2021-05-07 13:42           ` Eli Zaretskii
2021-05-07 14:30             ` Spencer Baugh
2021-05-07 14:39               ` Eli Zaretskii
2021-05-06 21:33 ` bug#48264: [PATCH v3 04/15] Combine unnecessarily separate loops in buffer.c Spencer Baugh
2021-05-06 21:33 ` bug#48264: [PATCH v3 05/15] Add and use KILL_PER_BUFFER_VALUE Spencer Baugh
2021-05-06 21:33 ` bug#48264: [PATCH v3 06/15] Rearrange set_internal for buffer forwarded symbols Spencer Baugh
2021-05-07 10:45   ` Eli Zaretskii
2021-05-07 14:26     ` Spencer Baugh
2021-05-06 21:33 ` bug#48264: [PATCH v3 07/15] Add BVAR_OR_DEFAULT macro as a stub Spencer Baugh
2021-05-07 10:54   ` Eli Zaretskii
2021-05-07 13:05     ` Spencer Baugh
2021-05-07 13:12       ` Eli Zaretskii
2021-05-07 13:24         ` Spencer Baugh
2021-05-07 13:32           ` Eli Zaretskii
2021-05-06 21:33 ` bug#48264: [PATCH v3 08/15] Set non-buffer-local BVARs to Qunbound Spencer Baugh
2021-05-07 10:37   ` Eli Zaretskii
2021-05-07 12:54     ` Spencer Baugh
2021-05-07 13:00       ` Eli Zaretskii
2021-05-06 21:33 ` bug#48264: [PATCH v3 09/15] Remove unnecessary Qunbound check Spencer Baugh
2021-05-06 21:33 ` bug#48264: [PATCH v3 10/15] Get rid of buffer_permanent_local_flags array Spencer Baugh
2021-05-06 21:33 ` bug#48264: [PATCH v3 11/15] Delete SET_PER_BUFFER_VALUE_P and buffer local_flags field Spencer Baugh
2021-05-06 21:33 ` bug#48264: [PATCH v3 12/15] Set buffer_defaults fields without a default to Qunbound Spencer Baugh
2021-05-07 10:42   ` Eli Zaretskii
2021-05-07 13:20     ` Spencer Baugh
2021-05-07 13:29       ` Eli Zaretskii
2021-05-07 14:15         ` Spencer Baugh
2021-05-07 14:30           ` Eli Zaretskii
2021-05-07 21:35             ` Spencer Baugh
2021-05-08  6:40               ` Eli Zaretskii
2021-05-08 13:22                 ` Spencer Baugh
2021-05-06 21:33 ` bug#48264: [PATCH v3 13/15] Assert that PER_BUFFER_IDX for Lisp variables is not 0 Spencer Baugh
2021-05-06 21:33 ` bug#48264: [PATCH v3 14/15] Remove PER_BUFFER_IDX and buffer_local_flags Spencer Baugh
2021-05-06 21:33 ` bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros Spencer Baugh
2021-05-07 11:03   ` Eli Zaretskii
2021-05-07 12:59     ` Spencer Baugh
2021-05-07 13:08       ` Eli Zaretskii
2021-05-07 21:43         ` Spencer Baugh
2021-05-08  6:55           ` Eli Zaretskii
2021-05-08 13:35             ` Spencer Baugh
2021-05-08 13:58               ` Eli Zaretskii
2021-05-08 17:13                 ` Spencer Baugh
2021-05-08 19:03                 ` Spencer Baugh
2021-05-09  8:10                   ` Eli Zaretskii
2021-05-09 17:09                     ` Spencer Baugh
2021-05-09 17:09                       ` bug#48264: [PATCH 1/2] Take buffer field name in DEFVAR_PER_BUFFER Spencer Baugh
2021-05-09 17:09                       ` bug#48264: [PATCH 2/2] Add compile-time check that BVAR is used correctly Spencer Baugh
2021-05-09 18:09                         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-05-09 18:29                           ` Eli Zaretskii
2021-05-10 14:09                             ` Spencer Baugh
2022-07-01 12:18                               ` bug#48264: 28.0.50; Changing the default for DEFVAR_PER_BUFFER variables takes O(#buffers) time Lars Ingebrigtsen
2022-07-01 18:49                                 ` Spencer Baugh
2022-07-02 12:00                                   ` Lars Ingebrigtsen
2022-08-02 11:11                                     ` Lars Ingebrigtsen
2021-05-09 10:08                 ` bug#48264: [PATCH v3 15/15] Add and use BVAR_FIELD macros Lars Ingebrigtsen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210508020905.13583-2-sbaugh@catern.com \
    --to=sbaugh@catern.com \
    --cc=48264@debbugs.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).