[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER
From: |
Stefan Monnier |
Subject: |
Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER |
Date: |
Mon, 30 Nov 2020 17:10:20 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
> Yes, it's probably a trade-off. My guess is that the extra overhead of
> BVAR will not be significant, but benchmarking is the only way to know
> for sure. (Indeed, it's possible things will speed up by removing the
> metadata, which reduces the size of the working set and lets more things
> stay in cache)
Maybe one way to figure it out is to magnify the impact: add extra
slowdown code to BVAR to make sure the slowdown is noticeable.
This might point to the place where a performance impact may be
measurable, or it might make it obvious that the impact is negligible.
> Well, with a completely naive static branch-predictor which assumes the
> Qunbound branch will never be taken, there will be no overhead for
> fields which are never Qunbound, namely non-DEFVAR_PER_BUFFER fields and
> permanent-buffer-locals. We could encourage that prediction by marking
> the Qunbound branch with an "unlikely()" macro using GCC's
> __builtin_expect.
While this might work for some vars, I suspect that it could be
detrimental for some.
> It's probably prematurely low-level for me to talk about such things
> right now though - benchmarking is the only way to know.
100% agreement. And modern branch predictors are quite sophisticated so
I expect they'll do a much better job than any hint we may be able
to provide.
> A DEFVAR_PER_BUFFER variable is normally marked as
> permanent-buffer-local by:
Indeed, there are two concepts: one if the concept that corresponds to
the `permanent-local` symbol property (that's implemented via
buffer_permanent_local_flags) and the other is the concept of
buffer-local variables which are always buffer-local and don't even have
a default value (that's the one your patch modifies, tho it only
modifies the implementation, not the semantics).
> - in the old version, setting -1 in the corresponding field in
> buffer_local_flags (which is a `struct buffer' distinct from
> buffer_permanent_local_flags which is an array of chars)
> - in the new version, setting the corresponding field in buffer_defaults
> to Qunbound (indicating there's no default value)
BTW, one potentially unintended side effect of your change is that
`makunbound` on PER_BUFFER vars won't work quite as usual. I guess we
could address that by using some other special value than Qunbound, tho
I'm not sure how important this is.
> All this is rather confusing and inconsistent; I don't think we should
> add more of these "pseudo-permanent" variables.
Agreed: I think we should get rid of this special category of "always
local" variables. This is a concept that only applies to the following
variables:
bset_filename (&buffer_local_flags, make_fixnum (-1));
bset_directory (&buffer_local_flags, make_fixnum (-1));
bset_backed_up (&buffer_local_flags, make_fixnum (-1));
bset_save_length (&buffer_local_flags, make_fixnum (-1));
bset_auto_save_file_name (&buffer_local_flags, make_fixnum (-1));
bset_read_only (&buffer_local_flags, make_fixnum (-1));
bset_major_mode (&buffer_local_flags, make_fixnum (-1));
bset_mode_name (&buffer_local_flags, make_fixnum (-1));
bset_undo_list (&buffer_local_flags, make_fixnum (-1));
bset_mark_active (&buffer_local_flags, make_fixnum (-1));
bset_point_before_scroll (&buffer_local_flags, make_fixnum (-1));
bset_file_truename (&buffer_local_flags, make_fixnum (-1));
bset_invisibility_spec (&buffer_local_flags, make_fixnum (-1));
bset_file_format (&buffer_local_flags, make_fixnum (-1));
bset_auto_save_file_format (&buffer_local_flags, make_fixnum (-1));
bset_display_count (&buffer_local_flags, make_fixnum (-1));
bset_display_time (&buffer_local_flags, make_fixnum (-1));
bset_enable_multibyte_characters (&buffer_local_flags, make_fixnum (-1));
I don't see the benefit of forcing those vars to always be buffer-local.
We could just as well make them "normal" and they'll still have
a buffer-local value pretty much all the time anyway.
Stefan
- [PATCH v2 15/16] Remove local_flags array in struct buffer, (continued)
- [PATCH v2 16/16] Remove usage of buffer_local_flags, Spencer Baugh, 2020/11/21
- Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER, Kévin Le Gouguec, 2020/11/22
- Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER, Eli Zaretskii, 2020/11/22
- Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER, Spencer Baugh, 2020/11/22
- Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER, Eli Zaretskii, 2020/11/22
- Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER, Spencer Baugh, 2020/11/29
- Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER, Eli Zaretskii, 2020/11/30
- Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER, Spencer Baugh, 2020/11/30
- Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER,
Stefan Monnier <=
- Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER, Andrea Corallo, 2020/11/30
- Re: [PATCH v2 00/16] Speeding up DEFVAR_PER_BUFFER, Stefan Monnier, 2020/11/22
- [PATCH 03/10] Use bset_last_selected_window everywhere, Spencer Baugh, 2020/11/19
- [PATCH 01/10] Take buffer field name in DEFVAR_PER_BUFFER, Spencer Baugh, 2020/11/19
- [PATCH 06/10] Disallow using BVAR as an lvalue, Spencer Baugh, 2020/11/19
- [PATCH 09/10] Access buffer_defaults in BVAR if there's no local binding, Spencer Baugh, 2020/11/19
- Re: [PATCH 09/10] Access buffer_defaults in BVAR if there's no local binding, Stefan Monnier, 2020/11/19
- Re: [PATCH 09/10] Access buffer_defaults in BVAR if there's no local binding, Eli Zaretskii, 2020/11/19
- [PATCH 05/10] Add BVAR_DEFAULT for access to buffer defaults, Spencer Baugh, 2020/11/19
- [PATCH 08/10] Make cache_long_scans buffer-local when setting it, Spencer Baugh, 2020/11/19