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.

Sign up for GitHub

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

Jump to bottom

upstream: Re-implement rust-in-macro for performance #108

Merged
merged 1 commit into from
May 16, 2020
Merged

upstream: Re-implement rust-in-macro for performance #108

merged 1 commit into from
May 16, 2020

Conversation

brotzeit
Copy link
Owner

@brotzeit brotzeit commented May 15, 2020 •
edited
Loading

Thanks @phillord

@brownjohnf can you test this please.

close #107

@brownjohnf
Copy link

Yep, will do. Probably this weekend.

@brotzeit
Copy link
Owner Author

I merge this now, but it would be nice if you could post the differences with these changes.

@brotzeit brotzeit merged commit 52b632d into master May 16, 2020
@brotzeit brotzeit deleted the rust-in-macro branch May 16, 2020 07:59
@brownjohnf
Copy link

This works amazingly well! Including a screenshot of the profiler now, showing basically no resource usage by this code. Thank you SO much to both of you for troubleshooting/making this fix so quickly and helping out an ignorant emacs newb like myself :-).

@brotzeit I'd be happy to make a PR to doom emacs (how I'm consuming it) bumping the version to current master if you think it's ready to go.

Screenshot from 2020-05-16 07-54-31

@brotzeit
Copy link
Owner Author

brotzeit commented May 16, 2020 •
edited
Loading

@brownjohnf Nice! And yeah that would be great, I was planning to open a PR myself.

brownjohnf added a commit to brownjohnf/doom-emacs that referenced this pull request May 17, 2020
@brownjohnf
Upgrade rustic to fix performance regression
dce77ee 
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.
@brownjohnf brownjohnf mentioned this pull request May 17, 2020
Closed
hlissner added a commit to doomemacs/doomemacs that referenced this pull request May 18, 2020
@hlissner @brownjohnf
Bump :lang rust
1b02641 
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 <[email protected]>
@brownjohnf
Copy link

So after using this more, it definitely is better, but still not as snappy as I'd hope, or as fast as it seemed before the regression. I'm not sure what more to do about it though, and it'd be odd if these fixes make it fast enough in rust-mode but not rustic. Maybe I just have a slow machine.

@brotzeit
Copy link
Owner Author

Hmmm, do you know that it's faster in rust-mode ?

@brotzeit
Copy link
Owner Author

When I have the time, I will take a look at it, but I doubt I'm able to really improve it. I'm currently waiting for the native-comp branch in emacs to be more stable. I guess this will bring more performance improvements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Suspected poor performance from rustic-syntax-propertize
2 participants
@brotzeit @brownjohnf