bug#36190: 27.0.50; `put-text-property' etc. with buffer argument calls current buffer's `after-change-functions'

[Pip Cet]

On Thu, Jun 13, 2019 at 8:01 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Pip Cet <pipcet@gmail.com>
> > Date: Thu, 13 Jun 2019 19:42:29 +0000
> > Cc: 36190@debbugs.gnu.org
> >
> > > Not sure I have a clear idea of how you intend to use that additional
> > > argument.  Are you suggesting that we switch to that buffer?
> >
> > Yes:
> >
> > @@ -2183,6 +2184,9 @@ signal_after_change (ptrdiff_t charpos,
> > ptrdiff_t lendel, ptrdiff_t lenins)
> >    if (inhibit_modification_hooks)
> >      return;
> >
> > +  record_unwind_current_buffer ();
> > +  set_buffer_internal (buffer);
>
> Ugh! switching buffers just to run a hook!
>  This will kill performance
> in some cases.

I really don't think it will have a noticeable impact on performance,
but if you can think of a scenario, we could try to fix it.

> We had something similar with JSON parsing a few
> months ago.
> I wish we had a better alternative.

(Such as not calling regular modification hooks for text property changes?)

>  Maybe we should warn
> in the documentation that calling these functions with BUFFER being
> other than the current buffer might hurt performance when
> after-change-functions is non-nil.

It'll hurt performance even when after-change-functions is nil, so
such a warning would be overspecific.

> > As a practical matter, it's hard to change the text property functions
> > to use NULL when passed a nil argument
>
> How is it harder than passing current_buffer?

The code path goes through

  if (NILP (object))
    XSETBUFFER (object, current_buffer);

> It's really a matter of stylistic preferences, but you did ask why...

It was out of genuine interest, because passing NULL to implicitly
specify a default argument is something that people advocate against,
and that C in particular has a history of avoiding (stdout isn't NULL,
and post-VAX NULL isn't an empty string, for example). Thank you for
responding, and I'll change my patch accordingly.