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

Compiling rust-uinput takes almost 20 minutes #37074

Closed
Theemuts opened this issue Oct 10, 2016 · 11 comments
Closed

Compiling rust-uinput takes almost 20 minutes #37074

Theemuts opened this issue Oct 10, 2016 · 11 comments
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..)

Comments

@Theemuts
Copy link

Hi,

A few minutes ago I opened an issue on the rust-uinput repo, but @meh advised me to open one here. If I compile that library, it takes a significant amount of time which I haven't experienced while compiling before, potentially due to custom_derive. My computer has an i5 2550K, 8GB of ram, is running Ubuntu 16.04 and rustc 1.12.

@jonas-schievink
Copy link
Contributor

It indeed gets stuck inside expansion. Can also reproduce on nightly. I'm on 64-bit Arch Linux with 24GB RAM.

cc @jseyfried and @nrc probably

@durka
Copy link
Contributor

durka commented Oct 10, 2016

It's probably because of enums (with macros running over them) like this (102 variants) and this (232 variants).

@nrc
Copy link
Member

nrc commented Oct 11, 2016

Would be good to narrow this down. It could be the custom derive for a big enum, or it could be the input to the custom_derive macro itself. I'd recommend using macros 1.1 custom derive in any case. But we should try and find out why this takes so long to compile.

cc @rust-lang/compiler

@nrc nrc added the A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) label Oct 11, 2016
@durka
Copy link
Contributor

durka commented Oct 11, 2016

Note that custom_derive! (and IterVariants! which it calls here) are token munchers. Is there any way to profile expansion except trace_macros?

@jseyfried jseyfried self-assigned this Nov 1, 2016
alexcrichton added a commit to alexcrichton/rust that referenced this issue Nov 4, 2016
…=eddyb

macros: improve expansion performance

This PR fixes that regression, further improves performance on recursive, `tt`-heavy workloads, and makes a variety of other improvements to parsing and expansion performance.

Expansion performance improvements:

| Test case      | Run-time | Memory usage |
| -------------- | -------- | ------------ |
| libsyntax      | 8%       | 10%          |
| librustc       | 15%      | 6%           |
| librustc_trans | 30%      | 6%           |
| rust-lang#37074         | 20%      | 15%          |
| rust-lang#34630         | 40%      | 8%           |

r? @eddyb
alexcrichton added a commit to alexcrichton/rust that referenced this issue Nov 5, 2016
…=eddyb

macros: improve expansion performance

This PR fixes that regression, further improves performance on recursive, `tt`-heavy workloads, and makes a variety of other improvements to parsing and expansion performance.

Expansion performance improvements:

| Test case      | Run-time | Memory usage |
| -------------- | -------- | ------------ |
| libsyntax      | 8%       | 10%          |
| librustc       | 15%      | 6%           |
| librustc_trans | 30%      | 6%           |
| rust-lang#37074         | 20%      | 15%          |
| rust-lang#34630         | 40%      | 8%           |

r? @eddyb
bors added a commit that referenced this issue Nov 14, 2016
…fried

Macro parser performance improvements and refactoring

This PR locally increased performance of #37074 by ~6.6 minutes.

Follow up to #37569, but doesn't focus explicitly on expansion performance.

History is relatively clean, but I can/will do some more polishing if this is deemed mergeable. Partially posting this now so I can get Travis to run tests for me.

r? @jseyfried
@Mark-Simulacrum
Copy link
Member

@Theemuts Can you test with a recent nightly and see if the performance improvements were as-expected? We can close this then, since I don't know if there's anything overly helpful to be gained from keeping this open (other than a "expansion can be slow" issue?).

@Mark-Simulacrum
Copy link
Member

So there's certainly more room for improvement here, as always, but I think we made some fairly large advances with #37701 and #37569, so I'm going to close. If @Theemuts still sees extremely long compile times then we can reopen, but I think they should be down to something at least a little more manageable.

@oberien
Copy link
Contributor

oberien commented Jun 5, 2017

On rustc 1.19.0-nightly (0418fa9 2017-06-04) compilation of uinput still takes 5 minutes. I mean this is way better than 20 minutes, but could definitely take some more improvements.

@Mark-Simulacrum
Copy link
Member

Hm, 5 minutes does seem rather long. I'll reopen and do some local testing to see if I can find any more clear slow spots.

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Jun 7, 2017

Just a quick update: I have a local (relatively -- libstd builds) untested branch that runs expansion on rust-uinput in ~12 seconds. I've never actually waited for expansion to complete on nightly, though, so not sure how much of an improvement this is yet. It's most certainly in the ballpark of 6 minutes, though.

Edit: 440.130 seconds on nightly.

@eddyb
Copy link
Member

eddyb commented Jun 8, 2017

@Mark-Simulacrum Whoa, what'd you do?!

@oberien
Copy link
Contributor

oberien commented Jun 8, 2017

12 seconds is amazing! Does it only influence rust-uinput or is it working for all code expansions? How could it influence other libraries?

bors added a commit that referenced this issue Jun 10, 2017
…yfried

Speed up expansion

This reduces duplication, thereby increasing expansion speed. Based on tests with rust-uinput, this produces a 29x performance win (440 seconds to 15 seconds). I want to land this first, since it's a minimal patch, but with more changes to the macro parsing I can get down to 12 seconds locally.

There is one FIXME added to the code that I'll keep for now since changing it will spread outward and increase the patch size, I think.

Fixes #37074.

r? @jseyfried
cc @oberien
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..)
Projects
None yet
Development

No branches or pull requests

8 participants