diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ad5a05d7..a9a39481 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,11 +12,12 @@ on: jobs: build: name: Build and Test - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} strategy: matrix: rust_version: [stable, 1.70.0] + os: [ubuntu-latest, windows-latest, macos-latest] steps: - uses: actions/checkout@v3 diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a06ccf9..6e101d01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,12 @@ ## Unreleased Changes * Added headless API for Studio companion plugins. ([#631]) +* Improved the visualization for array properties like Tags ([#829]) +* Significantly improved performance of `rojo serve`, `rojo build --watch`, and `rojo sourcemap --watch` on macOS. [#830] [#631]: https://github.com/rojo-rbx/rojo/pull/631 +[#829]: https://github.com/rojo-rbx/rojo/pull/829 +[#830]: https://github.com/rojo-rbx/rojo/pull/830 ## [7.4.0-rc3] - October 25, 2023 * Changed `sourcemap --watch` to only generate the sourcemap when it's necessary ([#800]) @@ -142,7 +146,6 @@ * Add buttons for navigation on the Connected page ([#722]) ### Fixes -* Significantly improved performance of `rojo serve` and `rojo build` on macOS. [#783] * Significantly improved performance of `rojo sourcemap` ([#668]) * Fixed the diff visualizer of connected sessions. ([#674]) * Fixed disconnected session activity. ([#675]) @@ -176,7 +179,6 @@ [#770]: https://github.com/rojo-rbx/rojo/pull/770 [#771]: https://github.com/rojo-rbx/rojo/pull/771 [#774]: https://github.com/rojo-rbx/rojo/pull/774 -[#783]: https://github.com/rojo-rbx/rojo/pull/783 [rbx-dom#299]: https://github.com/rojo-rbx/rbx-dom/pull/299 [rbx-dom#296]: https://github.com/rojo-rbx/rbx-dom/pull/296 diff --git a/Cargo.lock b/Cargo.lock index e850ee23..d0565dc1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1852,7 +1852,6 @@ dependencies = [ "log", "maplit", "memofs", - "notify", "num_cpus", "opener", "paste", diff --git a/Cargo.toml b/Cargo.toml index 8c3ea003..1401d2e0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,7 +69,6 @@ hyper = { version = "0.14.13", features = ["server", "tcp", "http1"] } jod-thread = "0.1.2" log = "0.4.14" maplit = "1.0.2" -notify = "4.0.17" num_cpus = "1.15.0" opener = "0.5.0" rayon = "1.7.0" diff --git a/crates/memofs/CHANGELOG.md b/crates/memofs/CHANGELOG.md index 43516260..c9961d0e 100644 --- a/crates/memofs/CHANGELOG.md +++ b/crates/memofs/CHANGELOG.md @@ -1,7 +1,9 @@ # memofs Changelog ## Unreleased Changes -* Changed the `StdBackend` file watcher to use `PollWatcher` on macOS. +* Changed `StdBackend` file watching component to use minimal recursive watches. [#830] + +[#830]: https://github.com/rojo-rbx/rojo/pull/830 ## 0.2.0 (2021-08-23) * Updated to `crossbeam-channel` 0.5.1. diff --git a/crates/memofs/src/std_backend.rs b/crates/memofs/src/std_backend.rs index 01a554a5..0a8fb3b7 100644 --- a/crates/memofs/src/std_backend.rs +++ b/crates/memofs/src/std_backend.rs @@ -1,38 +1,24 @@ -use std::io; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::sync::mpsc; use std::thread; use std::time::Duration; +use std::{collections::HashSet, io}; use crossbeam_channel::Receiver; -use notify::{DebouncedEvent, RecursiveMode, Watcher}; - -#[cfg(target_os = "macos")] -use notify::PollWatcher; -#[cfg(not(target_os = "macos"))] -use notify::{watcher, RecommendedWatcher}; +use notify::{watcher, DebouncedEvent, RecommendedWatcher, RecursiveMode, Watcher}; use crate::{DirEntry, Metadata, ReadDir, VfsBackend, VfsEvent}; /// `VfsBackend` that uses `std::fs` and the `notify` crate. pub struct StdBackend { - // We use PollWatcher on macos because using the KQueue watcher - // can cause some gnarly performance problems. - #[cfg(target_os = "macos")] - watcher: PollWatcher, - #[cfg(not(target_os = "macos"))] watcher: RecommendedWatcher, - watcher_receiver: Receiver, + watches: HashSet, } impl StdBackend { pub fn new() -> StdBackend { let (notify_tx, notify_rx) = mpsc::channel(); - - #[cfg(target_os = "macos")] - let watcher = PollWatcher::new(notify_tx, Duration::from_millis(50)).unwrap(); - #[cfg(not(target_os = "macos"))] let watcher = watcher(notify_tx, Duration::from_millis(50)).unwrap(); let (tx, rx) = crossbeam_channel::unbounded(); @@ -63,6 +49,7 @@ impl StdBackend { Self { watcher, watcher_receiver: rx, + watches: HashSet::new(), } } } @@ -112,12 +99,22 @@ impl VfsBackend for StdBackend { } fn watch(&mut self, path: &Path) -> io::Result<()> { - self.watcher - .watch(path, RecursiveMode::NonRecursive) - .map_err(|inner| io::Error::new(io::ErrorKind::Other, inner)) + if self.watches.contains(path) + || path + .ancestors() + .any(|ancestor| self.watches.contains(ancestor)) + { + Ok(()) + } else { + self.watches.insert(path.to_path_buf()); + self.watcher + .watch(path, RecursiveMode::Recursive) + .map_err(|inner| io::Error::new(io::ErrorKind::Other, inner)) + } } fn unwatch(&mut self, path: &Path) -> io::Result<()> { + self.watches.remove(path); self.watcher .unwatch(path) .map_err(|inner| io::Error::new(io::ErrorKind::Other, inner)) diff --git a/plugin/src/App/Components/PatchVisualizer/DisplayValue.lua b/plugin/src/App/Components/PatchVisualizer/DisplayValue.lua index 439c95d9..a231b879 100644 --- a/plugin/src/App/Components/PatchVisualizer/DisplayValue.lua +++ b/plugin/src/App/Components/PatchVisualizer/DisplayValue.lua @@ -53,8 +53,23 @@ local function DisplayValue(props) elseif next(props.value) == nil then -- If it's empty, show empty braces textRepresentation = "{}" + elseif next(props.value) == 1 then + -- We don't need to support mixed tables, so checking the first key is enough + -- to determine if it's a simple array + local out, i = table.create(#props.value), 0 + for k, v in props.value do + i += 1 + + -- Wrap strings in quotes + if type(v) == "string" then + v = '"' .. v .. '"' + end + + out[i] = tostring(v) + end + textRepresentation = "{ " .. table.concat(out, ", ") .. " }" else - -- If it has children, list them out + -- Otherwise, show the table contents as a dictionary local out, i = {}, 0 for k, v in pairs(props.value) do i += 1 diff --git a/plugin/src/preloadAssets.lua b/plugin/src/preloadAssets.lua index c37ca032..1cf9b2d8 100644 --- a/plugin/src/preloadAssets.lua +++ b/plugin/src/preloadAssets.lua @@ -13,7 +13,7 @@ function gatherAssetUrlsRecursive(currentTable, currentUrls) if typeof(value) == "string" then table.insert(currentUrls, value) elseif typeof(value) == "table" then - gatherAssetUrlsRecursive(value) + gatherAssetUrlsRecursive(value, currentUrls) end end diff --git a/src/change_processor.rs b/src/change_processor.rs index 2429b590..497ba4e7 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -1,6 +1,5 @@ use std::{ fs, - path::PathBuf, sync::{Arc, Mutex}, }; @@ -126,14 +125,41 @@ impl JobThreadContext { // For a given VFS event, we might have many changes to different parts // of the tree. Calculate and apply all of these changes. let applied_patches = match event { - VfsEvent::Write(path) => { - if path.is_dir() { - return; + VfsEvent::Create(path) | VfsEvent::Remove(path) | VfsEvent::Write(path) => { + let mut tree = self.tree.lock().unwrap(); + let mut applied_patches = Vec::new(); + + // Find the nearest ancestor to this path that has + // associated instances in the tree. This helps make sure + // that we handle additions correctly, especially if we + // receive events for descendants of a large tree being + // created all at once. + let mut current_path = path.as_path(); + let affected_ids = loop { + let ids = tree.get_ids_at_path(¤t_path); + + log::trace!("Path {} affects IDs {:?}", current_path.display(), ids); + + if !ids.is_empty() { + break ids.to_vec(); + } + + log::trace!("Trying parent path..."); + match current_path.parent() { + Some(parent) => current_path = parent, + None => break Vec::new(), + } + }; + + for id in affected_ids { + if let Some(patch) = compute_and_apply_changes(&mut tree, &self.vfs, id) { + if !patch.is_empty() { + applied_patches.push(patch); + } + } } - on_vfs_event(path, &self.tree, &self.vfs) - } - VfsEvent::Create(path) | VfsEvent::Remove(path) => { - on_vfs_event(path, &self.tree, &self.vfs) + + applied_patches } _ => { log::warn!("Unhandled VFS event: {:?}", event); @@ -236,45 +262,6 @@ impl JobThreadContext { } } -// Find the nearest ancestor to this path that has -// associated instances in the tree. This helps make sure -// that we handle additions correctly, especially if we -// receive events for descendants of a large tree being -// created all at once. -fn on_vfs_event( - path: PathBuf, - tree: &Arc>, - vfs: &Arc, -) -> Vec { - let mut tree = tree.lock().unwrap(); - let mut applied_patches = Vec::new(); - - let mut current_path = path.as_path(); - let affected_ids = loop { - let ids = tree.get_ids_at_path(current_path); - - log::trace!("Path {} affects IDs {:?}", current_path.display(), ids); - - if !ids.is_empty() { - break ids.to_vec(); - } - - log::trace!("Trying parent path..."); - match current_path.parent() { - Some(parent) => current_path = parent, - None => break Vec::new(), - } - }; - - for id in affected_ids { - if let Some(patch) = compute_and_apply_changes(&mut tree, vfs, id) { - if !patch.is_empty() { - applied_patches.push(patch); - } - } - } - applied_patches -} fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: Ref) -> Option { let metadata = tree .get_metadata(id) diff --git a/tests/rojo_test/serve_util.rs b/tests/rojo_test/serve_util.rs index d8253612..6f049020 100644 --- a/tests/rojo_test/serve_util.rs +++ b/tests/rojo_test/serve_util.rs @@ -66,7 +66,11 @@ impl TestServeSession { let source_path = Path::new(SERVE_TESTS_PATH).join(name); let dir = tempdir().expect("Couldn't create temporary directory"); - let project_path = dir.path().join(name); + let project_path = dir + .path() + .canonicalize() + .expect("Couldn't canonicalize temporary directory path") + .join(name); let source_is_file = fs::metadata(&source_path).unwrap().is_file();