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

post command hook slows down cursor movement when buffer has many (40~50k) overlays #926

Closed
sebastiencs opened this issue Apr 10, 2018 · 10 comments · Fixed by #927
Closed

Comments

@sebastiencs
Copy link

sebastiencs commented Apr 10, 2018

Hi,

I noticed that my cursor is very slow to move, it seems that yasnippet is allocating a lot of memory.
Moving the cursor from left to right on the same line produced this report:
yasnippet

This is in a buffer with lsp-mode (cquery). There are a lots of overlays (don't know if it is relevant):

(length (overlay-lists)) ;; 49390

yas--version: "0.12.2"
GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.26) of 2018-04-05

@npostavs
Copy link
Collaborator

There are a lots of overlays (don't know if it is relevant):

Yes, it probably is. I think the patch in #927 should help, please test it out.

@sebastiencs
Copy link
Author

Thanks for your quick answer.
I applied your patch but the cursor is still slow to move:

yasnippet2

Is looping on all overlays of the whole buffer necessary on a post-command-hook ?

@npostavs
Copy link
Collaborator

Is looping on all overlays of the whole buffer necessary on a post-command-hook ?

I think it should be avoidable for the (yas-active-snippets 'all) case.

By the way, it looks like yas-active-snippets is running uncompiled in that profile report. I guess you applied the patch and then just re-evaluated that function in the source buffer. Could you try again after doing (byte-compile 'yas-active-snippets)?

@sebastiencs
Copy link
Author

sebastiencs commented Apr 10, 2018

So I tried to make more consistent tests, and with yas-active-snippets byte compiled.
I used this function to profile yas--post-command-handler

(defun my-yas-report nil
  (profiler-start 'cpu+mem)
  (dotimes (_ 100)
    (yas--post-command-handler))
  (profiler-report))

Without your patch:
yas-unpatched

With your patch:
yas-patched

I also run:

(benchmark-run 100 (yas--post-command-handler)) ;; unpatched
;; (2.696426935 20 1.9017052230000004)

(benchmark-run 100 (yas--post-command-handler)) ;; patched
;; (1.635645147 10 0.9392250539999907)

So thanks to your patch there is twice less garbage collection, but in the buffer my cursor is still slow to move unfortunately..

@npostavs
Copy link
Collaborator

Are your benchmarks run without any active snippet? I wouldn't have thought there would be that much to do in that case.

I've added a commit to #927 which avoids using overlay-get in the post command hook.

@sebastiencs
Copy link
Author

I'm not sure what means exactly active snippets, but (yas-active-snippets 'all) returns nil.
Your last commit totally fix the issue, thank you for working on this !

@npostavs
Copy link
Collaborator

I'm not sure what means exactly active snippets,

It means that you have expanded a snippet, and point is one of its fields.

Your last commit totally fix the issue, thank you for working on this !

Cool, so it was really just the looping over the overlays then. I thought there might be something else, since your profile report still listed 178M of allocation, and I didn't think calling overlay-get with result nil would actually allocate anything. Maybe it's all let-binding; I should switch to lexical-binding at some point.

@npostavs npostavs changed the title Performance issue post command hook slows down cursor movement when buffer has many (40~50k) overlays Apr 11, 2018
@sebastiencs
Copy link
Author

No I didnt' have any active snippet during the benchmarks.
Yes that's surprising, could it be the overlays-in ?

@npostavs
Copy link
Collaborator

Okay, I'm a little bit uncertain about how stable that patch will be, so I'm going to merge it only after releasing version 0.13, which I expect to do in a couple of weeks.

@sebastiencs
Copy link
Author

Alright, no problem

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

Successfully merging a pull request may close this issue.

2 participants