Skip to content

Commit 9492404

Browse files
encounterencounter
committed
Job state handling cleanup
1 parent f5f6869 commit 9492404

File tree

10 files changed

+102
-76
lines changed

10 files changed

+102
-76
lines changed

‎src/app.rs

Lines changed: 19 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ use time::{OffsetDateTime, UtcOffset};
1717
use crate::{
1818
config::{build_globset, load_project_config, ProjectUnit, ProjectUnitNode, CONFIG_FILENAMES},
1919
jobs::{
20-
check_update::{queue_check_update, CheckUpdateResult},
21-
objdiff::{queue_build, BuildStatus, ObjDiffResult},
22-
Job, JobResult, JobState, JobStatus,
20+
check_update::{start_check_update, CheckUpdateResult},
21+
objdiff::{start_build, BuildStatus, ObjDiffResult},
22+
Job, JobQueue, JobResult, JobStatus,
2323
},
2424
views::{
2525
appearance::{appearance_window, DEFAULT_COLOR_ROTATION},
@@ -107,7 +107,7 @@ pub struct SymbolReference {
107107
#[serde(default)]
108108
pub struct ViewState {
109109
#[serde(skip)]
110-
pub jobs: Vec<JobState>,
110+
pub jobs: JobQueue,
111111
#[serde(skip)]
112112
pub build: Option<Box<ObjDiffResult>>,
113113
#[serde(skip)]
@@ -142,7 +142,7 @@ pub struct ViewState {
142142
impl Default for ViewState {
143143
fn default() -> Self {
144144
Self {
145-
jobs: vec![],
145+
jobs: Default::default(),
146146
build: None,
147147
highlighted_symbol: None,
148148
selected_symbol: None,
@@ -380,7 +380,7 @@ impl eframe::App for App {
380380
if function_diff_ui(ui, view_state) {
381381
view_state
382382
.jobs
383-
.push(queue_build(config.clone(), view_state.diff_config.clone()));
383+
.push(start_build(config.clone(), view_state.diff_config.clone()));
384384
}
385385
});
386386
} else if view_state.current_view == View::DataDiff
@@ -390,7 +390,7 @@ impl eframe::App for App {
390390
if data_diff_ui(ui, view_state) {
391391
view_state
392392
.jobs
393-
.push(queue_build(config.clone(), view_state.diff_config.clone()));
393+
.push(start_build(config.clone(), view_state.diff_config.clone()));
394394
}
395395
});
396396
} else {
@@ -410,14 +410,7 @@ impl eframe::App for App {
410410

411411
// Windows + request_repaint_after breaks dialogs:
412412
// https://github.com/emilk/egui/issues/2003
413-
if cfg!(windows)
414-
|| view_state.jobs.iter().any(|job| {
415-
if let Some(handle) = &job.handle {
416-
return !handle.is_finished();
417-
}
418-
false
419-
})
420-
{
413+
if cfg!(windows) || view_state.jobs.any_running() {
421414
ctx.request_repaint();
422415
} else {
423416
ctx.request_repaint_after(Duration::from_millis(100));
@@ -433,14 +426,8 @@ impl eframe::App for App {
433426
}
434427

435428
fn post_rendering(&mut self, _window_size_px: [u32; 2], _frame: &eframe::Frame) {
436-
for job in &mut self.view_state.jobs {
437-
let Some(handle) = &job.handle else {
438-
continue;
439-
};
440-
if !handle.is_finished() {
441-
continue;
442-
}
443-
match job.handle.take().unwrap().join() {
429+
for (job, result) in self.view_state.jobs.iter_finished() {
430+
match result {
444431
Ok(result) => {
445432
log::info!("Job {} finished", job.id);
446433
match result {
@@ -496,26 +483,12 @@ impl eframe::App for App {
496483
}
497484
}
498485
}
499-
if self.view_state.jobs.iter().any(|v| v.should_remove) {
500-
let mut i = 0;
501-
while i < self.view_state.jobs.len() {
502-
let job = &self.view_state.jobs[i];
503-
if job.should_remove
504-
&& job.handle.is_none()
505-
&& job.status.read().unwrap().error.is_none()
506-
{
507-
self.view_state.jobs.remove(i);
508-
} else {
509-
i += 1;
510-
}
511-
}
512-
}
486+
self.view_state.jobs.clear_finished();
513487

514488
if let Ok(mut config) = self.config.write() {
515489
let config = &mut *config;
516490

517-
if self.config_modified.load(Ordering::Relaxed) {
518-
self.config_modified.store(false, Ordering::Relaxed);
491+
if self.config_modified.swap(false, Ordering::Relaxed) {
519492
config.config_change = true;
520493
}
521494

@@ -551,23 +524,17 @@ impl eframe::App for App {
551524
}
552525
}
553526

554-
if config.obj_path.is_some() && self.modified.load(Ordering::Relaxed) {
555-
if !self
556-
.view_state
527+
if config.obj_path.is_some()
528+
&& self.modified.swap(false, Ordering::Relaxed)
529+
&& !self.view_state.jobs.is_running(Job::ObjDiff)
530+
{
531+
self.view_state
557532
.jobs
558-
.iter()
559-
.any(|j| j.job_type == Job::ObjDiff && j.handle.is_some())
560-
{
561-
self.view_state.jobs.push(queue_build(
562-
self.config.clone(),
563-
self.view_state.diff_config.clone(),
564-
));
565-
}
566-
self.modified.store(false, Ordering::Relaxed);
533+
.push(start_build(self.config.clone(), self.view_state.diff_config.clone()));
567534
}
568535

569536
if config.queue_update_check {
570-
self.view_state.jobs.push(queue_check_update());
537+
self.view_state.jobs.push(start_check_update());
571538
config.queue_update_check = false;
572539
}
573540
}

‎src/jobs/bindiff.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use anyhow::{Error, Result};
55
use crate::{
66
app::{AppConfig, DiffConfig},
77
diff::diff_objs,
8-
jobs::{queue_job, update_status, Job, JobResult, JobState, Status},
8+
jobs::{start_job, update_status, Job, JobResult, JobState, Status},
99
obj::{elf, ObjInfo},
1010
};
1111

@@ -37,8 +37,8 @@ fn run_build(
3737
Ok(Box::new(BinDiffResult { first_obj: left_obj, second_obj: right_obj }))
3838
}
3939

40-
pub fn queue_bindiff(config: Arc<RwLock<AppConfig>>) -> JobState {
41-
queue_job("Binary diff", Job::BinDiff, move |status, cancel| {
40+
pub fn start_bindiff(config: Arc<RwLock<AppConfig>>) -> JobState {
41+
start_job("Binary diff", Job::BinDiff, move |status, cancel| {
4242
run_build(status, cancel, config).map(JobResult::BinDiff)
4343
})
4444
}

‎src/jobs/check_update.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use anyhow::{Context, Result};
44
use self_update::{cargo_crate_version, update::Release};
55

66
use crate::{
7-
jobs::{queue_job, update_status, Job, JobResult, JobState, Status},
7+
jobs::{start_job, update_status, Job, JobResult, JobState, Status},
88
update::{build_updater, BIN_NAME},
99
};
1010

@@ -26,8 +26,8 @@ fn run_check_update(status: &Status, cancel: Receiver<()>) -> Result<Box<CheckUp
2626
Ok(Box::new(CheckUpdateResult { update_available, latest_release, found_binary }))
2727
}
2828

29-
pub fn queue_check_update() -> JobState {
30-
queue_job("Check for updates", Job::CheckUpdate, move |status, cancel| {
29+
pub fn start_check_update() -> JobState {
30+
start_job("Check for updates", Job::CheckUpdate, move |status, cancel| {
3131
run_check_update(status, cancel).map(JobResult::CheckUpdate)
3232
})
3333
}

‎src/jobs/mod.rs

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,72 @@ pub enum Job {
2727
Update,
2828
}
2929
pub static JOB_ID: AtomicUsize = AtomicUsize::new(0);
30+
31+
#[derive(Default)]
32+
pub struct JobQueue {
33+
pub jobs: Vec<JobState>,
34+
}
35+
36+
impl JobQueue {
37+
/// Adds a job to the queue.
38+
pub fn push(&mut self, state: JobState) { self.jobs.push(state); }
39+
40+
/// Returns whether a job of the given kind is running.
41+
pub fn is_running(&self, kind: Job) -> bool {
42+
self.jobs.iter().any(|j| j.kind == kind && j.handle.is_some())
43+
}
44+
45+
/// Returns whether any job is running.
46+
pub fn any_running(&self) -> bool {
47+
self.jobs.iter().any(|job| {
48+
if let Some(handle) = &job.handle {
49+
return !handle.is_finished();
50+
}
51+
false
52+
})
53+
}
54+
55+
/// Iterates over all jobs mutably.
56+
pub fn iter_mut(&mut self) -> impl Iterator<Item = &mut JobState> + '_ { self.jobs.iter_mut() }
57+
58+
/// Iterates over all finished jobs, returning the job state and the result.
59+
pub fn iter_finished(
60+
&mut self,
61+
) -> impl Iterator<Item = (&mut JobState, std::thread::Result<JobResult>)> + '_ {
62+
self.jobs.iter_mut().filter_map(|job| {
63+
if let Some(handle) = &job.handle {
64+
if !handle.is_finished() {
65+
return None;
66+
}
67+
let result = job.handle.take().unwrap().join();
68+
return Some((job, result));
69+
}
70+
None
71+
})
72+
}
73+
74+
/// Clears all finished jobs.
75+
pub fn clear_finished(&mut self) {
76+
self.jobs.retain(|job| {
77+
!(job.should_remove
78+
&& job.handle.is_none()
79+
&& job.status.read().unwrap().error.is_none())
80+
});
81+
}
82+
83+
/// Removes a job from the queue given its ID.
84+
pub fn remove(&mut self, id: usize) { self.jobs.retain(|job| job.id != id); }
85+
}
86+
3087
pub struct JobState {
3188
pub id: usize,
32-
pub job_type: Job,
89+
pub kind: Job,
3390
pub handle: Option<JoinHandle<JobResult>>,
3491
pub status: Arc<RwLock<JobStatus>>,
3592
pub cancel: Sender<()>,
3693
pub should_remove: bool,
3794
}
95+
3896
#[derive(Default)]
3997
pub struct JobStatus {
4098
pub title: String,
@@ -43,6 +101,7 @@ pub struct JobStatus {
43101
pub status: String,
44102
pub error: Option<anyhow::Error>,
45103
}
104+
46105
pub enum JobResult {
47106
None,
48107
ObjDiff(Box<ObjDiffResult>),
@@ -60,9 +119,9 @@ fn should_cancel(rx: &Receiver<()>) -> bool {
60119

61120
type Status = Arc<RwLock<JobStatus>>;
62121

63-
fn queue_job(
122+
fn start_job(
64123
title: &str,
65-
job_type: Job,
124+
kind: Job,
66125
run: impl FnOnce(&Status, Receiver<()>) -> Result<JobResult> + Send + 'static,
67126
) -> JobState {
68127
let status = Arc::new(RwLock::new(JobStatus {
@@ -89,7 +148,7 @@ fn queue_job(
89148
log::info!("Started job {}", id);
90149
JobState {
91150
id,
92-
job_type,
151+
kind,
93152
handle: Some(handle),
94153
status: status_clone,
95154
cancel: tx,

‎src/jobs/objdiff.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use time::OffsetDateTime;
1111
use crate::{
1212
app::{AppConfig, DiffConfig},
1313
diff::diff_objs,
14-
jobs::{queue_job, update_status, Job, JobResult, JobState, Status},
14+
jobs::{start_job, update_status, Job, JobResult, JobState, Status},
1515
obj::{elf, ObjInfo},
1616
};
1717

@@ -136,8 +136,8 @@ fn run_build(
136136
Ok(Box::new(ObjDiffResult { first_status, second_status, first_obj, second_obj, time }))
137137
}
138138

139-
pub fn queue_build(config: Arc<RwLock<AppConfig>>, diff_config: DiffConfig) -> JobState {
140-
queue_job("Object diff", Job::ObjDiff, move |status, cancel| {
139+
pub fn start_build(config: Arc<RwLock<AppConfig>>, diff_config: DiffConfig) -> JobState {
140+
start_job("Object diff", Job::ObjDiff, move |status, cancel| {
141141
run_build(status, cancel, config, diff_config).map(JobResult::ObjDiff)
142142
})
143143
}

‎src/jobs/update.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use anyhow::{Context, Result};
99
use const_format::formatcp;
1010

1111
use crate::{
12-
jobs::{queue_job, update_status, Job, JobResult, JobState, Status},
12+
jobs::{start_job, update_status, Job, JobResult, JobState, Status},
1313
update::{build_updater, BIN_NAME},
1414
};
1515

@@ -53,8 +53,8 @@ fn run_update(status: &Status, cancel: Receiver<()>) -> Result<Box<UpdateResult>
5353
Ok(Box::from(UpdateResult { exe_path: target_file }))
5454
}
5555

56-
pub fn queue_update() -> JobState {
57-
queue_job("Update app", Job::Update, move |status, cancel| {
56+
pub fn start_update() -> JobState {
57+
start_job("Update app", Job::Update, move |status, cancel| {
5858
run_update(status, cancel).map(JobResult::Update)
5959
})
6060
}

‎src/views/config.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use self_update::cargo_crate_version;
1818
use crate::{
1919
app::{AppConfig, DiffKind, ViewConfig, ViewState},
2020
config::{ProjectUnit, ProjectUnitNode},
21-
jobs::{bindiff::queue_bindiff, objdiff::queue_build, update::queue_update},
21+
jobs::{bindiff::start_bindiff, objdiff::start_build, update::start_update},
2222
update::RELEASE_URL,
2323
};
2424

@@ -101,7 +101,7 @@ pub fn config_ui(ui: &mut egui::Ui, config: &Arc<RwLock<AppConfig>>, view_state:
101101
)
102102
.clicked()
103103
{
104-
view_state.jobs.push(queue_update());
104+
view_state.jobs.push(start_update());
105105
}
106106
if ui
107107
.button("Manual")
@@ -190,7 +190,7 @@ pub fn config_ui(ui: &mut egui::Ui, config: &Arc<RwLock<AppConfig>>, view_state:
190190
build = true;
191191
}
192192
if build {
193-
view_state.jobs.push(queue_build(config.clone(), view_state.diff_config.clone()));
193+
view_state.jobs.push(start_build(config.clone(), view_state.diff_config.clone()));
194194
}
195195
}
196196
} else if view_state.diff_kind == DiffKind::WholeBinary {
@@ -218,7 +218,7 @@ pub fn config_ui(ui: &mut egui::Ui, config: &Arc<RwLock<AppConfig>>, view_state:
218218

219219
if let (Some(_), Some(_)) = (left_obj, right_obj) {
220220
if ui.button("Build").clicked() {
221-
view_state.jobs.push(queue_bindiff(config.clone()));
221+
view_state.jobs.push(start_bindiff(config.clone()));
222222
}
223223
}
224224
}

‎src/views/data_diff.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ pub fn data_diff_ui(ui: &mut egui::Ui, view_state: &mut ViewState) -> bool {
212212
ui.scope(|ui| {
213213
ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace);
214214
ui.style_mut().wrap = Some(false);
215-
if view_state.jobs.iter().any(|job| job.job_type == Job::ObjDiff) {
215+
if view_state.jobs.is_running(Job::ObjDiff) {
216216
ui.colored_label(view_state.view_config.replace_color, "Building…");
217217
} else {
218218
ui.label("Last built:");

‎src/views/function_diff.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,7 @@ pub fn function_diff_ui(ui: &mut egui::Ui, view_state: &mut ViewState) -> bool {
445445
ui.scope(|ui| {
446446
ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace);
447447
ui.style_mut().wrap = Some(false);
448-
if view_state.jobs.iter().any(|job| job.job_type == Job::ObjDiff) {
448+
if view_state.jobs.is_running(Job::ObjDiff) {
449449
ui.colored_label(view_state.view_config.replace_color, "Building…");
450450
} else {
451451
ui.label("Last built:");

‎src/views/jobs.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub fn jobs_ui(ui: &mut egui::Ui, view_state: &mut ViewState) {
66
ui.label("Jobs");
77

88
let mut remove_job: Option<usize> = None;
9-
for (idx, job) in view_state.jobs.iter_mut().enumerate() {
9+
for job in view_state.jobs.iter_mut() {
1010
let Ok(status) = job.status.read() else {
1111
continue;
1212
};
@@ -20,7 +20,7 @@ pub fn jobs_ui(ui: &mut egui::Ui, view_state: &mut ViewState) {
2020
log::error!("Failed to cancel job: {e:?}");
2121
}
2222
} else {
23-
remove_job = Some(idx);
23+
remove_job = Some(job.id);
2424
}
2525
}
2626
});

0 commit comments

Comments
 (0)