unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Spencer Baugh <sbaugh@catern.com>
To: emacs-devel@gnu.org
Cc: Spencer Baugh <sbaugh@catern.com>,
	Arnold Noronha <arnold@tdrhq.com>,
	Stefan Monnier <monnier@iro.umontreal.ca>,
	Dmitry Gutov <dgutov@yandex.ru>
Subject: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER
Date: Sat, 21 Nov 2020 21:34:29 -0500	[thread overview]
Message-ID: <cover.1606009917.git.sbaugh@catern.com> (raw)
In-Reply-To: <20201119153814.17541-1-sbaugh@catern.com>

This patch series reworks access to Lisp_Object fields in struct
buffer. The primary motivation is performance of DEFVAR_PER_BUFFER
Lisp variables, but this also removes several now-unnecessary pieces
of metadata and generally simplifies things.

Relative to v1, this patch series now performs a larger refactoring of
struct buffer field access, which should provide for good performance
while keeping the access method uniform.  I've also corrected style
issues and bugs pointed out in review, and added some testing for
buffer-local let variables.  (Thanks Stefan for the review, and the
tests you added in the fix for bug#44733, both were quite helpful.)

It's possible to provide further performance improvements, although
the current state of the patch series has minimal overhead, and the
uniformity is somewhat appealing.

If we do need to completely remove the overhead, we could do a mass
change to all the uses of BVAR, to introduce separate, maybe,
BVAR_LISP and BVAR_C macros, for fields which need fallback and fields
which don't.  I wasn't sure whether that would be acceptable.

I have an idea which would allow avoiding the mass change, if that's
desired, which also has some additional benefits.  We could define
getter functions (marked as inline) for each Lisp_Object field, and
have BVAR call those field-specific getter functions by forming the
name of the getter through token pasting.  The boilerplate of the
getters would be kept under control by using an X macro to define both
the fields in struct buffer and the getters. Something like:

#define FIELD_LIST \
  C_FIELD(name) \
  C_FIELD(filename) \
  ...
  LISP_FIELD(mode_line_format) \
  ...

And then "instantiating" FIELD_LIST twice in two different contexts to
define the Lisp_Object fields and the getters.  This would have some
additional benefits:
- it would make it a lot easier to tell which fields are C-only and
  which fields are accessed from Lisp
- we can attach more metadata to the individual fields
- we could also generate the existing hand-written setters from these
  macros

Regardless of what approach we take, at least some level of
benchmarking will be necessary.  Is there an established benchmark
suite for these kinds of changes?

(My commit messages are still not in ChangeLog style - sorry - with 16
patches in the series right now, and having not yet personally figured
out a good magit-integrated way to generate ChangeLog-appropriate
messages, I figured I'm better off adjusting the commit messages at
the end of the review process, rather than continually throughout. If
anyone has a recommendation for ChangeLog generation that is
integrated with magit, that would be welcome - I haven't found
anything yet...)

Spencer Baugh (16):
  Add a test for let-binding unwinding
  Assert not local-variable-p after setq in let_default binding
  Stop checking the constant default for enable_multibyte_characters
  Take buffer field name in DEFVAR_PER_BUFFER
  Add BVAR_DEFAULT for access to buffer defaults
  Use bset_* functions instead of BVAR
  Take offset not idx in PER_BUFFER_VALUE_P
  Combine unnecessarily separate loops in buffer.c
  Add and use BUFFER_DEFAULT_VALUE_P
  Add and use KILL_PER_BUFFER_VALUE
  Assert that PER_BUFFER_IDX for Lisp variables is not 0
  Rearrange set_internal for buffer forwarded symbols
  Get rid of buffer_permanent_local_flags array
  Remove unnecessary Qunbound check
  Remove local_flags array in struct buffer
  Remove usage of buffer_local_flags

 lisp/bindings.el       |   3 +-
 src/buffer.c           | 417 ++++++++++++-----------------------------
 src/buffer.h           | 145 ++++++--------
 src/category.c         |   4 -
 src/category.h         |   2 +-
 src/data.c             |  82 ++------
 src/fileio.c           |  12 +-
 src/pdumper.c          |   3 -
 src/print.c            |   6 +-
 src/process.c          |  15 +-
 src/syntax.c           |   4 -
 src/syntax.h           |   2 +-
 src/window.c           |   5 +-
 test/src/data-tests.el |  24 ++-
 14 files changed, 228 insertions(+), 496 deletions(-)

-- 
2.28.0




  parent reply	other threads:[~2020-11-22  2:34 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-14 16:34 ido-switch-buffer is slow with many buffers; others are fast catern
2020-11-14 18:22 ` Stefan Monnier
2020-11-14 20:06   ` sbaugh
2020-11-14 23:00     ` Stefan Monnier
2020-11-14 23:16 ` Dmitry Gutov
2020-11-15  0:19   ` Spencer Baugh
2020-11-15 15:15     ` Stefan Monnier
2020-11-15 20:49       ` Spencer Baugh
2020-11-15 23:58         ` Arnold Noronha
2020-11-19 15:38         ` [PATCH 00/10] Speeding up DEFVAR_PER_BUFFER (Was: ido-switch-buffer is slow) Spencer Baugh
2020-11-19 17:29           ` [PATCH 00/10] Speeding up DEFVAR_PER_BUFFER Stefan Monnier
2020-11-22  2:34           ` Spencer Baugh [this message]
2020-11-22  2:34             ` [PATCH v2 01/16] Add a test for let-binding unwinding Spencer Baugh
2020-11-25 20:53               ` Stefan Monnier
2020-11-30 17:31                 ` Spencer Baugh
2020-12-01 16:44               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 02/16] Assert not local-variable-p after setq in let_default binding Spencer Baugh
2020-11-25 20:54               ` Stefan Monnier
2020-12-01 16:45               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 03/16] Stop checking the constant default for enable_multibyte_characters Spencer Baugh
2020-11-25 20:57               ` Stefan Monnier
2020-12-01 16:52               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 04/16] Take buffer field name in DEFVAR_PER_BUFFER Spencer Baugh
2020-12-01 16:56               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 05/16] Add BVAR_DEFAULT for access to buffer defaults Spencer Baugh
2020-12-01 17:00               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 06/16] Use bset_* functions instead of BVAR Spencer Baugh
2020-12-01 17:12               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 07/16] Take offset not idx in PER_BUFFER_VALUE_P Spencer Baugh
2020-12-01 17:20               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 08/16] Combine unnecessarily separate loops in buffer.c Spencer Baugh
2020-12-01 17:22               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 09/16] Add and use BUFFER_DEFAULT_VALUE_P Spencer Baugh
2020-12-01 17:24               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 10/16] Add and use KILL_PER_BUFFER_VALUE Spencer Baugh
2020-12-01 17:26               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 11/16] Assert that PER_BUFFER_IDX for Lisp variables is not 0 Spencer Baugh
2020-12-01 17:32               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 12/16] Rearrange set_internal for buffer forwarded symbols Spencer Baugh
2020-12-01 17:35               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 13/16] Get rid of buffer_permanent_local_flags array Spencer Baugh
2020-11-22 16:16               ` Eli Zaretskii
2020-12-01 17:39               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 14/16] Remove unnecessary Qunbound check Spencer Baugh
2020-12-01 17:40               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 15/16] Remove local_flags array in struct buffer Spencer Baugh
2020-11-22 20:04               ` Stefan Monnier
2020-12-01 17:57               ` Eli Zaretskii
2020-11-22  2:34             ` [PATCH v2 16/16] Remove usage of buffer_local_flags Spencer Baugh
2020-12-01 18:05               ` Eli Zaretskii
2020-11-22 11:19             ` [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER Kévin Le Gouguec
2020-11-22 16:12             ` Eli Zaretskii
2020-11-22 16:28               ` Spencer Baugh
2020-11-22 17:13                 ` Eli Zaretskii
2020-11-29 17:41                   ` Spencer Baugh
2020-11-30 18:32                     ` Eli Zaretskii
2020-11-30 20:11                       ` Spencer Baugh
2020-11-30 22:10                         ` Stefan Monnier
2020-11-30 22:26                           ` Andrea Corallo via Emacs development discussions.
2020-12-01 15:15                           ` Eli Zaretskii
2020-12-01 15:56                             ` Stefan Monnier
2020-12-01 15:10                         ` Eli Zaretskii
2020-12-01 15:20                           ` Spencer Baugh
2020-12-01 16:01                           ` Stefan Monnier
2020-12-01 16:16                             ` Eli Zaretskii
2020-12-01 18:10                       ` Eli Zaretskii
2020-11-22 20:11                 ` Stefan Monnier
2020-11-19 15:38         ` [PATCH 01/10] Take buffer field name in DEFVAR_PER_BUFFER Spencer Baugh
2020-11-19 15:38         ` [PATCH 02/10] Add bset_save_length and use it Spencer Baugh
2020-11-19 15:38         ` [PATCH 03/10] Use bset_last_selected_window everywhere Spencer Baugh
2020-11-19 15:38         ` [PATCH 04/10] Use bset_enable_multibyte_characters everywhere Spencer Baugh
2020-11-19 15:38         ` [PATCH 05/10] Add BVAR_DEFAULT for access to buffer defaults Spencer Baugh
2020-11-19 15:38         ` [PATCH 06/10] Disallow using BVAR as an lvalue Spencer Baugh
2020-11-19 15:38         ` [PATCH 07/10] Reorder buffer.h for upcoming rework of BVAR Spencer Baugh
2020-11-19 15:38         ` [PATCH 08/10] Make cache_long_scans buffer-local when setting it Spencer Baugh
2020-11-19 15:38         ` [PATCH 09/10] Access buffer_defaults in BVAR if there's no local binding Spencer Baugh
2020-11-19 18:08           ` Stefan Monnier
2020-11-19 18:21             ` Eli Zaretskii
2020-11-19 15:38         ` [PATCH 10/10] Don't iterate over all buffers in set_default_internal Spencer Baugh
2020-11-19 18:21           ` Stefan Monnier

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=cover.1606009917.git.sbaugh@catern.com \
    --to=sbaugh@catern.com \
    --cc=arnold@tdrhq.com \
    --cc=dgutov@yandex.ru \
    --cc=emacs-devel@gnu.org \
    --cc=monnier@iro.umontreal.ca \
    /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).