Re: [RFC]: replace-region-contents

[Tassilo Horn]

Hi all,

if nobody complains, I'd like to install the following patch.

I've set too_expensive to 64 because then my benchmark was almost as
fast as the original version without `replace-buffer-contents', and I
couldn't find any difference in observable behavior in that value anyway
except lower values give much better performance but a too low value
like 10 will segfault.  My main concern was that point was at the same
position before and after pretty-printing my JSON.

I also enabled the heuristic flag, although I've seen no difference
without it.  The comments just suggest there are cases where performance
benefits from that.

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el
index 7d9f0bba4c..5618f1b478 100644
--- a/lisp/emacs-lisp/subr-x.el
+++ b/lisp/emacs-lisp/subr-x.el
@@ -250,6 +250,33 @@ string-remove-suffix
       (substring string 0 (- (length string) (length suffix)))
     string))
 
+(defun replace-region-contents (beg end replace-fn)
+  "Replace the region between BEG and END using REPLACE-FN.
+
+REPLACE-FN runs on the current buffer narrowed to the region.  It
+should return either a string or a buffer replacing the region.
+
+The replacement is performed using `replace-buffer-contents'.
+
+Note: If the replacement is a string, it'll be placed in a
+temporary buffer so that `replace-buffer-contents' can operate on
+it.  Therefore, if you already have the replacement in a buffer,
+it makes no sense to convert it to a string using
+`buffer-substring' or similar."
+  (save-excursion
+    (save-restriction
+      (narrow-to-region beg end)
+      (goto-char (point-min))
+      (let ((repl (funcall replace-fn)))
+       (if (bufferp repl)
+           (replace-buffer-contents repl)
+         (let ((source-buffer (current-buffer)))
+           (with-temp-buffer
+             (insert repl)
+             (let ((tmp-buffer (current-buffer)))
+               (set-buffer source-buffer)
+               (replace-buffer-contents tmp-buffer)))))))))
+
 (provide 'subr-x)
 
 ;;; subr-x.el ends here
diff --git a/lisp/json.el b/lisp/json.el
index 26cd48f41d..8f86104c19 100644
--- a/lisp/json.el
+++ b/lisp/json.el
@@ -52,6 +52,7 @@
 
 ;;; Code:
 
+(require 'subr-x)
 (require 'map)
 
 ;; Parameters
@@ -740,14 +741,14 @@ json-pretty-print-buffer
 (defun json-pretty-print (begin end)
   "Pretty-print selected region."
   (interactive "r")
-  (atomic-change-group
-    (let ((json-encoding-pretty-print t)
-          ;; Distinguish an empty objects from 'null'
-          (json-null :json-null)
-          ;; Ensure that ordering is maintained
-          (json-object-type 'alist)
-          (txt (delete-and-extract-region begin end)))
-      (insert (json-encode (json-read-from-string txt))))))
+  (let ((json-encoding-pretty-print t)
+        ;; Distinguish an empty objects from 'null'
+        (json-null :json-null)
+        ;; Ensure that ordering is maintained
+        (json-object-type 'alist))
+    (replace-region-contents
+     begin end
+     (lambda () (json-encode (json-read))))))
 
 (defun json-pretty-print-buffer-ordered ()
   "Pretty-print current buffer with object keys ordered."
diff --git a/src/editfns.c b/src/editfns.c
index a9ac263daf..7a600bacf1 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -1910,6 +1910,11 @@ determines whether case is significant or ignored.  */)
 
 #undef ELEMENT
 #undef EQUAL
+#define USE_HEURISTIC
+
+#ifdef USE_HEURISTIC
+#define DIFFSEQ_HEURISTIC
+#endif
 
 /* Counter used to rarely_quit in replace-buffer-contents.  */
 static unsigned short rbc_quitcounter;
@@ -2017,8 +2022,11 @@ differences between the two buffers.  */)
     .insertions = SAFE_ALLOCA (ins_bytes),
     .fdiag = buffer + size_b + 1,
     .bdiag = buffer + diags + size_b + 1,
+#ifdef DIFFSEQ_HEURISTIC
+    .heuristic = true,
+#endif
     /* FIXME: Find a good number for .too_expensive.  */
-    .too_expensive = 1000000,
+    .too_expensive = 64,
   };
   memclear (ctx.deletions, del_bytes);
   memclear (ctx.insertions, ins_bytes);
--8<---------------cut here---------------end--------------->8---

Bye,
Tassilo