Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Detect the long first build and tell the user #377

Closed
sophiajt opened this issue Jun 23, 2017 · 12 comments
Closed

Detect the long first build and tell the user #377

sophiajt opened this issue Jun 23, 2017 · 12 comments
Labels
Milestone

Comments

@sophiajt
Copy link

If it's possible, can we detect when we're going to have to do a long rebuild and tell the user "Rebuilding dependencies (this may take a few minutes)" or somesuch?

I saw a few people mention not knowing if it was working because the spinner just kept spinning when it was their first build.

@Xanewok
Copy link
Member

Xanewok commented Jul 7, 2017

@nrc @jonathandturner maybe we could introduce a custom server->client notification, such as rustDocument/updateProgress, which could initially return an update string, e.g. Compiling languageserver-types v0.11.1 ($num left) on every compilation start, where $num would be a counter that would be increased and decreased on each crate compilation start/stop?

It seems that there's something being worked on for LSP 4.0 spec (microsoft/language-server-protocol#70), so it might be worth having that in mind.

@nrc
Copy link
Member

nrc commented Jul 7, 2017

The hard bit of this is more on the Cargo side - we don't know whether Cargo is going to rebuild everything or nothing. At the moment we don't know how many crates Cargo is going to build either. We could keep count of how many have been done. Saying exactly which crate is building (or just built) is tricky because we could be building multiple crates in parallel.

@Xanewok
Copy link
Member

Xanewok commented Jul 8, 2017

($num left)

My bad, I wanted to say 'currently building' here, so something like Compiling languageserver-types v0.11.1 ($num other active). Without an explicit build plan prepared beforehand I don't think we can or should estimate how many crates Cargo will want to rebuild.

Saying exactly which crate is building (or just built) is tricky because we could be building multiple crates in parallel.

Cargo also doesn't say which one are actively building now as well, so I think displaying most recently started crate as a least effort attempt could be acceptable here.
To improve that we could hold a simple vec of structs like { name, time_started } and select currently longest compiling crate to kind of emphasize who's the biggest offender in terms of compilation time, but I guess it's more of a detail that can be resolved later on.

@Nashenas88
Copy link
Contributor

What about working with the cargo team to get some kind of notification API set up? Then we can tell the user exactly which crates are being rebuilt.

@nrc
Copy link
Member

nrc commented Jul 8, 2017

What about working with the cargo team to get some kind of notification API set up? Then we can tell the user exactly which crates are being rebuilt.

We can get this info at the moment since we intercept every rustc call. I'm not sure how to present this info to the user though. I suppose it is all just wiring it together though.

Example problem, the project has two dep crates, foo and bar, foo is very small, bar is very big. They don't depend on each other, but the next crate depends on both of them. We start building bar first, so we flash to the user 'building bar', then we start building foo, so we replace that message with 'building foo', that completes quickly, but we don't start another crate for a long time (because we're waiting for bar), but to the user it looks like foo is taking ages. We could display all crates currently being built, but that would be a bit complicated.

@Xanewok
Copy link
Member

Xanewok commented Jul 8, 2017

but we don't start another crate for a long time (because we're waiting for bar), but to the user it looks like foo is taking ages.

I'd say we update the progress on both start and end of compilation, so in my previous example we'd update num of actively built crates, as well as the longest compiling till now crate - I don't think it'd still be an issue?

@Nashenas88
Copy link
Contributor

What about a different message based on plurality? If we're only building one crate, display that, but if we're building more, just display the count. Could we also make it so it's something you can click to display a list of all currently building/built crates?

@nrc
Copy link
Member

nrc commented Jul 8, 2017

num of actively built crates

this would work, I'm not sure how informative it would be

as well as the longest compiling till now crate

this too, just a matter of complexity

What about a different message based on plurality? If we're only building one crate, display that, but if we're building more, just display the count.

this too

Could we also make it so it's something you can click to display a list of all currently building/built crates?

I don't think vscode has APIs for this, but I might be wrong

@NoraCodes
Copy link

Taking into account both this scenario:

Example problem, the project has two dep crates, foo and bar, foo is very small, bar is very big. They don't depend on each other, but the next crate depends on both of them. We start building bar first, so we flash to the user 'building bar', then we start building foo, so we replace that message with 'building foo', that completes quickly, but we don't start another crate for a long time (because we're waiting for bar), but to the user it looks like foo is taking ages.

And this suggestion:

What about a different message based on plurality? If we're only building one crate, display that, but if we're building more, just display the count.

I suggest putting all the builds in a stack by most recently started. If there's more than one element, display the count and the build at the top; otherwise display the single element.

@algesten
Copy link
Contributor

algesten commented Jan 7, 2018

Together with @Xanewok over IRC i started doing this.

wip here https://github.com/algesten/rls/tree/progress-params

Next step is to calculate the progress percentages and send them back for the two cases:

  1. On first build in the RlsExectutor callbacks.
  2. On cached build in the JobQueue::execute() function.

This currently relies on a fork off languageserver-types 0.16 that adds the Progress LS message.

@algesten
Copy link
Contributor

algesten commented Jan 8, 2018

Did a bit more today. Now started sending some kind of progress message, but there are a couple of issues to resolve.

  1. the progress messages are out of order, we get the first update before the begin message
  2. the tests fail as a consequence
  3. changed the type from f64 to an enum, but the import of action::ProgressUpdate might be awkward in cargo.rs etc.

@algesten
Copy link
Contributor

I've opened a PR to get the code review/feedback going.

@nrc nrc closed this as completed in #653 Feb 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants