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

Suspected poor performance from rustic-syntax-propertize #107

Closed
brownjohnf opened this issue May 11, 2020 · 31 comments · Fixed by #108
Closed

Suspected poor performance from rustic-syntax-propertize #107

brownjohnf opened this issue May 11, 2020 · 31 comments · Fixed by #108

Comments

@brownjohnf
Copy link

brownjohnf commented May 11, 2020

Hello! I apologize if I'm confused or misguided here, but I believe that the rustic-syntax-propertize function is locking up emacs frequently when editing any rust source file of non-trivial size. It seems that with anything larger than about 100 lines emacs becomes pretty much unusable.

This suddenly started happening a few weeks ago, and at first I thought it was my workstation, but then had the same thing happen after switching to another workstation, but only after upgrading my doom emacs installation. Unfortunately, I don't recall whether I updated rust/rls at the same time. I'm not proficient with emacs lisp, so from the little of the source I've looked at, it's unclear how much this function relies on rls for any functionality.

I tracked down this function as the culprit by running the profiler in emacs; as you can see in the screenshot, it's consuming ~75% of CPU and 55% memory during my sample run; that's pretty typical over the 2 machines and several samples I've run. Specifically, rustic-ordinary-lt-gt-p seems to be the problem function.

Screenshot from 2020-05-11 14-45-12

As I said before, this very much outside my area of expertise, so I might be completely wrong, but I'm hitting dead-ends on trying to find any other issue in other projects that explain it, so I thought I'd bring it up here. I am running nightly rust, but I've been updating frequently since hitting this issue and the problem persists, so if that's the cause it's something more than a whoops commit.

Setting syntax-propertize-function to nil appears to completely resolve the problem, but then you don't get formatting while editing. That's been my workaround today since tracking this down.

@brotzeit
Copy link
Owner

Thanks for the bug report! I don't have the time to take a look at it at the moment. Any help is appreciated.

@brownjohnf
Copy link
Author

I'm not sure how much more I can provide, given my limited experience in these areas, but I'm happy help in whatever way I can.

@brotzeit
Copy link
Owner

I'm afraid we have to look at the code. Maybe the function has to be rewritten. I didn't write it so I don't know how it works.

@brotzeit
Copy link
Owner

Maybe we should try this

rust-lang/rust-mode@bfe4056

@phillord
Copy link
Contributor

Yep, fixed that finally yesterday. Why are there two code bases again?

@brotzeit
Copy link
Owner

Because we want a lightweight package and a package that comes with all features. I know this sucks, but making rustic depend on rust-mode is a lot of work...

@brownjohnf
Copy link
Author

I took a look at those changes; its seems like rustic.el and rust-mode.el have diverged quite a bit, and I'm not comfortable with elisp to make a useful patch, I don't think. I tried just applying a patch straight out of rust-mode, but it fails. I'm sorry I'm not more useful here.

@phillord
Copy link
Contributor

It should patch up, but it would have to done manually. The change in rust-mode is mostly in a single function (rustic-in-macro) which is largely the same form as the old rust-in-macro.

brownjohnf added a commit to brownjohnf/doom-emacs that referenced this issue May 17, 2020
The `rustic-syntax-propertize` function in rustic (set as
`syntax-propertize-function` in emacs) had a performance regression
(reported in brotzeit/rustic#107) that
caused emacs to effectively lock up every time the viewport changed.
This was fixed upstream in rust-mode by @phillord in
rust-lang/rust-mode@bfe4056,
and ported to rustic by @brotzeit in
brotzeit/rustic#108.

I've confirmed that this version of rustic seems to resolve the issue.
hlissner added a commit to doomemacs/doomemacs that referenced this issue May 18, 2020
brotzeit/rustic@32a962a -> brotzeit/rustic@52b632d

The `rustic-syntax-propertize` function in rustic (set as
`syntax-propertize-function` in emacs) had a performance regression
(reported in brotzeit/rustic#107) that caused emacs to effectively lock
up every time the viewport changed. This was fixed upstream in rust-mode
by @phillord in rust-lang/rust-mode@bfe4056, and ported to rustic by
@brotzeit in brotzeit/rustic#108.

Closes #3144

Co-authored-by: Jack Brown <jack@brownjohnf.com>
@brotzeit brotzeit reopened this May 18, 2020
@brotzeit
Copy link
Owner

brotzeit commented May 18, 2020

It still doesn't seem to be solved completely.

@brotzeit
Copy link
Owner

@brownjohnf can you do me a favor and try it with rust-mode ? Would be good to know if rust-mode also still has this issue.

@phillord
Copy link
Contributor

#109

@brotzeit
Copy link
Owner

@brownjohnf another round of testing please ;)

@brownjohnf
Copy link
Author

You got it! I can do that some time today.

@brownjohnf
Copy link
Author

Re testing with rust-mode, I could probably get something together, but as a doom emacs user, I'm not sure exactly how that would work, or how accurate a test result it would be in that context. Seems like with these changes in #109, I'll just test those and hopefully that'll sort things out.

@phillord
Copy link
Contributor

Probably I would port this commit as well.

rust-lang/rust-mode@eca55c0

For me, now, rustic-ordinary-lt-gt-p is consuming more CPU than rustic-in-macro. It's a really simple patch. It might well help. I am still finding rustic editing very laggy.

@brownjohnf
Copy link
Author

Unfortunately, it's still not looking great :-(. I'm attaching a screenshot from the latest test. I took a look at the rustic-syntax-propertize function using emacs' describe-function command, and it's the definition from current master.

@phillord In your comment above, are you saying that rustic-ordinary-lt-gt-p is consuming more CPI than rustic-in-macro after rust-lang/rust-mode@eca55c0 or without it? I assume without?

Screenshot from 2020-05-19 19-15-45

@phillord
Copy link
Contributor

@brownjohnf I am saying that the fix to rustic-ordinary-lt-gt-p is dead simple, and might as well be applied. But, yes, the issue with rustic-ordinary-lt-gt-p is only obvious because rustic-in-macro is no longer quadratic.

@brotzeit
Copy link
Owner

@brownjohnf I wasn't able to reproduce a laggy behaviour. Can you give me some context so I can try to reproduce it ?

@brownjohnf
Copy link
Author

Sorry about the delay here. I basically hit it any time that I'm editing larger files. Probably once I'm over ~500 lines, but it's hard to say for certain. Once the effect is noticeable, it basically freezes the editor any time the viewport changes. So things like jumping around in the file can take ~3 seconds or more. Once you're actually inputting text, sometimes it's laggy and sometimes not. Haven't really found a pattern here. Maybe depends on whether the input pushes text off the edge of the viewport?

I should also say that I'm using evil-mode, and that I'm using it in doom emacs. However, I had no issues whatsoever prior to the changes that caused the poor performance, so I don't think it's caused by doom/evil.

@brotzeit
Copy link
Owner

brotzeit commented Jun 5, 2020

It would be really nice if you(or anybody else with this issue) could find the time to test it with vanilla emacs and rustic only. And if it persists it would be good to also try it with rust-mode. Just load rustic with (use-package rustic) and nothing else in your config.

@jbms
Copy link

jbms commented Aug 2, 2020

I was also observing serious lag --- and the top line of the profile @brownjohnf listed here #107 (comment) provided the key clue.

I had adaptive-wrap-prefix-mode enabled --- turning it off seems to eliminate the lag.

@brotzeit
Copy link
Owner

brotzeit commented Aug 5, 2020

@jbms Thanks for the info. @brownjohnf can you confirm this ?

@phillord
Copy link
Contributor

Hmmm. rustic-in-comment-paragraph continually flushes the syntax cache and then calls syntax-ppss on every couple of lines. Seems like the place to start looking to me.

@brownjohnf
Copy link
Author

Sorry about my long silence here... life stuff and all.

I've actually recently switched from rls to rust-analyzer and fully enabled everything in rustic (I'd previously disabled syntax-propertize-function to avoid the slowness), and things seem to be normally performant now. I'll try to dig into this in more detail this week, but I'm not sure if I'll have time.

@brotzeit
Copy link
Owner

If there are still problems with this, please reopen this issue. @phillord @brownjohnf thanks for your help.

@darwin67
Copy link

darwin67 commented Apr 6, 2021

Hello,

I'm facing the exact issue.
Editing any file that's over 500+ lines will make things very laggy, including the screen freezing for a couple seconds.

1839  85% - redisplay_internal (C function)
1804  83%  - jit-lock-function
1804  83%   - jit-lock-fontify-now
1804  83%    - jit-lock--run-functions
1804  83%     - #<compiled 0x1f23b15c07c9943e>
1804  83%      - adaptive-wrap-prefix-function
1804  83%       - adaptive-wrap-fill-context-prefix
1804  83%        - apply
1804  83%         - +word-wrap--adjust-extra-indent-a
1804  83%          - let
1799  83%           - funcall
1799  83%            - #<compiled 0x1669391e9bd2eef4>
1799  83%             - fill-context-prefix
1798  83%              - fill-match-adaptive-prefix
1587  73%               - rustic-find-fill-prefix
1587  73%                - rustic-in-comment-paragraph
1554  72%                 - syntax-ppss
1530  71%                  - syntax-propertize
1513  70%                   - rustic-syntax-propertize
1365  63%                    - rustic-ordinary-lt-gt-p
 739  34%                     + rustic-is-lt-char-operator
 249  11%                     + rustic-in-str-or-cmnt
 238  11%                     + rustic-in-macro
 116   5%                     + rustic-paren-level
   6   0%                     + backward-up-list
 101   4%                    + rustic-macro-scope
   1   0%                      #<compiled 0xb4c1fba9bcca7>
  33   1%                 + #<compiled 0x1ca80e7d7a946>
 211   9%               + current-left-margin
   1   0%              + move-to-left-margin
   5   0%           + +word-wrap--calc-extra-indent
  33   1%  + eval
   2   0%  + #<compiled 0xe2451aa401e81ee>
 156   7% + command-execute
 139   6% + ...
   9   0% + timer-event-handler
   6   0% + #<compiled 0xe0248dc2029577f>
   1   0%   lsp-ui-sideline
   1   0% + whitespace-post-command-hook
   1   0% + sp--save-pre-command-state

I can also confirm that disabling adaptive-wrap-prefix-mode does make the problem go away.
So maybe it's worth putting a note in the README saying that enabling adaptive-wrap-prefix-mode with rustic-mode could lead to severe lag when editing larger files?

https://elpa.gnu.org/packages/adaptive-wrap.html

@brotzeit
Copy link
Owner

The issue now belongs to rust-mode. Please comment in the linked issue instead.

@brotzeit
Copy link
Owner

@darwin67 added a note to the readme

@darwin67
Copy link

Thanks!

@phillord
Copy link
Contributor

I hadn't realised that you had unforked rustic and rust-mode. Good job and thanks for the work!

@brotzeit
Copy link
Owner

@phillord thanks, but @tarsius did most of the work.

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

Successfully merging a pull request may close this issue.

5 participants