Scale progress down

There are two reasons why we don't want a generic ra_progress crate
just yet:

*First*, it introduces a common interface between separate components,
and that is usually undesirable (b/c components start to fit the
interface, rather than doing what makes most sense for each particular
component).

*Second*, it introduces a separate async channel for progress, which
makes it harder to correlate progress reports with the work done. Ie,
when we see 100% progress, it's not blindly obvious that the work has
actually finished, we might have some pending messages still.
This commit is contained in:
Aleksey Kladov 2020-06-25 08:01:03 +02:00
parent 76a530242a
commit 874a5f80c7
11 changed files with 101 additions and 353 deletions

13
Cargo.lock generated
View file

@ -967,7 +967,6 @@ dependencies = [
"crossbeam-channel",
"jod-thread",
"log",
"ra_progress",
"ra_toolchain",
"serde_json",
]
@ -1081,11 +1080,7 @@ dependencies = [
"ra_hir",
"ra_ide_db",
"ra_prof",
<<<<<<< HEAD
"ra_ssr",
=======
"ra_progress",
>>>>>>> Veetaha-feat/sync-branch
"ra_syntax",
"ra_text_edit",
"rand",
@ -1173,13 +1168,6 @@ dependencies = [
"ra_arena",
]
[[package]]
name = "ra_progress"
version = "0.1.0"
dependencies = [
"crossbeam-channel",
]
[[package]]
name = "ra_project_model"
version = "0.1.0"
@ -1404,7 +1392,6 @@ dependencies = [
"ra_mbe",
"ra_proc_macro_srv",
"ra_prof",
"ra_progress",
"ra_project_model",
"ra_syntax",
"ra_text_edit",

View file

@ -14,4 +14,3 @@ cargo_metadata = "0.10.0"
serde_json = "1.0.48"
jod-thread = "0.1.1"
ra_toolchain = { path = "../ra_toolchain" }
ra_progress = { path = "../ra_progress" }

View file

@ -17,9 +17,6 @@ pub use cargo_metadata::diagnostic::{
Applicability, Diagnostic, DiagnosticLevel, DiagnosticSpan, DiagnosticSpanMacroExpansion,
};
type Progress = ra_progress::Progress<(), String>;
type ProgressSource = ra_progress::ProgressSource<(), String>;
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum FlycheckConfig {
CargoCommand {
@ -59,15 +56,11 @@ pub struct Flycheck {
}
impl Flycheck {
pub fn new(
config: FlycheckConfig,
workspace_root: PathBuf,
progress_src: ProgressSource,
) -> Flycheck {
pub fn new(config: FlycheckConfig, workspace_root: PathBuf) -> Flycheck {
let (task_send, task_recv) = unbounded::<CheckTask>();
let (cmd_send, cmd_recv) = unbounded::<CheckCommand>();
let handle = jod_thread::spawn(move || {
FlycheckThread::new(config, workspace_root, progress_src).run(&task_send, &cmd_recv);
FlycheckThread::new(config, workspace_root).run(&task_send, &cmd_recv);
});
Flycheck { task_recv, cmd_send, handle }
}
@ -85,6 +78,16 @@ pub enum CheckTask {
/// Request adding a diagnostic with fixes included to a file
AddDiagnostic { workspace_root: PathBuf, diagnostic: Diagnostic },
/// Request check progress notification to client
Status(Status),
}
#[derive(Debug)]
pub enum Status {
Being,
Progress(String),
End,
}
pub enum CheckCommand {
@ -96,8 +99,6 @@ struct FlycheckThread {
config: FlycheckConfig,
workspace_root: PathBuf,
last_update_req: Option<Instant>,
progress_src: ProgressSource,
progress: Option<Progress>,
// XXX: drop order is significant
message_recv: Receiver<CheckEvent>,
/// WatchThread exists to wrap around the communication needed to be able to
@ -109,17 +110,11 @@ struct FlycheckThread {
}
impl FlycheckThread {
fn new(
config: FlycheckConfig,
workspace_root: PathBuf,
progress_src: ProgressSource,
) -> FlycheckThread {
fn new(config: FlycheckConfig, workspace_root: PathBuf) -> FlycheckThread {
FlycheckThread {
config,
workspace_root,
progress_src,
last_update_req: None,
progress: None,
message_recv: never(),
check_process: None,
}
@ -157,9 +152,9 @@ impl FlycheckThread {
}
}
fn clean_previous_results(&mut self, task_send: &Sender<CheckTask>) {
fn clean_previous_results(&self, task_send: &Sender<CheckTask>) {
task_send.send(CheckTask::ClearDiagnostics).unwrap();
self.progress = None;
task_send.send(CheckTask::Status(Status::End)).unwrap();
}
fn should_recheck(&mut self) -> bool {
@ -178,17 +173,18 @@ impl FlycheckThread {
}
}
fn handle_message(&mut self, msg: CheckEvent, task_send: &Sender<CheckTask>) {
fn handle_message(&self, msg: CheckEvent, task_send: &Sender<CheckTask>) {
match msg {
CheckEvent::Begin => {
self.progress = Some(self.progress_src.begin(()));
task_send.send(CheckTask::Status(Status::Being)).unwrap();
}
CheckEvent::End => self.progress = None,
CheckEvent::End => {
task_send.send(CheckTask::Status(Status::End)).unwrap();
}
CheckEvent::Msg(Message::CompilerArtifact(msg)) => {
self.progress
.as_mut()
.expect("check process reported progress without the 'Begin' notification")
.report(msg.target.name);
task_send.send(CheckTask::Status(Status::Progress(msg.target.name))).unwrap();
}
CheckEvent::Msg(Message::CompilerMessage(msg)) => {

View file

@ -1,8 +0,0 @@
[package]
name = "ra_progress"
version = "0.1.0"
authors = ["rust-analyzer developers"]
edition = "2018"
[dependencies]
crossbeam-channel = { version = "0.4" }

View file

@ -1,129 +0,0 @@
//! General-purpose instrumentation for progress reporting.
//!
//! Note:
//! Most of the methods accept `&mut self` just to be more restrictive (for forward compat)
//! even tho for some of them we can weaken this requirement to shared reference (`&self`).
use crossbeam_channel::Receiver;
use std::fmt;
#[derive(Debug)]
pub enum ProgressStatus<B, P> {
Begin(B),
Progress(P),
End,
}
pub struct Progress<B, P>(Option<crossbeam_channel::Sender<ProgressStatus<B, P>>>);
impl<B, P> Progress<B, P> {
pub fn report(&mut self, payload: P) {
self.report_with(|| payload);
}
pub fn report_with(&mut self, payload: impl FnOnce() -> P) {
self.send_status(|| ProgressStatus::Progress(payload()));
}
fn send_status(&self, status: impl FnOnce() -> ProgressStatus<B, P>) {
if let Some(sender) = &self.0 {
sender.try_send(status()).expect("progress report must not block");
}
}
}
impl<B, P> Drop for Progress<B, P> {
fn drop(&mut self) {
self.send_status(|| ProgressStatus::End);
}
}
pub struct ProgressSource<B, P>(Option<crossbeam_channel::Sender<ProgressStatus<B, P>>>);
impl<B, P> ProgressSource<B, P> {
pub fn real_if(real: bool) -> (Receiver<ProgressStatus<B, P>>, Self) {
if real {
let (sender, receiver) = crossbeam_channel::unbounded();
(receiver, Self(Some(sender)))
} else {
(crossbeam_channel::never(), Self(None))
}
}
pub fn begin(&mut self, payload: B) -> Progress<B, P> {
self.begin_with(|| payload)
}
pub fn begin_with(&mut self, payload: impl FnOnce() -> B) -> Progress<B, P> {
let progress = Progress(self.0.clone());
progress.send_status(|| ProgressStatus::Begin(payload()));
progress
}
}
impl<B, P> Clone for ProgressSource<B, P> {
fn clone(&self) -> Self {
Self(self.0.clone())
}
}
impl<B, P> fmt::Debug for ProgressSource<B, P> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_tuple("ProgressSource").field(&self.0).finish()
}
}
pub type U32ProgressStatus = ProgressStatus<U32ProgressReport, U32ProgressReport>;
#[derive(Debug)]
pub struct U32ProgressReport {
pub processed: u32,
pub total: u32,
}
impl U32ProgressReport {
pub fn percentage(&self) -> f64 {
f64::from(100 * self.processed) / f64::from(self.total)
}
pub fn to_message(&self, prefix: &str, unit: &str) -> String {
format!("{} ({}/{} {})", prefix, self.processed, self.total, unit)
}
}
pub struct U32Progress {
inner: Progress<U32ProgressReport, U32ProgressReport>,
processed: u32,
total: u32,
}
#[derive(Debug, Eq, PartialEq)]
pub struct IsDone(pub bool);
impl U32Progress {
pub fn report(&mut self, new_processed: u32) -> IsDone {
if self.processed < new_processed {
self.processed = new_processed;
self.inner.report(U32ProgressReport { processed: new_processed, total: self.total });
}
IsDone(self.processed >= self.total)
}
}
#[derive(Clone)]
pub struct U32ProgressSource {
inner: ProgressSource<U32ProgressReport, U32ProgressReport>,
}
impl U32ProgressSource {
pub fn real_if(
real: bool,
) -> (Receiver<ProgressStatus<U32ProgressReport, U32ProgressReport>>, Self) {
let (recv, inner) = ProgressSource::real_if(real);
(recv, Self { inner })
}
pub fn begin(&mut self, initial: u32, total: u32) -> U32Progress {
U32Progress {
inner: self.inner.begin(U32ProgressReport { processed: initial, total }),
processed: initial,
total,
}
}
}

View file

@ -48,7 +48,6 @@ hir = { path = "../ra_hir", package = "ra_hir" }
hir_def = { path = "../ra_hir_def", package = "ra_hir_def" }
hir_ty = { path = "../ra_hir_ty", package = "ra_hir_ty" }
ra_proc_macro_srv = { path = "../ra_proc_macro_srv" }
ra_progress = { path = "../ra_progress" }
[target.'cfg(windows)'.dependencies]
winapi = "0.3.8"

View file

@ -27,11 +27,7 @@ use crate::{
};
use rustc_hash::{FxHashMap, FxHashSet};
fn create_flycheck(
workspaces: &[ProjectWorkspace],
config: &FlycheckConfig,
progress_src: &ProgressSource<(), String>,
) -> Option<Flycheck> {
fn create_flycheck(workspaces: &[ProjectWorkspace], config: &FlycheckConfig) -> Option<Flycheck> {
// FIXME: Figure out the multi-workspace situation
workspaces.iter().find_map(move |w| match w {
ProjectWorkspace::Cargo { cargo, .. } => {
@ -147,12 +143,7 @@ impl GlobalState {
}
change.set_crate_graph(crate_graph);
let (flycheck_progress_receiver, flycheck_progress_src) =
ProgressSource::real_if(config.client_caps.work_done_progress);
let flycheck = config
.check
.as_ref()
.and_then(|c| create_flycheck(&workspaces, c, &flycheck_progress_src));
let flycheck = config.check.as_ref().and_then(|c| create_flycheck(&workspaces, c));
let mut analysis_host = AnalysisHost::new(lru_capacity);
analysis_host.apply_change(change);
@ -162,8 +153,6 @@ impl GlobalState {
loader,
task_receiver,
flycheck,
flycheck_progress_src,
flycheck_progress_receiver,
diagnostics: Default::default(),
mem_docs: FxHashSet::default(),
vfs: Arc::new(RwLock::new((vfs, FxHashMap::default()))),
@ -181,10 +170,8 @@ impl GlobalState {
pub(crate) fn update_configuration(&mut self, config: Config) {
self.analysis_host.update_lru_capacity(config.lru_capacity);
if config.check != self.config.check {
self.flycheck = config
.check
.as_ref()
.and_then(|it| create_flycheck(&self.workspaces, it, &self.flycheck_progress_src));
self.flycheck =
config.check.as_ref().and_then(|it| create_flycheck(&self.workspaces, it));
}
self.config = config;

View file

@ -29,16 +29,14 @@ mod markdown;
mod diagnostics;
mod line_endings;
mod request_metrics;
mod lsp_utils;
pub mod lsp_ext;
pub mod config;
use serde::de::DeserializeOwned;
pub type Result<T, E = Box<dyn std::error::Error + Send + Sync>> = std::result::Result<T, E>;
pub use crate::{
caps::server_capabilities,
main_loop::{main_loop, show_message},
};
pub use crate::{caps::server_capabilities, lsp_utils::show_message, main_loop::main_loop};
use std::fmt;
pub fn from_json<T: DeserializeOwned>(what: &'static str, json: serde_json::Value) -> Result<T> {

View file

@ -1,10 +1,10 @@
//! Utilities for LSP-related boilerplate code.
use std::error::Error;
use crossbeam_channel::Sender;
use lsp_server::{Message, Notification, Request, RequestId};
use lsp_server::{Message, Notification};
use ra_db::Canceled;
use serde::{de::DeserializeOwned, Serialize};
use std::error::Error;
pub fn show_message(
typ: lsp_types::MessageType,
@ -42,11 +42,3 @@ where
{
Notification::new(N::METHOD.to_string(), params)
}
pub(crate) fn request_new<R>(id: RequestId, params: R::Params) -> Request
where
R: lsp_types::request::Request,
R::Params: Serialize,
{
Request::new(id, R::METHOD.to_string(), params)
}

View file

@ -25,17 +25,10 @@ use crate::{
from_proto,
global_state::{file_id_to_url, GlobalState, GlobalStateSnapshot, Status},
handlers, lsp_ext,
lsp_utils::{is_canceled, notification_cast, notification_is, notification_new, show_message},
request_metrics::RequestMetrics,
LspError, Result,
};
pub use lsp_utils::show_message;
use lsp_utils::{is_canceled, notification_cast, notification_is, notification_new, request_new};
use ra_progress::{
IsDone, ProgressStatus, U32Progress, U32ProgressReport, U32ProgressSource, U32ProgressStatus,
};
const FLYCHECK_PROGRESS_TOKEN: &str = "rustAnalyzer/flycheck";
const ROOTS_SCANNED_PROGRESS_TOKEN: &str = "rustAnalyzer/rootsScanned";
pub fn main_loop(config: Config, connection: Connection) -> Result<()> {
log::info!("initial config: {:#?}", config);
@ -147,18 +140,6 @@ pub fn main_loop(config: Config, connection: Connection) -> Result<()> {
Ok(task) => Event::CheckWatcher(task),
Err(RecvError) => return Err("check watcher died".into()),
},
recv(global_state.flycheck_progress_receiver) -> status => match status {
Ok(status) => Event::ProgressReport(ProgressReport::Flycheck(status)),
Err(RecvError) => return Err("check watcher died".into()),
},
recv(roots_scanned_progress_receiver) -> status => match status {
Ok(status) => Event::ProgressReport(ProgressReport::RootsScanned(status)),
Err(RecvError) => {
// Roots analysis has finished, we no longer need this receiver
roots_scanned_progress_receiver = never();
continue;
}
}
};
if let Event::Msg(Message::Request(req)) = &event {
if connection.handle_shutdown(&req)? {
@ -188,8 +169,6 @@ pub fn main_loop(config: Config, connection: Connection) -> Result<()> {
#[derive(Debug)]
enum Task {
Respond(Response),
Notify(Notification),
SendRequest(Request),
Diagnostic(DiagnosticTask),
}
@ -198,13 +177,6 @@ enum Event {
Task(Task),
Vfs(vfs::loader::Message),
CheckWatcher(CheckTask),
ProgressReport(ProgressReport),
}
#[derive(Debug)]
enum ProgressReport {
Flycheck(ProgressStatus<(), String>),
RootsScanned(U32ProgressStatus),
}
impl fmt::Debug for Event {
@ -221,11 +193,6 @@ impl fmt::Debug for Event {
return debug_verbose_not(not, f);
}
}
Event::Task(Task::Notify(not)) => {
if notification_is::<lsp_types::notification::PublishDiagnostics>(not) {
return debug_verbose_not(not, f);
}
}
Event::Task(Task::Respond(resp)) => {
return f
.debug_struct("Response")
@ -240,7 +207,6 @@ impl fmt::Debug for Event {
Event::Task(it) => fmt::Debug::fmt(it, f),
Event::Vfs(it) => fmt::Debug::fmt(it, f),
Event::CheckWatcher(it) => fmt::Debug::fmt(it, f),
Event::ProgressReport(it) => fmt::Debug::fmt(it, f),
}
}
}
@ -283,16 +249,28 @@ fn loop_turn(
}
}
vfs::loader::Message::Progress { n_total, n_done } => {
if n_done == n_total {
let state = if n_done == 0 {
ProgressState::Start
} else if n_done < n_total {
ProgressState::Report
} else {
assert_eq!(n_done, n_total);
global_state.status = Status::Ready;
became_ready = true;
}
report_progress(global_state, &connection.sender, n_done, n_total, "roots scanned")
ProgressState::End
};
report_progress(
global_state,
&connection.sender,
"roots scanned",
state,
Some(format!("{}/{}", n_done, n_total)),
Some(percentage(n_done, n_total)),
)
}
},
Event::CheckWatcher(task) => on_check_task(task, global_state, task_sender)?,
Event::ProgressReport(report) => {
on_progress_report(report, task_sender, loop_state, global_state)
Event::CheckWatcher(task) => {
on_check_task(task, global_state, task_sender, &connection.sender)?
}
Event::Msg(msg) => match msg {
Message::Request(req) => {
@ -367,9 +345,6 @@ fn on_task(task: Task, msg_sender: &Sender<Message>, global_state: &mut GlobalSt
msg_sender.send(response.into()).unwrap();
}
}
Task::Notify(n) => {
msg_sender.send(n.into()).unwrap();
}
Task::Diagnostic(task) => on_diagnostic_task(task, msg_sender, global_state),
}
}
@ -621,6 +596,7 @@ fn on_check_task(
task: CheckTask,
global_state: &mut GlobalState,
task_sender: &Sender<Task>,
msg_sender: &Sender<Message>,
) -> Result<()> {
match task {
CheckTask::ClearDiagnostics => {
@ -652,39 +628,13 @@ fn on_check_task(
}
CheckTask::Status(status) => {
if global_state.config.client_caps.work_done_progress {
let progress = match status {
ra_flycheck::Status::Being => {
lsp_types::WorkDoneProgress::Begin(lsp_types::WorkDoneProgressBegin {
title: "Running `cargo check`".to_string(),
cancellable: Some(false),
message: None,
percentage: None,
})
}
ra_flycheck::Status::Progress(target) => {
lsp_types::WorkDoneProgress::Report(lsp_types::WorkDoneProgressReport {
cancellable: Some(false),
message: Some(target),
percentage: None,
})
}
ra_flycheck::Status::End => {
lsp_types::WorkDoneProgress::End(lsp_types::WorkDoneProgressEnd {
message: None,
})
}
};
let (state, message) = match status {
ra_flycheck::Status::Being => (ProgressState::Start, None),
ra_flycheck::Status::Progress(target) => (ProgressState::Report, Some(target)),
ra_flycheck::Status::End => (ProgressState::End, None),
};
let params = lsp_types::ProgressParams {
token: lsp_types::ProgressToken::String(
"rustAnalyzer/cargoWatcher".to_string(),
),
value: lsp_types::ProgressParamsValue::WorkDone(progress),
};
let not = notification_new::<lsp_types::notification::Progress>(params);
task_sender.send(Task::Notify(not)).unwrap();
}
report_progress(global_state, msg_sender, "cargo check", state, message, None);
}
};
@ -703,39 +653,55 @@ fn on_diagnostic_task(task: DiagnosticTask, msg_sender: &Sender<Message>, state:
}
}
#[derive(Eq, PartialEq)]
enum ProgressState {
Start,
Report,
End,
}
fn percentage(done: usize, total: usize) -> f64 {
(done as f64 / total.max(1) as f64) * 100.0
}
fn report_progress(
global_state: &mut GlobalState,
sender: &Sender<Message>,
done: usize,
total: usize,
message: &str,
title: &str,
state: ProgressState,
message: Option<String>,
percentage: Option<f64>,
) {
let token = lsp_types::ProgressToken::String(format!("rustAnalyzer/{}", message));
let message = Some(format!("{}/{} {}", done, total, message));
let percentage = Some(100.0 * done as f64 / total.max(1) as f64);
let work_done_progress = if done == 0 {
let work_done_progress_create = global_state.req_queue.outgoing.register(
lsp_types::request::WorkDoneProgressCreate::METHOD.to_string(),
lsp_types::WorkDoneProgressCreateParams { token: token.clone() },
DO_NOTHING,
);
sender.send(work_done_progress_create.into()).unwrap();
if !global_state.config.client_caps.work_done_progress {
return;
}
let token = lsp_types::ProgressToken::String(format!("rustAnalyzer/{}", title));
let work_done_progress = match state {
ProgressState::Start => {
let work_done_progress_create = global_state.req_queue.outgoing.register(
lsp_types::request::WorkDoneProgressCreate::METHOD.to_string(),
lsp_types::WorkDoneProgressCreateParams { token: token.clone() },
DO_NOTHING,
);
sender.send(work_done_progress_create.into()).unwrap();
lsp_types::WorkDoneProgress::Begin(lsp_types::WorkDoneProgressBegin {
title: "rust-analyzer".into(),
cancellable: None,
message,
percentage,
})
} else if done < total {
lsp_types::WorkDoneProgress::Report(lsp_types::WorkDoneProgressReport {
cancellable: None,
message,
percentage,
})
} else {
assert!(done == total);
lsp_types::WorkDoneProgress::End(lsp_types::WorkDoneProgressEnd { message })
lsp_types::WorkDoneProgress::Begin(lsp_types::WorkDoneProgressBegin {
title: title.into(),
cancellable: None,
message,
percentage,
})
}
ProgressState::Report => {
lsp_types::WorkDoneProgress::Report(lsp_types::WorkDoneProgressReport {
cancellable: None,
message,
percentage,
})
}
ProgressState::End => {
lsp_types::WorkDoneProgress::End(lsp_types::WorkDoneProgressEnd { message })
}
};
let notification =
notification_new::<lsp_types::notification::Progress>(lsp_types::ProgressParams {
@ -898,41 +864,6 @@ fn update_file_notifications_on_threadpool(
}
}
pub fn show_message(
typ: lsp_types::MessageType,
message: impl Into<String>,
sender: &Sender<Message>,
) {
let message = message.into();
let params = lsp_types::ShowMessageParams { typ, message };
let not = notification_new::<lsp_types::notification::ShowMessage>(params);
sender.send(not.into()).unwrap();
}
fn is_canceled(e: &Box<dyn std::error::Error + Send + Sync>) -> bool {
e.downcast_ref::<Canceled>().is_some()
}
fn notification_is<N: lsp_types::notification::Notification>(notification: &Notification) -> bool {
notification.method == N::METHOD
}
fn notification_cast<N>(notification: Notification) -> std::result::Result<N::Params, Notification>
where
N: lsp_types::notification::Notification,
N::Params: DeserializeOwned,
{
notification.extract(N::METHOD)
}
fn notification_new<N>(params: N::Params) -> Notification
where
N: lsp_types::notification::Notification,
N::Params: Serialize,
{
Notification::new(N::METHOD.to_string(), params)
}
#[cfg(test)]
mod tests {
use lsp_types::{Position, Range, TextDocumentContentChangeEvent};

View file

@ -202,11 +202,7 @@ impl Server {
ProgressParams {
token: lsp_types::ProgressToken::String(ref token),
value: ProgressParamsValue::WorkDone(WorkDoneProgress::End(_)),
<<<<<<< HEAD
} if token == "rustAnalyzer/roots scanned" => true,
=======
} if token == "rustAnalyzer/rootsScanned" => true,
>>>>>>> Veetaha-feat/sync-branch
_ => false,
}
}