diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dee7866e5..d06bd37693 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- async fetching tags to improve reactivity in giant repos ([#170](https://github.com/extrawurst/gitui/issues/170)) + ### Fixed - removed unmaintained dependency `spin` ([#172](https://github.com/extrawurst/gitui/issues/172)) - opening relative paths in external editor may fail in subpaths ([#184](https://github.com/extrawurst/gitui/issues/184)) - crashes in revlog with utf8 commit messages ([#188](https://github.com/extrawurst/gitui/issues/188)) - `add_to_ignore` failed on files without a newline at EOF ([#191](https://github.com/extrawurst/gitui/issues/191)) +- new tags were not picked up in revlog view ([#190](https://github.com/extrawurst/gitui/issues/190)) ## [0.8.1] - 2020-07-07 diff --git a/asyncgit/src/diff.rs b/asyncgit/src/diff.rs index 69ace08909..f149d10482 100644 --- a/asyncgit/src/diff.rs +++ b/asyncgit/src/diff.rs @@ -122,11 +122,13 @@ impl AsyncDiff { arc_pending.fetch_sub(1, Ordering::Relaxed); - if notify { - sender - .send(AsyncNotification::Diff) - .expect("error sending diff"); - } + sender + .send(if notify { + AsyncNotification::Diff + } else { + AsyncNotification::FinishUnchanged + }) + .expect("error sending diff"); }); Ok(None) diff --git a/asyncgit/src/lib.rs b/asyncgit/src/lib.rs index e1632a00e0..d18c04ea41 100644 --- a/asyncgit/src/lib.rs +++ b/asyncgit/src/lib.rs @@ -13,6 +13,7 @@ mod error; mod revlog; mod status; pub mod sync; +mod tags; pub use crate::{ commit_files::AsyncCommitFiles, @@ -23,6 +24,7 @@ pub use crate::{ diff::{DiffLine, DiffLineType, FileDiff}, status::{StatusItem, StatusItemType}, }, + tags::AsyncTags, }; use std::{ collections::hash_map::DefaultHasher, @@ -32,6 +34,8 @@ use std::{ /// this type is used to communicate events back through the channel #[derive(Copy, Clone, Debug, PartialEq)] pub enum AsyncNotification { + /// this indicates that no new state was fetched but that a async process finished + FinishUnchanged, /// Status, /// @@ -40,6 +44,8 @@ pub enum AsyncNotification { Log, /// CommitFiles, + /// + Tags, } /// current working director `./` diff --git a/asyncgit/src/revlog.rs b/asyncgit/src/revlog.rs index 6fde2e9318..281b3f09a5 100644 --- a/asyncgit/src/revlog.rs +++ b/asyncgit/src/revlog.rs @@ -128,6 +128,7 @@ impl AsyncLog { ) .expect("failed to fetch"); arc_pending.store(false, Ordering::Relaxed); + Self::notify(&sender); }); diff --git a/asyncgit/src/sync/commits_info.rs b/asyncgit/src/sync/commits_info.rs index d81e6978bf..bb02d574f0 100644 --- a/asyncgit/src/sync/commits_info.rs +++ b/asyncgit/src/sync/commits_info.rs @@ -4,7 +4,7 @@ use git2::{Commit, Error, Oid}; use scopetime::scope_time; /// identifies a single commit -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Ord, PartialOrd)] pub struct CommitId(Oid); impl CommitId { diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index 9e5b742f98..acf6f08f37 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -17,6 +17,7 @@ mod tags; pub mod utils; pub(crate) use branch::get_branch_name; + pub use commit::{amend, commit}; pub use commit_details::{get_commit_details, CommitDetails}; pub use commit_files::get_commit_files; diff --git a/asyncgit/src/sync/tags.rs b/asyncgit/src/sync/tags.rs index 4fa09fdeb1..5e6c7cc31c 100644 --- a/asyncgit/src/sync/tags.rs +++ b/asyncgit/src/sync/tags.rs @@ -1,10 +1,10 @@ use super::{utils::repo, CommitId}; use crate::error::Result; use scopetime::scope_time; -use std::collections::HashMap; +use std::collections::BTreeMap; /// hashmap of tag target commit hash to tag names -pub type Tags = HashMap>; +pub type Tags = BTreeMap>; /// returns `Tags` type filled with all tags found in repo pub fn get_tags(repo_path: &str) -> Result { diff --git a/asyncgit/src/tags.rs b/asyncgit/src/tags.rs new file mode 100644 index 0000000000..20999c6983 --- /dev/null +++ b/asyncgit/src/tags.rs @@ -0,0 +1,127 @@ +use crate::{ + error::Result, + hash, + sync::{self}, + AsyncNotification, CWD, +}; +use crossbeam_channel::Sender; +use std::{ + sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, Mutex, + }, + time::{Duration, Instant}, +}; +use sync::Tags; + +/// +#[derive(Default, Clone)] +struct TagsResult { + hash: u64, + tags: Tags, +} + +/// +pub struct AsyncTags { + last: Arc>>, + sender: Sender, + pending: Arc, +} + +impl AsyncTags { + /// + pub fn new(sender: &Sender) -> Self { + Self { + last: Arc::new(Mutex::new(None)), + sender: sender.clone(), + pending: Arc::new(AtomicUsize::new(0)), + } + } + + /// last fetched result + pub fn last(&mut self) -> Result> { + let last = self.last.lock()?; + + Ok(last.clone().map(|last| last.1.tags)) + } + + /// + pub fn is_pending(&self) -> bool { + self.pending.load(Ordering::Relaxed) > 0 + } + + fn is_outdated(&self, dur: Duration) -> Result { + let last = self.last.lock()?; + + Ok(last + .as_ref() + .map(|(last_time, _)| last_time.elapsed() > dur) + .unwrap_or(true)) + } + + /// + pub fn request( + &mut self, + dur: Duration, + force: bool, + ) -> Result<()> { + log::trace!("request"); + + if !force && (self.is_pending() || !self.is_outdated(dur)?) { + return Ok(()); + } + + let arc_last = Arc::clone(&self.last); + let sender = self.sender.clone(); + let arc_pending = Arc::clone(&self.pending); + rayon_core::spawn(move || { + arc_pending.fetch_add(1, Ordering::Relaxed); + + let notify = AsyncTags::getter(arc_last) + .expect("error getting tags"); + + arc_pending.fetch_sub(1, Ordering::Relaxed); + + sender + .send(if notify { + AsyncNotification::Tags + } else { + AsyncNotification::FinishUnchanged + }) + .expect("error sending notify"); + }); + + Ok(()) + } + + fn getter( + arc_last: Arc>>, + ) -> Result { + let tags = sync::get_tags(CWD)?; + + let hash = hash(&tags); + + if Self::last_hash(arc_last.clone()) + .map(|last| last == hash) + .unwrap_or_default() + { + return Ok(false); + } + + { + let mut last = arc_last.lock()?; + let now = Instant::now(); + *last = Some((now, TagsResult { tags, hash })); + } + + Ok(true) + } + + fn last_hash( + last: Arc>>, + ) -> Option { + last.lock() + .ok() + .and_then(|last| last.as_ref().map(|(_, last)| last.hash)) + } +} diff --git a/src/components/commit_details/details.rs b/src/components/commit_details/details.rs index 76975ad71b..ef4e01d7f8 100644 --- a/src/components/commit_details/details.rs +++ b/src/components/commit_details/details.rs @@ -40,7 +40,7 @@ impl DetailsComponent { pub fn set_commit( &mut self, id: Option, - tags: &Tags, + tags: Option<&Tags>, ) -> Result<()> { self.tags.clear(); @@ -51,8 +51,10 @@ impl DetailsComponent { }; if let Some(id) = id { - if let Some(tags) = tags.get(&id) { - self.tags.extend(tags.clone()); + if let Some(tags) = tags { + if let Some(tags) = tags.get(&id) { + self.tags.extend(tags.clone()); + } } } diff --git a/src/components/commit_details/mod.rs b/src/components/commit_details/mod.rs index 60a21e63d0..b706ffc072 100644 --- a/src/components/commit_details/mod.rs +++ b/src/components/commit_details/mod.rs @@ -68,7 +68,7 @@ impl CommitDetailsComponent { pub fn set_commit( &mut self, id: Option, - tags: &Tags, + tags: Option<&Tags>, ) -> Result<()> { self.details.set_commit(id, tags)?; diff --git a/src/components/commitlist.rs b/src/components/commitlist.rs index 33e48ecd16..b44384403f 100644 --- a/src/components/commitlist.rs +++ b/src/components/commitlist.rs @@ -92,14 +92,8 @@ impl CommitList { self.tags.as_ref() } - /// - pub fn has_tags(&self) -> bool { - self.tags.is_some() - } - /// pub fn clear(&mut self) { - self.tags = None; self.items.clear(); } diff --git a/src/components/inspect_commit.rs b/src/components/inspect_commit.rs index c320f5756a..158b863a2c 100644 --- a/src/components/inspect_commit.rs +++ b/src/components/inspect_commit.rs @@ -9,8 +9,8 @@ use crate::{ }; use anyhow::Result; use asyncgit::{ - sync::{CommitId, Tags}, - AsyncDiff, AsyncNotification, DiffParams, DiffType, + sync::CommitId, AsyncDiff, AsyncNotification, DiffParams, + DiffType, }; use crossbeam_channel::Sender; use crossterm::event::Event; @@ -225,7 +225,7 @@ impl InspectCommitComponent { } fn update(&mut self) -> Result<()> { - self.details.set_commit(self.commit_id, &Tags::new())?; + self.details.set_commit(self.commit_id, None)?; self.update_diff()?; Ok(()) diff --git a/src/tabs/revlog.rs b/src/tabs/revlog.rs index 915bc8816a..d6f9cf0859 100644 --- a/src/tabs/revlog.rs +++ b/src/tabs/revlog.rs @@ -13,10 +13,11 @@ use anyhow::Result; use asyncgit::{ cached, sync::{self, CommitId}, - AsyncLog, AsyncNotification, FetchStatus, CWD, + AsyncLog, AsyncNotification, AsyncTags, FetchStatus, CWD, }; use crossbeam_channel::Sender; use crossterm::event::Event; +use std::time::Duration; use tui::{ backend::Backend, layout::{Constraint, Direction, Layout, Rect}, @@ -30,6 +31,7 @@ pub struct Revlog { commit_details: CommitDetailsComponent, list: CommitList, git_log: AsyncLog, + git_tags: AsyncTags, queue: Queue, visible: bool, branch_name: cached::BranchName, @@ -51,6 +53,7 @@ impl Revlog { ), list: CommitList::new(strings::LOG_TITLE, theme), git_log: AsyncLog::new(sender), + git_tags: AsyncTags::new(sender), visible: false, branch_name: cached::BranchName::new(CWD), } @@ -59,6 +62,7 @@ impl Revlog { /// pub fn any_work_pending(&self) -> bool { self.git_log.is_pending() + || self.git_tags.is_pending() || self.commit_details.any_work_pending() } @@ -78,9 +82,7 @@ impl Revlog { self.fetch_commits()?; } - if !self.list.has_tags() || log_changed { - self.list.set_tags(sync::get_tags(CWD)?); - } + self.git_tags.request(Duration::from_secs(3), false)?; self.list.set_branch( self.branch_name.lookup().map(Some).unwrap_or(None), @@ -89,7 +91,7 @@ impl Revlog { if self.commit_details.is_visible() { self.commit_details.set_commit( self.selected_commit(), - self.list.tags().expect("tags"), + self.list.tags(), )?; } } @@ -106,6 +108,12 @@ impl Revlog { match ev { AsyncNotification::CommitFiles | AsyncNotification::Log => self.update()?, + AsyncNotification::Tags => { + if let Some(tags) = self.git_tags.last()? { + self.list.set_tags(tags); + self.update()?; + } + } _ => (), } }