[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH] (Updated) Run hook when variable is set
From: |
Kelly Dean |
Subject: |
[PATCH] (Updated) Run hook when variable is set |
Date: |
Mon, 09 Feb 2015 03:24:51 +0000 |
Stefan Monnier wrote:
> No, the idea was rather to do:
>
> (defun set-internal-1 (args)
> (block nil ; Because Elisp isn't CL
> (if (constant-or-hooked-p)
> (if (constant-p)
> (if (forbidden-p)
> (error "setting constant")
> (return))
> (funcall symbol-watch-function ..args..))
> (do-some-stuff)
> (and-many-more-lines-of-stuff)
> (set-some-variable))))
Then the many more lines of stuff have to be copied into symbol-watch-function.
Those lines can't just be skipped; they're necessary for correctly setting
localized and forwarded symbols. By default, hooking a symbol must not change
the behavior of Emacs, except for slowing it down. A hook handler can
optionally change the behavior, but just hooking the symbol must cause no
change if the handler doesn't.
I decided to solve the problem by factoring out the many more lines of stuff to
a separate function, so they don't have to be duplicated. This introduces an
extra function call for them, but that's ok, since setting localized and
forwarded symbols isn't the hot path (and they're already slow anyway). This
change has no effect on the hot path.
> I think watchpoints usually have the functionality of catching the
> modifications, being able to see the "before-change value" and being
> able to replace the assignment with something else. So "watch" makes
> a lot of sense to me.
Um, ok. I already improved the names, but I can change them again if necessary.
Updated patches attached. The first applies to 24.4. The second applies to
trunk (using «patch», not «git apply»).
Feature improvements:
It reports the environment as «dyn-local» rather than «invalid» for set-default
within a dynamic let-binding, since you said that's a valid thing to do.
It passes not only the new value being set, but also the current value of the
variable.
The function names are more informative.
The API now uses function advice instead of a standard hook. Whenever a hooked
variable is set, symbol-setter-function is called. Since it's the setter
function, it's natural that overriding it would override the setting that
occurs, so this is how I implemented the API. As a result, it's not necessary
to explicitly set the variable in your handler, which means it's not necessary
to temporarily unhook it either; all you have to do is return the value you
want to set it to. See the docstring for symbol-setter-function for details.
Performance improvements:
I put «hooked» into «constant», and removed my branches from the hot paths of
specbind and set_internal.
I did the minor optimizations of set_internal I mentioned previously, plus the
relevant one in specbind and in find_symbol_value. Also in a few others that
aren't on hot paths, but they were trivial so I did them anyway.
I inlined grow_specpdl, which speeds up specbind, which more than compensates
for the conditional branch that varhook requires in unbind_to. I also inlined
the first part (the hot path) of set_internal (i.e. of set_internal_1 in my
patch). Both of those are fairly small and not used in very many places. The
cost is an increase in the size of the Emacs executable by about 20kB (out of
18.9MB on my system, so that's about 0.1%).
The result of these changes is a measurable performance improvement compared to
unmodified Emacs for the common cases, i.e. for setting and dynamically binding
plainval (non-buffer-local, non-forwarded) symbols. So now, my patch actually
_does_ make Emacs faster. ;-) Here are the benchmarks:
(require 'cl)
(defun test ()
(setq x 0)
(benchmark-run-compiled 100000 (incf x)))
(defun test1 ()
(setq x 0)
(benchmark-run-compiled
(do () ((> x 100000)) (incf x))))
(defun test2 ()
(benchmark-run-compiled 1000
(letrec
((foo
(lambda (x)
(let ((x (1+ x)))
(if (> x 100)
x
(1- (funcall foo x)))))))
(funcall foo 0))))
Execution time results to 4 significant digits:
test
minimum average maximum
Emacs 24.4 0.006253 0.007259 0.009721
with patch 0.001816 0.002148 0.002425
Average improvement: 70%
test1
minimum average maximum
Emacs 24.4 0.01074 0.01075 0.01078
with patch 0.009081 0.009301 0.009671
Average improvement: 13%
test2
minimum average maximum
Emacs 24.4 0.04401 0.04409 0.04417
with patch 0.04196 0.04202 0.04207
Average improvement: 4.7%
For all three tests, the slowest run with the patch was faster than the fastest
run without the patch.
varhook-advice-24.4.patch.gz
Description: Binary data
varhook-advice.patch.gz
Description: Binary data
- Re: [PATCH] Run hook when variable is set, (continued)
- Re: [PATCH] Run hook when variable is set, Kelly Dean, 2015/02/02
- Re: [PATCH] Run hook when variable is set, Stefan Monnier, 2015/02/02
- Re: [PATCH] Run hook when variable is set, Kelly Dean, 2015/02/03
- Re: [PATCH] Run hook when variable is set, Stefan Monnier, 2015/02/03
- [PATCH] (Updated) Run hook when variable is set, Kelly Dean, 2015/02/04
- Re: [PATCH] (Updated) Run hook when variable is set, Stefan Monnier, 2015/02/05
- Re: [PATCH] (Updated) Run hook when variable is set, Kelly Dean, 2015/02/06
- Re: [PATCH] (Updated) Run hook when variable is set, Stefan Monnier, 2015/02/06
- Re: [PATCH] (Updated) Run hook when variable is set, Kelly Dean, 2015/02/07
- Re: [PATCH] (Updated) Run hook when variable is set, Stefan Monnier, 2015/02/07
- [PATCH] (Updated) Run hook when variable is set,
Kelly Dean <=
- Re: [PATCH] (Updated) Run hook when variable is set, Stefan Monnier, 2015/02/12
- [PATCH] (Updated) Run hook when variable is set, Kelly Dean, 2015/02/13
- Re: [PATCH] (Updated) Run hook when variable is set, Stefan Monnier, 2015/02/13
- Re: [PATCH] (Updated) Run hook when variable is set, Kelly Dean, 2015/02/14
- Re: [PATCH] (Updated) Run hook when variable is set, Stefan Monnier, 2015/02/15
- [PATCH] (Updated) Run hook when variable is set, Kelly Dean, 2015/02/16
- Re: [PATCH] (Updated) Run hook when variable is set, Richard Stallman, 2015/02/17
- The purpose of makunbound (Was: Run hook when variable is set), Kelly Dean, 2015/02/17
- Re: The purpose of makunbound, Stefan Monnier, 2015/02/18
- Re: The purpose of makunbound, Kelly Dean, 2015/02/18