From b1dcdeb79b03a07325c77f7f265e8465f75b19e5 Mon Sep 17 00:00:00 2001 From: Abdelilah El Aissaoui Date: Fri, 29 Sep 2023 13:32:19 +0200 Subject: [PATCH] Added events batching from Rust part to prevent excessive exchange between both sides --- rs/src/watch_directory.rs | 52 +++++++++++++++++-- .../com/jetpackduba/gitnuro/AppConstants.kt | 4 +- .../gitnuro/git/FileChangesWatcher.kt | 5 +- .../com/jetpackduba/gitnuro/git/TabState.kt | 5 +- .../gitnuro/viewmodels/TabViewModel.kt | 46 ++++++---------- 5 files changed, 71 insertions(+), 41 deletions(-) diff --git a/rs/src/watch_directory.rs b/rs/src/watch_directory.rs index 81850a4..24d85b1 100644 --- a/rs/src/watch_directory.rs +++ b/rs/src/watch_directory.rs @@ -1,11 +1,15 @@ extern crate notify; use std::fmt::Debug; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::sync::mpsc::{channel, RecvTimeoutError}; -use std::time::Duration; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use notify::{Config, Error, ErrorKind, Event, RecommendedWatcher, RecursiveMode, Watcher}; +const MIN_TIME_IN_MS_BETWEEN_REFRESHES: u128 = 500; +const WATCH_TIMEOUT: u64 = 500; + + pub fn watch_directory( path: String, notifier: Box, @@ -27,11 +31,49 @@ pub fn watch_directory( .watch(Path::new(path.as_str()), RecursiveMode::Recursive) .map_err(|err| err.kind.into_watcher_init_error())?; + let mut paths_cached: Vec = Vec::new(); + + let mut last_update: u128 = 0; + while notifier.should_keep_looping() { - match rx.recv_timeout(Duration::from_secs(1)) { + let current_time = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("We need a TARDIS to fix this") + .as_millis(); + + // Updates are batched to prevent excessive communication between Kotlin and Rust, as the + // bridge has overhead + if last_update != 0 && current_time > (last_update + MIN_TIME_IN_MS_BETWEEN_REFRESHES) { + last_update = 0; + + if paths_cached.len() == 1 { + let first_path = paths_cached.first().unwrap(); + let is_dir = PathBuf::from(first_path).is_dir(); + + if is_dir { + println!("Ignored path cached {first_path} because it is a dir"); + } else { + println!("Sending single file event to Kotlin side"); + notifier.detected_change(paths_cached.to_vec()); + } + } else { + println!("Sending batched events to Kotlin side"); + notifier.detected_change(paths_cached.to_vec()); + } + + paths_cached.clear(); + } + + match rx.recv_timeout(Duration::from_millis(WATCH_TIMEOUT)) { Ok(e) => { - if let Some(paths) = get_paths_from_event_result(&e) { - notifier.detected_change(paths) + if let Some(mut paths) = get_paths_from_event_result(&e) { + paths_cached.append(&mut paths); + + last_update = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("We need a TARDIS to fix this") + .as_millis(); + println!("Event: {e:?}"); } } Err(e) => { diff --git a/src/main/kotlin/com/jetpackduba/gitnuro/AppConstants.kt b/src/main/kotlin/com/jetpackduba/gitnuro/AppConstants.kt index dc6bef3..e3b62b2 100644 --- a/src/main/kotlin/com/jetpackduba/gitnuro/AppConstants.kt +++ b/src/main/kotlin/com/jetpackduba/gitnuro/AppConstants.kt @@ -22,8 +22,8 @@ object AppConstants { const val APP_NAME = "Gitnuro" const val APP_DESCRIPTION = "Gitnuro is a Git client that allows you to manage multiple repositories with a modern experience and live visual representation of your repositories' state." - const val APP_VERSION = "1.3.0" - const val APP_VERSION_CODE = 10 + const val APP_VERSION = "1.3.1" + const val APP_VERSION_CODE = 11 const val VERSION_CHECK_URL = "https://raw.githubusercontent.com/JetpackDuba/Gitnuro/main/latest.json" } diff --git a/src/main/kotlin/com/jetpackduba/gitnuro/git/FileChangesWatcher.kt b/src/main/kotlin/com/jetpackduba/gitnuro/git/FileChangesWatcher.kt index e359d8b..5cdf638 100644 --- a/src/main/kotlin/com/jetpackduba/gitnuro/git/FileChangesWatcher.kt +++ b/src/main/kotlin/com/jetpackduba/gitnuro/git/FileChangesWatcher.kt @@ -59,7 +59,10 @@ class FileChangesWatcher @Inject constructor( // JGit may create .probe-UUID files for its internal stuff, we should not care about it val onlyProbeFiles = paths.all { it.contains("$systemSeparator.git$systemSeparator.probe-") } - matchesAnyIgnoreRule || isGitIgnoredFile || onlyProbeFiles + // Ignore it if the change is the directory itself + val isGitDir = paths.count() == 1 && paths.first() == "$pathStr$systemSeparator.git$systemSeparator" + + matchesAnyIgnoreRule || isGitIgnoredFile || onlyProbeFiles || isGitDir } val hasGitDirChanged = paths.any { it.startsWith("$pathStr$systemSeparator.git$systemSeparator") } diff --git a/src/main/kotlin/com/jetpackduba/gitnuro/git/TabState.kt b/src/main/kotlin/com/jetpackduba/gitnuro/git/TabState.kt index 84bd76e..27eea22 100644 --- a/src/main/kotlin/com/jetpackduba/gitnuro/git/TabState.kt +++ b/src/main/kotlin/com/jetpackduba/gitnuro/git/TabState.kt @@ -90,8 +90,6 @@ class TabState @Inject constructor( var refreshEvenIfCrashesInteractiveResult = false operationRunning = true - lastOperation = System.currentTimeMillis() - val processingInfo: ProcessingInfo = object : ProcessingInfo { override fun changeSubtitle(newSubtitle: String) { _processing.update { processingState -> @@ -152,6 +150,7 @@ class TabState @Inject constructor( } finally { _processing.value = ProcessingState.None operationRunning = false + lastOperation = System.currentTimeMillis() if (refreshType != RefreshType.NONE && (!hasProcessFailed || refreshEvenIfCrashes || refreshEvenIfCrashesInteractiveResult)) { _refreshData.emit(refreshType) @@ -217,7 +216,6 @@ class TabState @Inject constructor( var hasProcessFailed = false operationRunning = true - lastOperation = System.currentTimeMillis() try { block(git) @@ -235,6 +233,7 @@ class TabState @Inject constructor( _refreshData.emit(refreshType) operationRunning = false + lastOperation = System.currentTimeMillis() } } diff --git a/src/main/kotlin/com/jetpackduba/gitnuro/viewmodels/TabViewModel.kt b/src/main/kotlin/com/jetpackduba/gitnuro/viewmodels/TabViewModel.kt index 3c0e8ed..0caf766 100644 --- a/src/main/kotlin/com/jetpackduba/gitnuro/viewmodels/TabViewModel.kt +++ b/src/main/kotlin/com/jetpackduba/gitnuro/viewmodels/TabViewModel.kt @@ -40,7 +40,6 @@ import java.io.File import javax.inject.Inject import javax.inject.Provider -private const val MIN_TIME_IN_MS_BETWEEN_REFRESHES = 1000L private const val MIN_TIME_AFTER_GIT_OPERATION = 2000L private const val TAG = "TabViewModel" @@ -202,10 +201,14 @@ class TabViewModel @Inject constructor( _showAuthorInfo.value = false authorViewModel = null } + /** + * Sometimes external apps can run filesystem multiple operations in a fraction of a second. + * To prevent excessive updates, we add a slight delay between updates emission to prevent slowing down + * the app by constantly running "git status" or even full refreshes. + * + */ private suspend fun watchRepositoryChanges(git: Git) = tabScope.launch(Dispatchers.IO) { - var asyncJob: Job? = null - var lastNotify = 0L var hasGitDirChanged = false launch { @@ -213,15 +216,6 @@ class TabViewModel @Inject constructor( if (!tabState.operationRunning) { // Only update if there isn't any process running printDebug(TAG, "Detected changes in the repository's directory") - if (latestUpdateChangedGitDir) { - hasGitDirChanged = true - } - - asyncJob?.cancel() - - // Sometimes external apps can run filesystem multiple operations in a fraction of a second. - // To prevent excessive updates, we add a slight delay between updates emission to prevent slowing down - // the app by constantly running "git status". val currentTimeMillis = System.currentTimeMillis() if (currentTimeMillis - tabState.lastOperation < MIN_TIME_AFTER_GIT_OPERATION) { @@ -229,25 +223,17 @@ class TabViewModel @Inject constructor( return@collect } - val diffTime = currentTimeMillis - lastNotify - - // When .git dir has changed, do the refresh with a delay to avoid doing operations while a git - // operation may be running - if (diffTime > MIN_TIME_IN_MS_BETWEEN_REFRESHES && !hasGitDirChanged) { - updateApp(false) - printDebug(TAG, "Sync emit with diff time $diffTime") - } else { - asyncJob = async { - delay(MIN_TIME_IN_MS_BETWEEN_REFRESHES) - printDebug(TAG, "Async emit") - if (isActive) - updateApp(hasGitDirChanged) - - hasGitDirChanged = false - } + if (latestUpdateChangedGitDir) { + hasGitDirChanged = true } - lastNotify = currentTimeMillis + if (isActive) { + updateApp(hasGitDirChanged) + } + + hasGitDirChanged = false + } else { + printDebug(TAG, "Ignored file events during operation") } } } @@ -267,7 +253,7 @@ class TabViewModel @Inject constructor( is WatcherInitException.WatchNotFound -> null // This should never trigger as we don't unwatch files } - if(message != null) { + if (message != null) { errorsManager.addError( newErrorNow( exception = ex,