bug#31138: Native json slower than json.el

[yyoncho]

Hi Eli,

The patch seems to be addressing the hooks related issue. 

Thanks,

Ivan

On Mon, Mar 25, 2019 at 6:42 PM Eli Zaretskii <eliz@gnu.org> wrote:

> From: yyoncho <yyoncho@gmail.com>
> Date: Mon, 25 Mar 2019 07:44:00 +0200
> Cc: Sébastien Chapuis <sebastien@chapu.is>,
>       31138@debbugs.gnu.org
>
>  Is this with or without the patch I sent?
>
> It is with the patch (in the two profiler.el reports, the "kill" hooks are not present)

OK, thanks.

> Just found that it is not my setup which triggers the issue bug pressing C-g during the parsing:

Right, I think I see the reason for that.  The patch below should fix
this.  It also makes buffer-list-update-hook nil when creating and
killing the conversion buffer, for a good measure.  Please note that
the patch is relative to the current master, so be sure to run
"git checkout src/coding.c" before applying.

> I did exactly what you are saying here: I applied the patch and listed the effects:
> 1. The buffer-list-update-hook is called
> 2. The patch does not affect the perf difference between emacs -q and my setup.
>
> > I must rely on you in this matter, because I cannot reproduce the
> > slowdown on my system.
>
> I realize that and I am trying to be as helpful as possible by sharing all my observations.

OK, but please always say if your observations are before or after the
last patch I sent.

Thanks.

Here's the patch:

diff --git a/src/coding.c b/src/coding.c
index 905c7ce..c6d9643 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -7798,42 +7798,6 @@ static Lisp_Object Vcode_conversion_reused_workbuf;
 static bool reused_workbuf_in_use;


-/* Return a working buffer of code conversion.  MULTIBYTE specifies the
-   multibyteness of returning buffer.  */
-
-static Lisp_Object
-make_conversion_work_buffer (bool multibyte)
-{
-  Lisp_Object name, workbuf;
-  struct buffer *current;
-
-  if (reused_workbuf_in_use)
-    {
-      name = Fgenerate_new_buffer_name (Vcode_conversion_workbuf_name, Qnil);
-      workbuf = Fget_buffer_create (name);
-    }
-  else
-    {
-      reused_workbuf_in_use = 1;
-      if (NILP (Fbuffer_live_p (Vcode_conversion_reused_workbuf)))
-       Vcode_conversion_reused_workbuf
-         = Fget_buffer_create (Vcode_conversion_workbuf_name);
-      workbuf = Vcode_conversion_reused_workbuf;
-    }
-  current = current_buffer;
-  set_buffer_internal (XBUFFER (workbuf));
-  /* We can't allow modification hooks to run in the work buffer.  For
-     instance, directory_files_internal assumes that file decoding
-     doesn't compile new regexps.  */
-  Fset (Fmake_local_variable (Qinhibit_modification_hooks), Qt);
-  Ferase_buffer ();
-  bset_undo_list (current_buffer, Qt);
-  bset_enable_multibyte_characters (current_buffer, multibyte ? Qt : Qnil);
-  set_buffer_internal (current);
-  return workbuf;
-}
-
-
 static void
 code_conversion_restore (Lisp_Object arg)
 {
@@ -7846,7 +7810,12 @@ code_conversion_restore (Lisp_Object arg)
       if (EQ (workbuf, Vcode_conversion_reused_workbuf))
        reused_workbuf_in_use = 0;
       else
-       Fkill_buffer (workbuf);
+       {
+         ptrdiff_t count = SPECPDL_INDEX ();
+         specbind (Qbuffer_list_update_hook, Qnil);
+         Fkill_buffer (workbuf);
+         unbind_to (count, Qnil);
+       }
     }
   set_buffer_internal (XBUFFER (current));
 }
@@ -7857,9 +7826,51 @@ code_conversion_save (bool with_work_buf, bool multibyte)
   Lisp_Object workbuf = Qnil;

   if (with_work_buf)
-    workbuf = make_conversion_work_buffer (multibyte);
+    {
+      ptrdiff_t count = SPECPDL_INDEX ();
+      if (reused_workbuf_in_use)
+       {
+         Lisp_Object name
+           = Fgenerate_new_buffer_name (Vcode_conversion_workbuf_name, Qnil);
+         specbind (Qbuffer_list_update_hook, Qnil);
+         workbuf = Fget_buffer_create (name);
+         unbind_to (count, Qnil);
+       }
+      else
+       {
+         if (NILP (Fbuffer_live_p (Vcode_conversion_reused_workbuf)))
+           {
+             specbind (Qbuffer_list_update_hook, Qnil);
+             Vcode_conversion_reused_workbuf
+               = Fget_buffer_create (Vcode_conversion_workbuf_name);
+             unbind_to (count, Qnil);
+           }
+         workbuf = Vcode_conversion_reused_workbuf;
+       }
+    }
   record_unwind_protect (code_conversion_restore,
                         Fcons (Fcurrent_buffer (), workbuf));
+  if (!NILP (workbuf))
+    {
+      struct buffer *current = current_buffer;
+      set_buffer_internal (XBUFFER (workbuf));
+      /* We can't allow modification hooks to run in the work buffer.  For
+        instance, directory_files_internal assumes that file decoding
+        doesn't compile new regexps.  */
+      Fset (Fmake_local_variable (Qinhibit_modification_hooks), Qt);
+      Ferase_buffer ();
+      bset_undo_list (current_buffer, Qt);
+      bset_enable_multibyte_characters (current_buffer, multibyte ? Qt : Qnil);
+      if (EQ (workbuf, Vcode_conversion_reused_workbuf))
+       reused_workbuf_in_use = 1;
+      else
+       {
+         Fset (Fmake_local_variable (Qkill_buffer_query_functions), Qnil);
+         Fset (Fmake_local_variable (Qkill_buffer_hook), Qnil);
+       }
+      set_buffer_internal (current);
+    }
+
   return workbuf;
 }