Re: Lisp watchpoints

[Noam Postavsky <npostavs@users.sourceforge.net>]

On Sun, Nov 29, 2015 at 2:54 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Sun, 29 Nov 2015 14:40:57 -0500
>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, John Wiegley <jwiegley@gmail.com>, emacs-devel@gnu.org
>>
>> > The call to ARRAYELTS should be outside of the loop (for those poor
>> > souls who run unoptimized builds most of the time).
>>
>> Compiling the following shows gcc folds the constants even with -O0
>> (also, notify_variable_watchers() should only be called for a few
>> selected symbols so I don't think it would really slow things down
>> very much).
>
> That depends om your GCC version, and then there are -fFOO switches
> that can control each specific optimizations.  And seeing that in the
> code simply looks wrong, regardless.  So let's move it out, please.

Since ARRAYELTS is a compile-time constant, to me it looks completely
redundant to move it out of the loop. Would it help if I added a
WATCHER_NUMBER_LIMIT to the enum after WATCHER_NUMBER_SET_REDISPLAY
and used that instead?

>
>> >> +  DEFVAR_INT ("set-redisplay-internal-watcher-number",
>> >> +              Vset_redisplay_internal_watcher_number,
>> >> +              doc: /* Internal watch function constant.  */);
>> >> +  Vset_redisplay_internal_watcher_number = WATCHER_NUMBER_SET_REDISPLAY;
>> >> +  make_symbol_constant (intern_c_string ("set-redisplay-internal-watcher-number"));
>> >
>> > I'd prefer if all this were moved to window.c.  data.c has no business
>> > with display-related issues.
>>
>> Hmm, then how should we make the connection between watcher numbers
>> and C function pointers? I think data.c should be in charge of that.
>
> What connection?  I'm afraid I'm not following.

The watchers_table holds all of the C functions that can be used as
watchers. I think it makes sense to have this table in data.c. Lisp
code that wants to add a watcher to a variable needs to know which
number to use (0 for set_redisplay, because it's the 1st (and only)
entry in the table). Since data.c has the table, I figured it makes
sense that it would be in charge of exporting the number->Cpointer
mapping to lisp. Hence, set-redisplay-internal-watcher-number is
defined in data.c.

If window.c would be in charge of defining
set-redisplay-internal-watcher-number, then it would need to know the
right number, which would probably mean moving the
WATCHER_NUMBER_SET_REDISPLAY definition to a header file away from the
watcher_table definition, which would be suboptimal, I think.

Or I guess we could just hard code the number 0 with a comment to look
in data.c; is that too dirty?