all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Guido Kraemer <gkraemer@bgc-jena.mpg.de>
Cc: 33708@debbugs.gnu.org
Subject: bug#33708: 26.1.90; nhexl-mode performance
Date: Fri, 14 Dec 2018 13:40:36 -0500	[thread overview]
Message-ID: <jwv4lbg6jlr.fsf-monnier+emacsbugs@gnu.org> (raw)
In-Reply-To: <8d6352ac-4bca-fb76-8c2f-dd597e481e8e@bgc-jena.mpg.de> (Guido Kraemer's message of "Tue, 11 Dec 2018 18:06:40 +0100")

> Occurred in the files of the Bitcoin blockchain. The beginning of every
> block is marked with the byte sequence `f9beb4d9`. Searching for this
> byte sequence in nhexl-mode is really slow.

Duh, indeed it's painful, and it's a plain performance bug in nhexl-mode.
I just installed the patch below which seems to fix it (along with
another less severe bug).
The corresponding 1.2 package should appear soon on GNU ELPA.

Thanks for the test case (tho it was painful to get due to having to go
through my browser and coerce it to run non-free Javascript code).


        Stefan


diff --git a/packages/nhexl-mode/nhexl-mode.el b/packages/nhexl-mode/nhexl-mode.el
index 89d91182f..a52a90081 100644
--- a/packages/nhexl-mode/nhexl-mode.el
+++ b/packages/nhexl-mode/nhexl-mode.el
@@ -807,22 +807,31 @@ Return the corresponding nibble, if applicable."
       (push (string-to-number (substring string i (+ i 2)) 16)
             chars)
       (setq i (+ i 2)))
-    (let* ((base (regexp-quote (apply #'string (nreverse chars))))
-           (newstr
-            (if (>= i (length string))
-                base
-              (cl-assert (= (1+ i) (length string)))
-              (let ((nibble (string-to-number (substring string i) 16)))
-                ;; FIXME: if one of the two bounds is a special char
-                ;; like `]` or `^' we can get into trouble!
-                (format "%s[%c-%c]" base
-                        (* 16 nibble)
-                        (+ 15 (* 16 nibble)))))))
+    (let* ((base (regexp-quote (apply #'unibyte-string (nreverse chars))))
+           (re
+            (concat (if (>= i (length string))
+                        base
+                      (cl-assert (= (1+ i) (length string)))
+                      (let ((nibble (string-to-number (substring string i) 16)))
+                        ;; FIXME: if one of the two bounds is a special char
+                        ;; like `]` or `^' we can get into trouble!
+                        (concat base
+                                (unibyte-string ?\[ (* 16 nibble) ?-
+                                                   (+ 15 (* 16 nibble)) ?\]))))
+                    ;; We also search for the literal hex string here, so the
+                    ;; search stops as soon as one is found, otherwise we too
+                    ;; easily fall into the trap of bug#33708 where at every
+                    ;; cycle we first search unsuccessfully through the whole
+                    ;; buffer with one kind of search before trying the
+                    ;; other search.
+                    ;; Don't bother regexp-quoting the string since we know
+                    ;; it's only made of hex chars!
+                    "\\|" string)))
       (let ((case-fold-search nil))
         (funcall (if isearch-forward
                      #'re-search-forward
                    #'re-search-backward)
-                 newstr bound noerror)))))
+                 re bound noerror)))))
 
 (defun nhexl--isearch-search-fun (orig-fun)
   (let ((def-fun (funcall orig-fun)))
@@ -830,9 +839,18 @@ Return the corresponding nibble, if applicable."
       (unless bound
         (setq bound (if isearch-forward (point-max) (point-min))))
       (let ((startpos (point))
-            (def (funcall def-fun string bound noerror)))
-        ;; Don't search further than what `def-fun' found.
-        (if def (setq bound (match-beginning 0)))
+            def)
+        ;; Hex address search.
+        (when (and nhexl-isearch-hex-addresses
+                   (> (length string) 1)
+                   (string-match-p "\\`[[:xdigit:]]+:?\\'" string))
+          ;; Could be a hexadecimal address.
+          (goto-char startpos)
+          (let ((newdef (nhexl--isearch-match-hex-address string bound noerror)))
+            (when newdef
+              (setq def newdef)
+              (setq bound (match-beginning 0)))))
+        ;; Hex bytes search
         (when (and nhexl-isearch-hex-bytes
                    (> (length string) 1)
                    (string-match-p "\\`[[:xdigit:]]+\\'" string))
@@ -842,12 +860,10 @@ Return the corresponding nibble, if applicable."
             (when newdef
               (setq def newdef)
               (setq bound (match-beginning 0)))))
-        (when (and nhexl-isearch-hex-addresses
-                   (> (length string) 1)
-                   (string-match-p "\\`[[:xdigit:]]+:?\\'" string))
-          ;; Could be a hexadecimal address.
+        ;; Normal search.
+        (progn
           (goto-char startpos)
-          (let ((newdef (nhexl--isearch-match-hex-address string bound noerror)))
+          (let ((newdef (funcall def-fun string bound noerror)))
             (when newdef
               (setq def newdef)
               (setq bound (match-beginning 0)))))
@@ -909,17 +925,19 @@ Return the corresponding nibble, if applicable."
             #'nhexl--isearch-highlight-cleanup)
 (defun nhexl--isearch-highlight-cleanup (&rest _)
   (when (and nhexl-mode nhexl-isearch-hex-highlight)
-    (dolist (ol isearch-lazy-highlight-overlays)
-      (when (and (overlayp ol) (eq (overlay-buffer ol) (current-buffer)))
-        (put-text-property (overlay-start ol) (overlay-end ol)
-                           'fontified nil)))))
+    (with-silent-modifications
+      (dolist (ol isearch-lazy-highlight-overlays)
+        (when (and (overlayp ol) (eq (overlay-buffer ol) (current-buffer)))
+          (put-text-property (overlay-start ol) (overlay-end ol)
+                             'fontified nil))))))
 
 (advice-add 'isearch-lazy-highlight-match :after
             #'nhexl--isearch-highlight-match)
 (defun nhexl--isearch-highlight-match (&optional mb me)
   (when (and nhexl-mode nhexl-isearch-hex-highlight
              (integerp mb) (integerp me))
-    (put-text-property mb me 'fontified nil)))
+    (with-silent-modifications
+      (put-text-property mb me 'fontified nil))))
 
 (defun nhexl--line-width-watcher (_sym _newval op where)
   (when (eq op 'set)





  reply	other threads:[~2018-12-14 18:40 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-11 17:06 bug#33708: 26.1.90; nhexl-mode performance Guido Kraemer
2018-12-14 18:40 ` Stefan Monnier [this message]
     [not found]   ` <e64cb69b-c63c-0f8e-dcc5-b8d56006b67d@bgc-jena.mpg.de>
2018-12-14 22:52     ` Stefan Monnier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=jwv4lbg6jlr.fsf-monnier+emacsbugs@gnu.org \
    --to=monnier@iro.umontreal.ca \
    --cc=33708@debbugs.gnu.org \
    --cc=gkraemer@bgc-jena.mpg.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.