Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit the size of the displayed history to speed up Vundo #68

Open
agreselin opened this issue Apr 5, 2023 · 42 comments
Open

Limit the size of the displayed history to speed up Vundo #68

agreselin opened this issue Apr 5, 2023 · 42 comments

Comments

@agreselin
Copy link

agreselin commented Apr 5, 2023

When the history gets very long Vundo takes quite a bit of time to start up and then it is rather slow. I guess limiting the length of the history would solve this, but how can I limit the size of just the displayed history?

PS: I use

(setq undo-limit         (* 96 1024 1024)) ;  96 MiB. The change group at which this size is exceeded is the last one kept.
(setq undo-strong-limit (* 128 1024 1024)) ; 128 MiB. The change group at which this size is exceeded is discarded itself (along with all older change groups). There is one exception: the very latest change group is only discarded if it exceeds ‘undo-outer-limit’.
(setq undo-outer-limit (* 1024 1024 1024)) ;   1 GiB. If at garbage collection time the undo info for the current command exceeds this limit, Emacs discards the info and displays a warning. This is a last ditch limit to prevent memory overflow.

can it slow down Vundo?

@casouri
Copy link
Owner

casouri commented Apr 10, 2023

Should be fine, someone did some experiments and it seems vundo works fine with larger undo history: #67.

@agreselin
Copy link
Author

agreselin commented Apr 12, 2023

I've read that. In my case I have a 7.6 MB file of notes which I edit very often and the Vundo buffer takes about 25 s to open after doing M-x vundo. Then navigation between nodes is not that slow, but there's still a delay of about a second. (It's not the only file that slows down Vundo, but it's the worst.) My hardware specs are neither fancy nor archaeological: my PC is a 2015 Thinkpad T450s with an i5-5200U, and I'm on Emacs 28.2 and Fedora 37.

Could it be Emacs struggling with very long lines that causes the slowdown?

@casouri
Copy link
Owner

casouri commented Apr 12, 2023

Ah, ok. If the slowdown is at the start when vundo generates the tree, then I think vundo is to blame. The size of the file doesn't make vundo faster or slower, I wonder what's the size of the undo history? I assume you use persistent undo history? Could you check out the file used to store undo history and report its size?

Also, could you profile the undo tree generation for me: type M-x profiler-start RET cpu RET, then open vundo, then type M-x profiler-report, then in the profiler report buffer, type C-u RET to expand everything and paste the content here.

@agreselin
Copy link
Author

agreselin commented Apr 14, 2023

I assume you use persistent undo history?

Yes, sorry, I forgot to mention it.

Could you check out the file used to store undo history and report its size?

It's 110 kB zst compressed.

Also, could you profile the undo tree generation for me

Old profiler report
      31630  99% - command-execute
      31630  99%  - funcall-interactively
      31630  99%   - smex
      31548  98%    - smex-read-and-run
      22975  72%     - execute-extended-command
      22968  71%      - command-execute
      22967  71%       - funcall-interactively
      22964  71%        - vundo
      22352  70%         - vundo-1
      22346  70%          - vundo--refresh-buffer
         66   0%           - vundo--draw-tree
         54   0%            - vundo--translate
         50   0%             - seq-mapcat
         32   0%              - seq-map
         22   0%               - apply
         17   0%                - #<compiled 0x1856dfe4fae4f734>
         13   0%                 - mapcar
          4   0%                    #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_11>
         10   0%              - apply
          8   0%               - seq-concatenate
          6   0%                  apply
          5   0%           - vundo--build-tree
          3   0%            - vundo--sort-mod
          2   0%             - seq-sort
          2   0%              - apply
          2   0%               - #<compiled -0x59c02c6ae3ce13b>
          2   0%                  seq-copy
          6   0%          - vundo-mode
          1   0%             face-remap-add-relative
          4   0%         - fit-window-to-buffer
          4   0%          - jit-lock-function
          4   0%             jit-lock-fontify-now
          1   0%         - display-buffer-in-side-window
          1   0%          - window--make-major-side-window
          1   0%           - split-window-no-error
          1   0%              split-window
          1   0%       - byte-code
          1   0%        - custom-declare-face
          1   0%         - face-spec-set
          1   0%          - make-empty-face
          1   0%           - make-face
          1   0%            - make-face-x-resource-internal
          1   0%             - set-face-attributes-from-resources
          1   0%                set-face-attribute-from-resource
       8568  26%     - smex-completing-read
       8568  26%      - ido-completing-read
       8568  26%       - ido-read-internal
       8522  26%        - read-from-minibuffer
         64   0%         - ido-exhibit
         50   0%          - ido-set-matches
         50   0%           - ido-set-matches-1
         49   0%            - mapc
         30   0%               #<compiled -0x106af6854f37e99a>
          2   0%               #<compiled -0x857fe2f82a6be98>
          8   0%          - ido-set-common-completion
          4   0%             ido-find-common-substring
          3   0%          - ido-completions
          2   0%             #<compiled -0x127568e430ca67b9>
         35   0%         - redisplay_internal (C function)
         34   0%          - my-recenter-after-jump
         34   0%           - let
         34   0%            - save-current-buffer
         34   0%             - unwind-protect
         34   0%              - progn
         34   0%               - if
         34   0%                - let*
         34   0%                   line-number-at-pos
          1   0%          - window--resize-root-window-vertically
          1   0%             window--resize-this-window
          2   0%           timer-event-handler
          2   0%         - command-execute
          2   0%          - funcall-interactively
          1   0%           - self-insert-command
          1   0%            - electric-pair-post-self-insert-function
          1   0%             - electric-pair-syntax-info
          1   0%                syntax-ppss
          1   0%           - handle-focus-out
          1   0%            - #<compiled 0x14f1e59aef9002d>
          1   0%             - apply
          1   0%              - #<compiled -0xb47f0007cd981>
          1   0%               - apply
          1   0%                - run-with-timer-handle-change-of-focus
          1   0%                 - if
          1   0%                  - setq
          1   0%                   - run-with-timer
          1   0%                      run-at-time
          1   0%         - window--resize-root-window-vertically
          1   0%          - window--resize-this-window
          1   0%           - window--resize-child-windows
          1   0%              window-size-fixed-p
         11   0%        - ido-set-matches
         11   0%         - ido-set-matches-1
          9   0%          - mapc
          4   0%             #<compiled -0x106af6854f37e99a>
         61   0%    - smex-update
         54   0%     - smex-rebuild-cache
         15   0%        #<compiled 0xeb9a8ab1991c20>
          1   0%        smex-convert-for-ido
         18   0%    - smex-detect-new-commands
         13   0%       #<compiled -0x252f922c28e04b5>
        275   0% + ...
          1   0% + timer-event-handler

@agreselin
Copy link
Author

Here is a profiler report from an instance with a cleaner config.

       22121  98% - command-execute
       20666  92%  - funcall-interactively
       20660  92%   - execute-extended-command
       20646  92%    - command-execute
       20626  92%     - funcall-interactively
       20617  92%      - vundo
       20031  89%       - vundo-1
       20013  89%        - vundo--refresh-buffer
          49   0%         - vundo--draw-tree
          34   0%          - vundo--translate
          31   0%           - seq-mapcat
          23   0%            - seq-map
          18   0%             - apply
          11   0%              - #<compiled 0x1856cdad2b64f734>
           5   0%               - mapcar
           2   0%                  #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_11>
           6   0%            - apply
           2   0%             - seq-concatenate
           2   0%                apply
           5   0%         - vundo--build-tree
           4   0%          - vundo--sort-mod
           3   0%           - seq-sort
           1   0%            - apply
           1   0%               #<compiled -0x59ce19e634939fb>
          18   0%        - vundo-mode
           1   0%           face-remap-add-relative
           1   0%         - run-mode-hooks
           1   0%          - run-hooks
           1   0%           - global-font-lock-mode-enable-in-buffers
           1   0%            - turn-on-font-lock-if-desired
           1   0%             - turn-on-font-lock
           1   0%                font-lock-mode
           3   0%       - fit-window-to-buffer
           3   0%        - jit-lock-function
           3   0%           jit-lock-fontify-now
          20   0%       byte-code
        1455   6%  - byte-code
        1455   6%   - read-extended-command
        1363   6%    - completing-read-default
          18   0%     - command-execute
          18   0%      - funcall-interactively
          17   0%       - minibuffer-complete
          17   0%        - completion-in-region
          17   0%         - completion--in-region
          17   0%          - #<compiled -0x731a712fc48e962>
          17   0%           - apply
          17   0%            - #<compiled -0x17bc5b2160671e30>
          17   0%             - completion--in-region-1
          17   0%              - completion--do-completion
          17   0%               - completion-try-completion
          17   0%                - completion--nth-completion
          17   0%                 - completion--some
          17   0%                  - #<compiled 0xaf0040bd50f5b1c>
          17   0%                   - completion-basic-try-completion
          17   0%                    - #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_49>
          17   0%                       complete-with-action
           1   0%       - self-insert-command
           1   0%          undo-auto-amalgamate
           1   0%     - timer-event-handler
           1   0%        timer-inc-time
         255   1% + ...
       22121  98% - command-execute
       20666  92%  - funcall-interactively
       20660  92%   - execute-extended-command
       20646  92%    - command-execute
       20626  92%     - funcall-interactively
       20617  92%      - vundo
       20031  89%       - vundo-1
       20013  89%        - vundo--refresh-buffer
          49   0%         - vundo--draw-tree
          34   0%          - vundo--translate
          31   0%           - seq-mapcat
          23   0%            - seq-map
          18   0%             - apply
          11   0%              - #<compiled 0x1856cdad2b64f734>
           5   0%               - mapcar
           2   0%                  #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_11>
           6   0%            - apply
           2   0%             - seq-concatenate
           2   0%                apply
           5   0%         - vundo--build-tree
           4   0%          - vundo--sort-mod
           3   0%           - seq-sort
           1   0%            - apply
           1   0%               #<compiled -0x59ce19e634939fb>
          18   0%        - vundo-mode
           1   0%           face-remap-add-relative
           1   0%         - run-mode-hooks
           1   0%          - run-hooks
           1   0%           - global-font-lock-mode-enable-in-buffers
           1   0%            - turn-on-font-lock-if-desired
           1   0%             - turn-on-font-lock
           1   0%                font-lock-mode
           3   0%       - fit-window-to-buffer
           3   0%        - jit-lock-function
           3   0%           jit-lock-fontify-now
          20   0%       byte-code
        1455   6%  - byte-code
        1455   6%   - read-extended-command
        1363   6%    - completing-read-default
          18   0%     - command-execute
          18   0%      - funcall-interactively
          17   0%       - minibuffer-complete
          17   0%        - completion-in-region
          17   0%         - completion--in-region
          17   0%          - #<compiled -0x731a712fc48e962>
          17   0%           - apply
          17   0%            - #<compiled -0x17bc5b2160671e30>
          17   0%             - completion--in-region-1
          17   0%              - completion--do-completion
          17   0%               - completion-try-completion
          17   0%                - completion--nth-completion
          17   0%                 - completion--some
          17   0%                  - #<compiled 0xaf0040bd50f5b1c>
          17   0%                   - completion-basic-try-completion
          17   0%                    - #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_49>
          17   0%                       complete-with-action
           1   0%       - self-insert-command
           1   0%          undo-auto-amalgamate
           1   0%     - timer-event-handler
           1   0%        timer-inc-time
         255   1% + ...

Config:

;; -*- lexical-binding: t; -*-

;;;; Undo/redo

;; Increase undo lists’ size limits
;; https://codeberg.org/ideasman42/emacs-undo-fu#undo-limits
;; https://www.gnu.org/software/emacs/manual/html_node/elisp/Maintaining-Undo.html
(setq undo-limit         (* 96 1024 1024)) ;  96 MiB. The change group at which this size is exceeded is the last one kept.
(setq undo-strong-limit (* 128 1024 1024)) ; 128 MiB. The change group at which this size is exceeded is discarded itself (along with all older change groups). There is one exception: the very latest change group is only discarded if it exceeds ‘undo-outer-limit’.
(setq undo-outer-limit (* 1024 1024 1024)) ;   1 GiB. If at garbage collection time the undo info for the current command exceeds this limit, Emacs discards the info and displays a warning. This is a last ditch limit to prevent memory overflow.

(setq undo-no-redo t)
(global-set-key (kbd "M-_") #'undo-redo)

;;; Persistent undo history

;; https://codeberg.org/ideasman42/emacs-undo-fu-session

(setq undo-fu-session-compression 'zst)
(setq undo-fu-session-file-limit 350)

(undo-fu-session-global-mode)

;;; Visual undo tree navigation

;; https://github.com/casouri/vundo

(with-eval-after-load 'vundo
  (setq vundo-glyph-alist vundo-unicode-symbols)
  (set-face-attribute 'vundo-node nil :foreground "grey60")
  (set-face-attribute 'vundo-stem nil :foreground "grey60")
  (set-face-attribute 'vundo-highlight nil :inherit 'unspecified :foreground 'unspecified) ; Don’t change the face (change only the character).
  (set-face-attribute 'vundo-saved nil :foreground "black") ; REVIEW (bug?): Saving a node that wasn’t saved and then saving another strips the first one of the ‘vundo-saved’ face (it’s displayed as if it was never saved).
  (set-face-attribute 'vundo-last-saved nil :underline t))
(setq vundo-compact-display t)

@casouri
Copy link
Owner

casouri commented Apr 16, 2023

Thanks very much. The problem is, I couldn't tell what's taking the majority of time... vundo--refresh-buffer is a pretty high-up function and it contains a lot of things that might take time, and the profiler doesn't say which function it called inside is taking a lot of time. (which is weird, because when I run the profiler on my machine, it shows functions below vundo--refresh-buffer that are taking time.) But if I have to guess, it probably is the generation of vundo's data structure.

Before we go further, may I ask you why you want to keep a large history, most of which you can't really meaningfully access? Since you asked for vundo to limit the displayed portion of the history, I assume you don't plan to utilize the wouldn't-be-shown part of the history?

@agreselin
Copy link
Author

agreselin commented Apr 16, 2023

may I ask you why you want to keep a large history, most of which you can't really meaningfully access? Since you asked for vundo to limit the displayed portion of the history, I assume you don't plan to utilize the wouldn't-be-shown part of the history?

I don't want to keep a large history, I want not to lose it if I open a file, make a quick edit and close its buffer. I set the undo-*limit variables to experiment, I don't really need them. But I tried to remove the settings and run Emacs, see if it cut the history short, and it didn't. Also, limits by size are clumsy. I might delete 10 MB of text from a backup file with one edit, and I wouldn't want that to exhaust my history limit, while 10 MB worth of edits to a file of notes means I keep years of edits.

At first, seeing that Vundo was slow, I thought to ask you about introducing the history size limit, now I wonder whether setting it in Undo Fu Session would be better.

when I run the profiler on my machine, it shows functions below vundo--refresh-buffer that are taking time

I'd like to know why. I took a look at the customisations available for profiler and the only variable that seemed to me like it could change something was profiler-sampling-interval. Tried decreasing it by a factor 100 but the report came out the same.

@ideasman42
Copy link
Contributor

ideasman42 commented Apr 16, 2023

@agreselin (not exactly related to the issue), but you might want to enable (setq undo-fu-session-linear t) which makes the undo history linear when saving.

While it's nice to be able to save/restore undo-history exactly in the previous state, in practice I can't recall the last time I needed to traverse unused branches of previously saved sessions, so I enable linear undo history. This tends to make the undo history smaller with less overhead for vundo too.

@agreselin
Copy link
Author

Hi @ideasman42 ! Thanks for the tip. With (setq undo-fu-session-linear t) Vundo took less than a couple of second to start up for the same file, so I'd say it fixes the issue.

I agree that the full tree of previous sessions is rarely useful, but maybe the full tree of recent history is more useful than the linear history from long ago. Most likely, I'm overthinking my potential needs. Still, since we're discussing this I'm sharing the idea. For sure it's not a deal-breaker.

@ideasman42
Copy link
Contributor

@agreselin in practice the history created since last saving is often the full tree of recent history, but I suppose there could be exceptions to this where you want recent history to include branches from a previous session.

If I understand what your getting at, it would be possible to prune early branches off the tree by some other method (keep N-number of branches so as not to store an unreasonable amount of history for e.g.). This might be nice to support although I don't find myself wanting such a feature.

@agreselin
Copy link
Author

If I understand what your getting at, it would be possible to prune early branches off the tree by some other method (keep N-number of branches so as not to store an unreasonable amount of history for e.g.)

@ideasman42 I was thinking about deleting all nodes older than a certain amount of time and their branches. Would it be possible to do so? Sometimes I add something, say a reference, to my notes and then undo the insertion and save it somewhere else. It's happened that afterwards I couldn't find where I wrote down the reference and I retrieved it from the undone branch. After a week or so it's not of much use, but before that it may be.

@ideasman42
Copy link
Contributor

@agreselin you could remove them by date, although there would need to be time information in those branches (the user might have saved while working, but not necessarily). So I don't think it's something that's supported reliably.

@agreselin
Copy link
Author

agreselin commented Apr 21, 2023

I don't think it's something that's supported reliably.

@ideasman42 Well, then the option of keeping the latest N branches is still interesting. But it could cut the history unexpectedly short if it has a lot of branches. What about just limiting the depth of the history? I mean, limit the number of nodes in the trunk and keep all branches. Then the worst that could happen is that Vundo slows down a bit.

@jdtsmith
Copy link
Contributor

I wonder if your persistent history includes the save timestamps we are now using to highlight saved buffer states (the green circles)? If so, it would be possible to ascend the tree to a green circle at least some time in the past (1 month, say) and trim the tree up to that position, keeping the branches (it would need to be a saved state on the "main branch").

@ideasman42
Copy link
Contributor

Yes, it includes time-stamps, it's just possible some branches don't include them. Although it's possible to use those time-stamps to prune all branches before a certain date - in the event they do exist.

@agreselin
Copy link
Author

Hi, I've been using (setq undo-fu-session-linear t) for a while now and enough history has piled up (in the usual file of notes) that Vundo again takes ~20 s to launch. I think it really needs a cut-off threshold and I think it would be better to set it in Vundo rather than Undo Fu Session, so that the history is not actually deleted but just ignored (and still available by changing the cut-off).

@jdtsmith
Copy link
Contributor

jdtsmith commented Dec 10, 2023

I can confirm that vundo's slowness with long chains is entirely related to vundo--draw. Here's a test (first compile test/vundo-stress-test.el):

(progn 
  (elp-instrument-function #'vundo--draw-tree)
  (random "LONG-CHAIN-SLOW-DRAW") 
  (vundo-stress-test 5000 25 8 nil)
  (elp-results))

For me 2.35/2.5s is taken on draw-tree (2 calls, but one is a fast phantom call just getting the buffer name from vundo-1 for the stress-test). Navigation is just OK, and I suspect the slowness is because Emacs hates long lines. In this case vundo-buffer's width is 9575 columns. While such a "wide" vundo buffer is shown, navigation in other buffers slows down, and improves immediately when it is closed. This is a hallmark of "long-line"-itis, and it gets worse very rapidly as a function of line width. @agreselin I bet if you (current-column) on vundo-start you'll see > 15,000.

A potential solution:

  • Rather than actually drawing the tree, record the tree's layout in some column-indexed data structure, like a ragged vector list. My strong suspicion based on timing the "ingredients" of the tree-drawing algorithm is this will be 100x faster than actually drawing.
  • The current node then dictates which portion along the width of that tree should actually be drawn in-buffer, which can be done on demand as needed (perhaps with a bit of flourish regarding falling off the edge, etc.).
  • A frame resize hook will be needed so this updates as the window width changes.

An uglier but maybe easier solution:

  • Add a set of hidden backing buffers (as many as needed), and draw pieces of the tree in them, magically switching among them as needed.

@agreselin
Copy link
Author

I bet if you (current-column) on vundo-start you'll see > 15,000.

19075 😅

@jdtsmith
Copy link
Contributor

jdtsmith commented Dec 10, 2023

Here's how vundo launch performance behaves for a purely linear chain of undos as a function of final column number in the vundo buffer (approaching quadratic above a threshold):

image

Pretty fast laptop. I suppose it's possible the draw-tree algorithm is quadratic in nodes (which directly relates to columns for a linear case), but it looks to be a fairly local algorithm, just checking parent, children, etc. before inserting characters. If branching were the problem, then performance on linear chains should be good. Pretty good evidence it's just "long lines" .

@jdtsmith
Copy link
Contributor

Update: by modifying vundo--draw-tree to limit the maximum line length, I find the draw time decreases substantially. Important: this is not a solution, just a simple test. In fact, it's this simple:

(when (> col 200)
    (goto-char (point-max))
    (insert "\n\n")
    (setq col 0))

Here's how the performance looks, switching from max column to number of edits (directly correlated for the current algorithm). This clearly demonstrates the conjecture that long lines are the culprit for poor vundo performance with deep undo trees. In all cases, at a given number of edits, the vundo buffer contains roughly the same number of drawn characters, they are just organized into fewer or more lines of differing maximum length. Again, emacs really hates drawing super long lines.

Going below 80 columns improves times only modestly, so we are approaching the speed of the underlying draw algorithm, with its regex searches, lots of moves, etc. The smooth quadratic scaling with very long lines is an emacs display problem, not a vundo problem.

image

@minad
Copy link

minad commented Dec 16, 2023

@jdtsmith Why not sidestep the long line problem? Rotate the tree by 90° and show it in a window on the left or right side?

@casouri
Copy link
Owner

casouri commented Dec 16, 2023

If someone implements one that works, I'lm more than happy to make it an option :-)

@jdtsmith
Copy link
Contributor

jdtsmith commented Dec 16, 2023

What? And make it look like undo-tree? But seriously, I prefer horizontal because trees are usually pretty long and not very bushy, and this takes up quite a bit less room. I suppose a narrow vertical side buffer could be an option, but horizontal space is often at a premium with side-by-side windows.

BTW, I simply cannot find anything in vundo--draw-tree that justifies its slowness for long lines (6000 edits here):

Function Name               Call Count  Elapsed Time  Average Time
vundo--draw-tree            1           3.849056      3.849056
append                      48118       0.0478320000  9.940...e-07
propertize                  12063       0.0423950000  3.514...e-06
insert                      22018       0.0183619999  8.339...e-07
vundo--translate            12002       0.0085510000  7.124...e-07
looking-at                  12202       0.0038980000  3.194...e-07
vundo--node-timestamp       6002        0.0033740000  5.621...e-07
goto-char                   13517       0.0026380000  1.951...e-07
last                        12002       0.0021369999  1.780...e-07
vundo-m-children            12001       0.0021319999  1.776...e-07
pop                         6001        0.0017129999  2.854...e-07
vundo-m-point               6000        0.0015979999  2.663...e-07
vundo--put-node-at-point    6001        0.0015049999  2.507...e-07
vundo-m-parent              6001        0.0007300000  1.216...e-07
erase-buffer                48          0.0005819999  1.212...e-05
delete-char                 29          1.100...e-05  3.793...e-07

I've engaged on emacs-devel, and doubt has been cast on long lines as the culprit. We'll see if we can get to the bottom of it. If people want to try it, here the test code:

;; create and benchmark starting vundo with long linear undo trees
(defun vundo-linear-stress-test (nsen)
  (interactive
   (list (read-number "Number of edits (sentences): " 20000)))
  (let* ((buf (get-buffer-create "*vundo-linear-test*"))
         (res (cl-loop for nedits = 3 then (round (* nedits 1.25))
                       while (< nedits nsen) do
                       (with-current-buffer buf
                         (read-only-mode -1)
                         (font-lock-mode -1)
                         (erase-buffer)
                         (setq buffer-undo-list nil
                               pending-undo-list nil)
                         (dotimes (_ nedits)
                           (lorem-ipsum-insert-sentences 1)
                           (insert "\n")
                           (undo-boundary)))
                       
                       collect (flatten-tree
                                (list nedits (with-current-buffer buf
                                               (benchmark-call #'vundo))
                                      (with-current-buffer " *vundo tree*"
                                        (current-column))))
                       do (with-current-buffer (get-buffer " *vundo tree*")
                            (vundo-quit)))))
    (with-output-to-temp-buffer "*vundo-linear-stress-test-results*"
      (princ (format "%6s %10s %4s %10s %6s\n"
                     "Edits" "Time(s)" "GCs" "GCTime(s)" "Cols"))
      (dolist (x res) (princ (apply #'format "%6d %10.3f %4d %10.3f %6d\n" x))))))

@casouri
Copy link
Owner

casouri commented Dec 16, 2023

I just read the thread on emacs-devel, and Eli has a point: if it's because of redisplay, it should show up in the profile result as , but here we're seeing the time spent in vundo--draw-tree. So I think it's some operation in that function that gets slower when lines are long?

@casouri
Copy link
Owner

casouri commented Dec 16, 2023

Maybe vundo--next-line-at-column is the culprit? What happens if we comment it out?

@jdtsmith
Copy link
Contributor

I tried that and it didn't change anything. In fact that never runs with a purely linear tree, because there is always room-for-another-rx. Did I miss a word above though? "In the profile result as" ???. I can easily add anything to my elp report if you have ideas.

@casouri
Copy link
Owner

casouri commented Dec 16, 2023

That's my best guess 🥲 I felt that counting columns might be slow when line is long. I don't really see anything else that could slow down drastically with longer lines

@jdtsmith
Copy link
Contributor

The culprit was in fact just (current-column). It's much slower for us (>~1ms) than in vanilla buffers possibly due to all the text properties it has to check. But the doc for that does mention how expensive it is in long lines. Luckily it's really rarely needed. A trivial fix in #85. Updated timing:

image

@jdtsmith
Copy link
Contributor

(apologies, that PR got crossed with a small DOC PR). If you want to bump version so it hits ELPA prior to the l/r stuff (#81), we should in addition fix #84 (I have a fix waiting to see if it worked for them).

@jdtsmith
Copy link
Contributor

@agreselin can you try out master and see what the launch time in your deep-history file is?

@agreselin
Copy link
Author

~3 seconds. Moving to another node takes half a second or so. Another file in which Vundo was somewhat slow now hasn't any noticeable lag 👍

Btw I don't think rotating the tree is a bad idea. Someone willing to do that (I can't, sorry 🙏) could replicate Vim's undotree algorithm, which is pretty smart about keeping its tree narrow, see this picture.

@jdtsmith
Copy link
Contributor

Moving shouldn't be so slow since the tree isn't redrawn, not sure why that is (though it could still be long-line display issues, Eli thinks that sets in a much longer lines, like 50K columns). This is with 19K columns, right? How long did it take to launch before? I'm not sure rotating will speed things much beyond the current algorithm. If you can M-x elp-instrument-function current-column, invoke vundo, then M-x elp-results and post that'd be helpful.

@agreselin
Copy link
Author

This is with 19K columns, right?

Close to 26,000 now.

How long did it take to launch before?

I didn't check before updating. Two weeks or so ago it was still around 20 s, if I remember correctly.

If you can M-x elp-instrument-function current-column, invoke vundo, then M-x elp-results and post that'd be helpful.

The results buffer is empty 🤔. If I invoke current-column and then elp-results again I get

current-column  4           0.0266033480  0.0066508370

Moving shouldn't be so slow since the tree isn't redrawn, not sure why that is

I noticed while doing the test that Emacs in general is slower: even M-x (which for me brings up the Ido/Smex completion buffer) lags for that fraction of a second, and clicking another buffer lags too before switching to it.

@jdtsmith
Copy link
Contributor

Yes, I also notice everything (minibuffer, other buffers) is a tad slow when such a long-line buffer is showing. What build/OS are you on? How big is your undo-limit to let so many accumulate? At some point you need to trim that thing :).

@agreselin
Copy link
Author

I'm on my own build of Emacs 30.0.50 (with native compilation and PGTK) on Fedora 39.

How big is your undo-limit

It's 100 MB.. How much is yours?

@jdtsmith
Copy link
Contributor

That's truly huge. I use:

;; Give undo a bit more rope: up to 2MB per buffer
(setq undo-limit 1000000
      undo-strong-limit (* 2 undo-limit))

@agreselin
Copy link
Author

Sorry for the OT, I'm going to ask you a thing about undo-limit.

I had dialed it down to 50 MB before you replied but I wasn't sure about going lower. My understanding is that if you copy-paste more than 2 MB of data, that edit fills your undo list and that and all older edits are discarded. Sometimes I edit big-ish dumps in Emacs and I'm afraid that if I copy-paste a large chunk to another buffer by mistake, it wipes the undo history of this other buffer. (My understanding comes mostly from (info "(elisp) Maintaining Undo").) Am I wrong?

@jdtsmith
Copy link
Contributor

2MB is about 30,000 lines of normal line length text. How often do you accidentally remove 30,000 lines of material? Note that additions are referenced in buffer-undo-list by a trivial (BEG . END) so that data is stored in the buffer and adds almost nothing. If you worry about large atomic screw-ups, you can leave undo-limit somewhat modest (1 or 2 MB), but increase undo-strong-limit to something quite a big larger, like your estimate of the largest "screw up" you might make (10MB?). That way the data for any such screw up deletion will not be discarded before you can correct it (though everything earlier will). undo-outer-limit can be left at your giant 50MB or something as a last ditch protection.

Unless you find yourself unwinding many thousands of nodes into history for some reason, this seems a far more sensible setup. And if you are that concerned about your data for some file, you might put it under revision control.

@agreselin
Copy link
Author

Are there any downsides to keeping large limits? I never happened to hit even the default ones but with GBs of RAM I thought I could afford a bit of paranoia.

Anyway, would it help Vundo much? I guess that before I pile up even 2 MB in changes the list of edits grows quite long.

@jdtsmith
Copy link
Contributor

Yes it would help vundo and emacs not to have 25K long lines. Even with the latest fix, the performance is slowing relatively rapidly with increasing line length at that scale. Think of it not as a "RAM saving" limit, but a "keeping undo reasonable" limit.

@agreselin
Copy link
Author

But if Vundo uses a horizontal layout and chokes Emacs with long lines that's a problem with Vundo, not with my configuration, in my opinion. Emacs doesn't choke on long undo histories. My file of notes is 8 MB in size, if I set the limits lower than that I'm only a wrong key stroke away after C-x h from losing the undo history.

What makes undo reasonable is subjective, apparently, because other competent Elisp programmers use large undo limits, see Campbell Barton's suggestions here.

@jdtsmith
Copy link
Contributor

jdtsmith commented Dec 20, 2023

Options are to implement a vertical layout, or come up with a drawing algorithm that limits the amount of displayed information. Sounds like @casouri is open to either. But a factor of >~10 improvement is nothing to dismiss. It also appears you haven't understood undo-strong-limit, maybe read about that in the Elisp manual.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants