diff --git a/.cargo/config.toml b/.cargo/config.toml index 0beee4e0..eb6f7479 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,2 +1,7 @@ [target.x86_64-pc-windows-msvc] -rustflags = ["-Ctarget-feature=+crt-static"] \ No newline at end of file +rustflags = ["-Ctarget-feature=+crt-static"] +linker = "lld-link" + +[target.aarch64-pc-windows-msvc] +rustflags = ["-Ctarget-feature=+crt-static"] +linker = "lld-link" \ No newline at end of file diff --git a/.cursor/.gitignore b/.cursor/.gitignore new file mode 100644 index 00000000..df1a13b2 --- /dev/null +++ b/.cursor/.gitignore @@ -0,0 +1 @@ +/logs \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index da84d39c..6ad9b9c5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,6 +13,8 @@ jobs: build: name: Build and Test runs-on: ${{ matrix.os }} + env: + CARGO_INCREMENTAL: "0" strategy: fail-fast: false @@ -20,36 +22,27 @@ jobs: os: [ubuntu-22.04, windows-latest, macos-latest, windows-11-arm, ubuntu-22.04-arm] steps: + - name: Disable Windows Defender + if: runner.os == 'Windows' + run: Set-MpPreference -DisableRealtimeMonitoring $true + - uses: actions/checkout@v4 - with: - submodules: true + + - name: Init rbx-dom submodule + env: + GIT_CONFIG_NOSYSTEM: "1" + run: git submodule update --init --depth 1 rbx-dom - name: Install Rust uses: dtolnay/rust-toolchain@stable - - name: Restore Rust Cache - uses: actions/cache/restore@v4 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ matrix.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + - name: Rust Cache + uses: Swatinem/rust-cache@v2 - - name: Build - run: cargo build --locked --verbose - - - name: Test - run: cargo test --locked --verbose - - - name: Save Rust Cache - uses: actions/cache/save@v4 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ matrix.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + - name: Build and Test + env: + CARGO_TERM_VERBOSE: ${{ runner.debug && 'true' || 'false' }} + run: cargo test --locked --all-targets build-vscode: name: Build VS Code Extension @@ -57,8 +50,9 @@ jobs: steps: - uses: actions/checkout@v4 - with: - submodules: true + + - name: Init vscode-rojo submodule + run: git submodule update --init --depth 1 vscode-rojo - name: Setup Node.js uses: actions/setup-node@v4 @@ -76,58 +70,45 @@ jobs: msrv: name: Check MSRV runs-on: ubuntu-latest + env: + CARGO_INCREMENTAL: "0" steps: - uses: actions/checkout@v4 - with: - submodules: true + + - name: Init rbx-dom submodule + run: git submodule update --init --depth 1 rbx-dom - name: Install Rust uses: dtolnay/rust-toolchain@1.88.0 - - name: Restore Rust Cache - uses: actions/cache/restore@v4 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + - name: Rust Cache + uses: Swatinem/rust-cache@v2 - name: Build - run: cargo build --locked --verbose - - - name: Save Rust Cache - uses: actions/cache/save@v4 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + env: + CARGO_TERM_VERBOSE: ${{ runner.debug && 'true' || 'false' }} + run: cargo build --locked lint: name: Rustfmt, Clippy, Stylua, & Selene runs-on: ubuntu-latest + env: + CARGO_INCREMENTAL: "0" steps: - uses: actions/checkout@v4 - with: - submodules: true + + - name: Init rbx-dom submodule + run: git submodule update --init --depth 1 rbx-dom - name: Install Rust uses: dtolnay/rust-toolchain@stable with: components: rustfmt, clippy - - name: Restore Rust Cache - uses: actions/cache/restore@v4 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + - name: Rust Cache + uses: Swatinem/rust-cache@v2 - name: Setup Rokit uses: CompeyDev/setup-rokit@v0.1.2 @@ -144,13 +125,4 @@ jobs: run: cargo fmt -- --check - name: Clippy - run: cargo clippy - - - name: Save Rust Cache - uses: actions/cache/save@v4 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + run: cargo clippy -- -D warnings diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 447eae0e..30f71d5e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -20,6 +20,8 @@ jobs: needs: ["create-release"] name: Build Roblox Studio Plugin runs-on: ubuntu-latest + env: + CARGO_INCREMENTAL: "0" steps: - uses: actions/checkout@v4 with: @@ -28,6 +30,9 @@ jobs: - name: Install Rust uses: dtolnay/rust-toolchain@stable + - name: Rust Cache + uses: Swatinem/rust-cache@v2 + - name: Install Rokit uses: CompeyDev/setup-rokit@v0.1.2 @@ -62,8 +67,9 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - with: - submodules: true + + - name: Init vscode-rojo submodule + run: git submodule update --init --depth 1 vscode-rojo - name: Setup Node.js uses: actions/setup-node@v4 @@ -134,36 +140,33 @@ jobs: runs-on: ${{ matrix.os }} env: BIN: atlas + CARGO_INCREMENTAL: "0" steps: + - name: Disable Windows Defender + if: runner.os == 'Windows' + run: Set-MpPreference -DisableRealtimeMonitoring $true + - uses: actions/checkout@v4 - with: - submodules: true + + - name: Init rbx-dom submodule + env: + GIT_CONFIG_NOSYSTEM: "1" + run: git submodule update --init --depth 1 rbx-dom - name: Install Rust uses: dtolnay/rust-toolchain@stable with: targets: ${{ matrix.target }} - - name: Restore Rust Cache - uses: actions/cache/restore@v4 + - name: Rust Cache + uses: Swatinem/rust-cache@v2 with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ matrix.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + shared-key: release-${{ matrix.target }} - name: Build Release - run: cargo build --release --locked --verbose --target ${{ matrix.target }} - - - name: Save Rust Cache - uses: actions/cache/save@v4 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ matrix.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + env: + CARGO_TERM_VERBOSE: ${{ runner.debug && 'true' || 'false' }} + run: cargo build --release --locked --target ${{ matrix.target }} - name: Generate Artifact Name shell: bash diff --git a/CHANGELOG.md b/CHANGELOG.md index 020171c6..a7176b75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,10 @@ Making a new release? Simply add the new header with the version and date undern ## Unreleased +* Optimize serve/sourcemap startup performance: replace kqueue with FSEvents on macOS, lazy watch setup, build RefPathIndex from patch entries instead of directory walk +* Use platform-specific file watcher: raw notify + custom debounce on macOS, debouncer-full on Linux/Windows +* Use oneshot session for non-watch sourcemap generation + ## [8.5.6] (March 8th, 2026) * Add asynchronous log file compression with lowered thread priority to avoid impacting main workloads diff --git a/crates/memofs/Cargo.toml b/crates/memofs/Cargo.toml index 1eb76ec5..cfc5bbe4 100644 --- a/crates/memofs/Cargo.toml +++ b/crates/memofs/Cargo.toml @@ -18,14 +18,11 @@ homepage = "https://github.com/UserGeneratedLLC/rojo/tree/master/memofs" crossbeam-channel = "0.5.15" fs-err = "3" log = "0.4" -notify-debouncer-full = "0.7" +notify = "8.2" serde = { version = "1.0.228", features = ["derive"] } -[target.'cfg(target_os = "macos")'.dependencies] -notify = { version = "8.2", features = ["macos_kqueue"] } - [target.'cfg(not(target_os = "macos"))'.dependencies] -notify = "8.2" +notify-debouncer-full = "0.7" [dev-dependencies] tempfile = "3.26.0" diff --git a/crates/memofs/src/lib.rs b/crates/memofs/src/lib.rs index c99acb1a..6a825de5 100644 --- a/crates/memofs/src/lib.rs +++ b/crates/memofs/src/lib.rs @@ -190,7 +190,7 @@ impl VfsInner { fn read_raw(&mut self, path: &Path) -> io::Result> { if let Some(cache) = &mut self.prefetch_cache { if let Some(contents) = cache.files.remove(path) { - if self.watch_enabled && self.watch_recursive { + if self.watch_enabled { self.watch_or_record(path)?; } return Ok(contents); @@ -199,7 +199,7 @@ impl VfsInner { let contents = self.backend.read(path)?; - if self.watch_enabled && self.watch_recursive { + if self.watch_enabled { self.watch_or_record(path)?; } diff --git a/crates/memofs/src/std_backend.rs b/crates/memofs/src/std_backend.rs index bf44ca28..71c3135d 100644 --- a/crates/memofs/src/std_backend.rs +++ b/crates/memofs/src/std_backend.rs @@ -1,14 +1,15 @@ use std::path::{Path, PathBuf}; -use std::time::Duration; use std::{collections::HashSet, io}; -use crossbeam_channel::{Receiver, Sender}; +use crossbeam_channel::Receiver; +use notify::event::{CreateKind, EventKind, ModifyKind, RemoveKind, RenameMode}; use notify::RecursiveMode; -use notify_debouncer_full::{ - new_debouncer, - notify::event::{CreateKind, EventKind, ModifyKind, RemoveKind, RenameMode}, - DebounceEventResult, Debouncer, RecommendedCache, -}; + +#[cfg(target_os = "macos")] +use {notify::Watcher, std::thread}; + +#[cfg(not(target_os = "macos"))] +use notify_debouncer_full::{new_debouncer, DebounceEventResult, Debouncer, RecommendedCache}; use crate::{DirEntry, Metadata, ReadDir, VfsBackend, VfsEvent}; @@ -55,12 +56,13 @@ pub type CriticalErrorHandler = Box bool + Send /// `VfsBackend` that uses `std::fs` and the `notify` crate. pub struct StdBackend { + #[cfg(target_os = "macos")] + watcher: notify::RecommendedWatcher, + #[cfg(not(target_os = "macos"))] debouncer: Debouncer, watcher_receiver: Receiver, watches: HashSet, recursive_watches: HashSet, - /// Receiver for critical errors from the watcher thread. - /// Callers should poll this to detect when watching becomes unreliable. critical_error_receiver: Receiver, } @@ -108,34 +110,84 @@ impl StdBackend { let (event_tx, event_rx) = crossbeam_channel::unbounded(); let (error_tx, error_rx) = crossbeam_channel::unbounded(); - let debouncer = Self::create_debouncer(event_tx, error_tx, error_handler); + #[cfg(target_os = "macos")] + let watcher = { + let (raw_tx, raw_rx) = + std::sync::mpsc::channel::>(); - Self { - debouncer, - watcher_receiver: event_rx, - watches: HashSet::new(), - recursive_watches: HashSet::new(), - critical_error_receiver: error_rx, - } - } + let w = notify::RecommendedWatcher::new( + move |result| { + let _ = raw_tx.send(result); + }, + notify::Config::default(), + ) + .expect("Failed to create file watcher"); - fn create_debouncer( - event_tx: Sender, - error_tx: Sender, - error_handler: CriticalErrorHandler, - ) -> Debouncer { - // Use 50ms debounce timeout (same as the old v4 implementation) - let debounce_timeout = Duration::from_millis(50); + let debounce_ms = std::time::Duration::from_millis(50); - new_debouncer( - debounce_timeout, - None, // Use default tick rate - move |result: DebounceEventResult| { - match result { + thread::spawn(move || { + use std::collections::HashMap; + use std::time::Instant; + + let mut pending: HashMap = HashMap::new(); + let mut last_event = Instant::now(); + + loop { + match raw_rx.recv_timeout(debounce_ms) { + Ok(Ok(event)) => { + last_event = Instant::now(); + for vfs_event in Self::convert_event(&event) { + let path = match &vfs_event { + VfsEvent::Create(p) + | VfsEvent::Write(p) + | VfsEvent::Remove(p) => p.clone(), + }; + pending.insert(path, vfs_event); + } + } + Ok(Err(error)) => { + let critical_err = WatcherCriticalError::WatcherError { + error: format!("{:?}", error.kind), + path: error.paths.first().cloned(), + }; + let _ = error_tx.send(critical_err.clone()); + if error_handler(critical_err) { + return; + } + } + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => {} + Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => return, + } + + if !pending.is_empty() && last_event.elapsed() >= debounce_ms { + for (_, vfs_event) in pending.drain() { + if let Err(err) = event_tx.send(vfs_event) { + let critical_err = + WatcherCriticalError::ChannelSendFailed(err.to_string()); + let _ = error_tx.send(critical_err.clone()); + if error_handler(critical_err) { + return; + } + } + } + } + } + }); + + w + }; + + #[cfg(not(target_os = "macos"))] + let debouncer = { + let debounce_timeout = std::time::Duration::from_millis(50); + + new_debouncer( + debounce_timeout, + None, + move |result: DebounceEventResult| match result { Ok(events) => { for event in events { - let vfs_events = Self::convert_event(&event.event); - for vfs_event in vfs_events { + for vfs_event in Self::convert_event(&event.event) { if let Err(err) = event_tx.send(vfs_event) { let critical_err = WatcherCriticalError::ChannelSendFailed(err.to_string()); @@ -149,37 +201,41 @@ impl StdBackend { } Err(errors) => { for error in errors { - // Check if this is a rescan request - if error.paths.is_empty() { - let critical_err = WatcherCriticalError::RescanRequired; - let _ = error_tx.send(critical_err.clone()); - if error_handler(critical_err) { - return; - } + let critical_err = if error.paths.is_empty() { + WatcherCriticalError::RescanRequired } else { - let critical_err = WatcherCriticalError::WatcherError { + WatcherCriticalError::WatcherError { error: format!("{:?}", error.kind), path: error.paths.first().cloned(), - }; - let _ = error_tx.send(critical_err.clone()); - if error_handler(critical_err) { - return; } + }; + let _ = error_tx.send(critical_err.clone()); + if error_handler(critical_err) { + return; } } } - } - }, - ) - .expect("Failed to create file watcher debouncer") + }, + ) + .expect("Failed to create file watcher debouncer") + }; + + Self { + #[cfg(target_os = "macos")] + watcher, + #[cfg(not(target_os = "macos"))] + debouncer, + watcher_receiver: event_rx, + watches: HashSet::new(), + recursive_watches: HashSet::new(), + critical_error_receiver: error_rx, + } } - /// Convert a notify event to our VfsEvent(s) fn convert_event(event: ¬ify::Event) -> Vec { let mut vfs_events = Vec::new(); match &event.kind { - // Create events EventKind::Create(CreateKind::File) | EventKind::Create(CreateKind::Folder) | EventKind::Create(CreateKind::Any) @@ -189,7 +245,6 @@ impl StdBackend { } } - // Modify events (treat as Write) EventKind::Modify(ModifyKind::Data(_)) | EventKind::Modify(ModifyKind::Any) | EventKind::Modify(ModifyKind::Other) => { @@ -198,38 +253,43 @@ impl StdBackend { } } - // Metadata changes - we don't care about these for Rojo's purposes + // On macOS, FSEvents collapses Create+Remove into a Metadata event + // when both occur in the same batch. Treating it as Write ensures the + // change processor re-snapshots and detects the removal. + // On other platforms, metadata-only changes (chmod, touch) are noise. + #[cfg(target_os = "macos")] + EventKind::Modify(ModifyKind::Metadata(_)) => { + for path in &event.paths { + vfs_events.push(VfsEvent::Write(path.clone())); + } + } + + #[cfg(not(target_os = "macos"))] EventKind::Modify(ModifyKind::Metadata(_)) => {} - // Name changes (renames) - the debouncer-full handles rename tracking EventKind::Modify(ModifyKind::Name(RenameMode::Both)) => { - // Both paths present: old path at [0], new path at [1] if event.paths.len() >= 2 { vfs_events.push(VfsEvent::Remove(event.paths[0].clone())); vfs_events.push(VfsEvent::Create(event.paths[1].clone())); } } EventKind::Modify(ModifyKind::Name(RenameMode::From)) => { - // Only the old path - treat as removal for path in &event.paths { vfs_events.push(VfsEvent::Remove(path.clone())); } } EventKind::Modify(ModifyKind::Name(RenameMode::To)) => { - // Only the new path - treat as creation for path in &event.paths { vfs_events.push(VfsEvent::Create(path.clone())); } } EventKind::Modify(ModifyKind::Name(RenameMode::Any)) | EventKind::Modify(ModifyKind::Name(RenameMode::Other)) => { - // Ambiguous rename - treat as modification for path in &event.paths { vfs_events.push(VfsEvent::Write(path.clone())); } } - // Remove events EventKind::Remove(RemoveKind::File) | EventKind::Remove(RemoveKind::Folder) | EventKind::Remove(RemoveKind::Any) @@ -239,10 +299,8 @@ impl StdBackend { } } - // Access events - we don't care about these EventKind::Access(_) => {} - // Other/Any events - treat as potential modifications EventKind::Other | EventKind::Any => { for path in &event.paths { vfs_events.push(VfsEvent::Write(path.clone())); @@ -250,6 +308,10 @@ impl StdBackend { } } + for event in &vfs_events { + log::trace!("VFS event: {:?}", event); + } + vfs_events } @@ -347,7 +409,11 @@ impl VfsBackend for StdBackend { } else { RecursiveMode::NonRecursive }; - match self.debouncer.watch(path, mode) { + #[cfg(target_os = "macos")] + let result = self.watcher.watch(path, mode); + #[cfg(not(target_os = "macos"))] + let result = self.debouncer.watch(path, mode); + match result { Ok(()) => { log::debug!( "Watching path ({}): {}", @@ -374,7 +440,11 @@ impl VfsBackend for StdBackend { fn unwatch(&mut self, path: &Path) -> io::Result<()> { let was_watched = self.watches.contains(path); - match self.debouncer.unwatch(path) { + #[cfg(target_os = "macos")] + let result = self.watcher.unwatch(path); + #[cfg(not(target_os = "macos"))] + let result = self.debouncer.unwatch(path); + match result { Ok(()) => { if was_watched { log::info!("Unwatched path: {}", path.display()); @@ -424,8 +494,16 @@ mod tests { use super::*; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; + use std::time::Duration; use tempfile::tempdir; + /// On macOS, `tempdir()` returns `/var/folders/...` but FSEvents reports + /// canonical paths (`/private/var/folders/...`). Canonicalize so path + /// comparisons in event assertions work on all platforms. + fn canonical_dir(dir: &tempfile::TempDir) -> PathBuf { + std::fs::canonicalize(dir.path()).unwrap_or_else(|_| dir.path().to_path_buf()) + } + #[test] fn watch_adds_to_watches_only_on_success() { let dir = tempdir().unwrap(); @@ -646,12 +724,13 @@ mod tests { fn stress_rapid_file_modifications() { // Simulates undo/redo scenario: rapid modifications to the same file let dir = tempdir().unwrap(); - let file_path = dir.path().join("rapid_mod.luau"); + let dir_path = canonical_dir(&dir); + let file_path = dir_path.join("rapid_mod.luau"); fs_err::write(&file_path, "-- initial").unwrap(); let mut backend = StdBackend::new_for_testing(); let event_rx = backend.event_receiver(); - assert!(backend.watch(dir.path(), true).is_ok()); + assert!(backend.watch(&dir_path, true).is_ok()); std::thread::sleep(Duration::from_millis(100)); // Rapid modifications - 20 writes in quick succession @@ -659,8 +738,8 @@ mod tests { fs_err::write(&file_path, format!("-- version {}", i)).unwrap(); } - // Wait for debounce to settle (50ms debounce + buffer) - let events = collect_events_with_timeout(&event_rx, Duration::from_millis(500)); + // FSEvents on macOS has higher baseline latency than kqueue/inotify + let events = collect_events_with_timeout(&event_rx, Duration::from_millis(1500)); // Should receive at least one Write event (debouncer coalesces rapid writes) let write_events: Vec<_> = events @@ -769,37 +848,41 @@ mod tests { fn stress_rename_operations() { // Rename is particularly tricky for filesystem watchers let dir = tempdir().unwrap(); - let original = dir.path().join("original.luau"); - let renamed = dir.path().join("renamed.luau"); + let dir_path = canonical_dir(&dir); + let original = dir_path.join("original.luau"); + let renamed = dir_path.join("renamed.luau"); fs_err::write(&original, "-- content").unwrap(); let mut backend = StdBackend::new_for_testing(); let event_rx = backend.event_receiver(); - assert!(backend.watch(dir.path(), true).is_ok()); + assert!(backend.watch(&dir_path, true).is_ok()); std::thread::sleep(Duration::from_millis(100)); // Rename the file fs_err::rename(&original, &renamed).unwrap(); - let events = collect_events_with_timeout(&event_rx, Duration::from_millis(300)); + let events = collect_events_with_timeout(&event_rx, Duration::from_millis(1500)); log::info!("Rename: {} events received", events.len()); // Should get either: // - Remove(original) + Create(renamed) for RenameMode::Both // - Or separate From/To events - let has_remove = events - .iter() - .any(|e| matches!(e, VfsEvent::Remove(p) if p == &original)); - let has_create = events - .iter() - .any(|e| matches!(e, VfsEvent::Create(p) if p == &renamed)); + // FSEvents on macOS may report renames as Write/Metadata events rather + // than the specific Create/Remove pair. Verify that at least one event + // references either the original or renamed path. + let has_relevant_event = events.iter().any(|e| match e { + VfsEvent::Create(p) | VfsEvent::Write(p) | VfsEvent::Remove(p) => { + p == &original || p == &renamed + } + }); - // At minimum we should see the new file appear assert!( - has_create || has_remove, - "Expected rename to generate events" + has_relevant_event, + "Expected rename to generate events for original or renamed path (got {} events: {:?})", + events.len(), + events ); } @@ -961,7 +1044,8 @@ mod tests { // Simulates the exact scenario that caused the original bug: // File modified, then quickly reverted (undo), then possibly redone let dir = tempdir().unwrap(); - let file_path = dir.path().join("script.luau"); + let dir_path = canonical_dir(&dir); + let file_path = dir_path.join("script.luau"); let original_content = "-- original content\nlocal x = 1"; let modified_content = "-- modified content\nlocal x = 2"; @@ -969,7 +1053,7 @@ mod tests { let mut backend = StdBackend::new_for_testing(); let event_rx = backend.event_receiver(); - assert!(backend.watch(dir.path(), true).is_ok()); + assert!(backend.watch(&dir_path, true).is_ok()); std::thread::sleep(Duration::from_millis(100)); // Simulate multiple undo/redo cycles @@ -991,7 +1075,7 @@ mod tests { std::thread::sleep(Duration::from_millis(20)); } - let events = collect_events_with_timeout(&event_rx, Duration::from_millis(500)); + let events = collect_events_with_timeout(&event_rx, Duration::from_millis(1500)); log::info!( "Undo/redo simulation: {} events from 40 file writes", diff --git a/scripts/ci.ps1 b/scripts/ci.ps1 index ba2199bb..2f593ff3 100644 --- a/scripts/ci.ps1 +++ b/scripts/ci.ps1 @@ -63,7 +63,7 @@ Record-Result "Selene" $selene Write-Step 6 "Rust Linting (Clippy) - Auto-fix" cargo clippy -j $BuildThreads --fix --allow-dirty --allow-staged 2>&1 | Out-Null Write-Host "Verifying..." -ForegroundColor Yellow -cargo clippy -j $BuildThreads --all-targets --all-features 2>&1 +cargo clippy -j $BuildThreads --all-targets --all-features -- -D warnings 2>&1 $clippy = $LASTEXITCODE Record-Result "Clippy" $clippy diff --git a/scripts/ci.sh b/scripts/ci.sh index 2518d166..d67aee25 100755 --- a/scripts/ci.sh +++ b/scripts/ci.sh @@ -64,7 +64,7 @@ record "Selene" $selene_exit step 6 "Rust Linting (Clippy) - Auto-fix" cargo clippy -j $BUILD_THREADS --fix --allow-dirty --allow-staged >/dev/null 2>&1 echo "Verifying..." -cargo clippy -j $BUILD_THREADS --all-targets --all-features 2>&1 +cargo clippy -j $BUILD_THREADS --all-targets --all-features -- -D warnings 2>&1 clippy_exit=$? record "Clippy" $clippy_exit diff --git a/src/cli/sourcemap.rs b/src/cli/sourcemap.rs index 45db803c..a5588963 100644 --- a/src/cli/sourcemap.rs +++ b/src/cli/sourcemap.rs @@ -79,7 +79,17 @@ impl SourcemapCommand { let vfs = Vfs::new_default(); vfs.set_watch_enabled(self.watch); - let session = ServeSession::new(vfs, project_path, None)?; + let session_start = std::time::Instant::now(); + let session = if self.watch { + ServeSession::new(vfs, project_path, None)? + } else { + ServeSession::new_oneshot(vfs, project_path)? + }; + log::debug!( + "[PERF] ServeSession::new() total: {:.1?}", + session_start.elapsed() + ); + let mut cursor = session.message_queue().cursor(); let filter = if self.include_non_scripts { @@ -88,13 +98,12 @@ impl SourcemapCommand { filter_non_scripts }; - // Pre-build a rayon threadpool with a low number of threads to avoid - // dynamic creation overhead on systems with a high number of cpus. rayon::ThreadPoolBuilder::new() .num_threads(num_cpus::get().min(6)) .build_global() .ok(); + let sm_start = std::time::Instant::now(); write_sourcemap( &session, self.output.as_deref(), @@ -102,6 +111,7 @@ impl SourcemapCommand { self.absolute, false, )?; + log::debug!("[PERF] write_sourcemap: {:.1?}", sm_start.elapsed()); if self.watch { if !std::io::stdin().is_terminal() { @@ -261,6 +271,7 @@ pub(crate) fn write_sourcemap( use_absolute_paths: bool, quiet: bool, ) -> anyhow::Result<()> { + let t0 = std::time::Instant::now(); let tree = session.tree(); let canonical_project_dir = session.root_dir().to_path_buf(); @@ -272,22 +283,32 @@ pub(crate) fn write_sourcemap( filter, use_absolute_paths, ); + let t1 = std::time::Instant::now(); if let Some(output_path) = output { - // Use standard JSON (not JSON5) for sourcemaps - required by external tools like LSPs let json_output = serde_json::to_string(&root_node)?; + let t2 = std::time::Instant::now(); - // Use atomic write (temp file + rename) to prevent file watchers from - // reading partial files. Rename is atomic on all major filesystems when - // source and destination are on the same filesystem. write_atomic(output_path, json_output.as_bytes())?; + let t3 = std::time::Instant::now(); + + log::debug!( + "[PERF] write_sourcemap: tree_walk={:.1?}, json_serialize={:.1?} ({} bytes), write={:.1?}", + t1 - t0, + t2 - t1, + json_output.len(), + t3 - t2, + ); if !quiet { println!("Created sourcemap at {}", output_path.display()); } } else { - // Use standard JSON (not JSON5) for sourcemaps - required by external tools like LSPs let output = serde_json::to_string(&root_node)?; + log::debug!( + "[PERF] write_sourcemap: tree_walk={:.1?}, json_serialize=inline", + t1 - t0 + ); println!("{}", output); } diff --git a/src/git.rs b/src/git.rs index ce8e4351..8e77a596 100644 --- a/src/git.rs +++ b/src/git.rs @@ -306,6 +306,10 @@ impl GitIndexCache { self.entries.len() } + pub fn is_empty(&self) -> bool { + self.entries.is_empty() + } + /// Returns `true` if the new content's blob SHA1 matches the git index /// entry for this path, meaning the file on disk almost certainly /// already has this content. diff --git a/src/hungarian.rs b/src/hungarian.rs index fc0b29f5..30eb1020 100644 --- a/src/hungarian.rs +++ b/src/hungarian.rs @@ -94,9 +94,9 @@ pub fn min_cost_assignment( } let mut result = Vec::with_capacity(rows.min(cols)); - for j in 1..=n { - if p[j] != 0 && p[j] <= rows && j <= cols { - result.push((p[j] - 1, j - 1)); + for (j, &assigned_row) in p.iter().enumerate().skip(1) { + if assigned_row != 0 && assigned_row <= rows && j <= cols { + result.push((assigned_row - 1, j - 1)); } } result diff --git a/src/rojo_ref.rs b/src/rojo_ref.rs index dc6ac271..76ca01b8 100644 --- a/src/rojo_ref.rs +++ b/src/rojo_ref.rs @@ -12,6 +12,14 @@ use rbx_dom_weak::{ }; use serde::{Deserialize, Serialize}; +/// Returns `true` if the filename is a meta or model JSON file. +pub(crate) fn is_meta_or_model_name(name: &str) -> bool { + name.ends_with(".meta.json5") + || name.ends_with(".model.json5") + || name.ends_with(".meta.json") + || name.ends_with(".model.json") +} + /// Legacy: ID attribute on target instances (kept for backwards compatibility) pub const REF_ID_ATTRIBUTE_NAME: &str = "Rojo_Id"; /// Legacy: Pointer attribute prefix using IDs (kept for backwards compatibility) @@ -326,6 +334,22 @@ impl RefPathIndex { } } + /// Build from pre-collected entries (avoids re-walking the directory tree). + pub fn from_entries(entries: Vec<(String, PathBuf)>) -> Self { + let mut index = Self::new(); + let count = entries.len(); + for (ref_path, meta_file) in entries { + index.add(&ref_path, &meta_file); + } + if count > 0 { + log::debug!( + "RefPathIndex: built from {} collected Rojo_Ref_* entries", + count + ); + } + index + } + /// Record that `meta_file` contains a `Rojo_Ref_*` attribute with value /// `ref_path` (the instance path string, e.g., "Workspace/Model/Part1"). pub fn add(&mut self, ref_path: &str, meta_file: &Path) { @@ -373,19 +397,12 @@ impl RefPathIndex { use rayon::prelude::*; use walkdir::WalkDir; - fn is_meta_or_model(name: &str) -> bool { - name.ends_with(".meta.json5") - || name.ends_with(".model.json5") - || name.ends_with(".meta.json") - || name.ends_with(".model.json") - } - let meta_paths: Vec = WalkDir::new(root) .follow_links(true) .into_iter() .filter_map(|e: Result| e.ok()) .filter(|e: &walkdir::DirEntry| { - e.file_type().is_file() && e.file_name().to_str().is_some_and(is_meta_or_model) + e.file_type().is_file() && e.file_name().to_str().is_some_and(is_meta_or_model_name) }) .map(|e: walkdir::DirEntry| e.into_path()) .collect(); diff --git a/src/serve_session.rs b/src/serve_session.rs index 50212629..3f7bb7c7 100644 --- a/src/serve_session.rs +++ b/src/serve_session.rs @@ -17,7 +17,7 @@ use crate::{ session_id::SessionId, snapshot::{ apply_patch_set, compute_patch_set, AppliedPatchSet, InstanceContext, InstanceSnapshot, - PatchSet, PathIgnoreRule, RojoTree, + PatchSet, RojoTree, }, snapshot_middleware::{is_script_relevant_path, snapshot_from_vfs, INIT_FILE_PRIORITY}, }; @@ -326,11 +326,21 @@ pub fn collect_path_roots(node: &crate::project::ProjectNode, base: &Path, out: impl ServeSession { /// Shared initialization: loads the project and builds the initial /// snapshot tree. Used by both `new()` and `new_oneshot()`. - /// Returns the walked paths from prefetch (if available) for syncback reuse. + /// Returns the walked paths from prefetch (if available) for syncback reuse, + /// plus collected ref path index entries for building RefPathIndex. + #[allow(clippy::type_complexity)] fn init_tree( vfs: &Vfs, start_path: &Path, - ) -> Result<(Project, RojoTree, Option>), ServeSessionError> { + ) -> Result< + ( + Project, + RojoTree, + Option>, + Vec<(String, std::path::PathBuf)>, + ), + ServeSessionError, + > { log::trace!("Starting new ServeSession at path {}", start_path.display()); let root_project = Project::load_initial_project(vfs, start_path)?; @@ -403,87 +413,23 @@ impl ServeSession { let mut instance_context = InstanceContext::new(); instance_context.sync_scripts_only = sync_scripts_only; - let mut path_roots = Vec::new(); - collect_path_roots( - &root_project.tree, - root_project.folder_location(), - &mut path_roots, - ); - let snap_start = Instant::now(); log::trace!("Generating snapshot of instances from VFS"); - let was_watch_enabled = vfs.is_watch_enabled(); - vfs.set_watch_enabled(false); let snapshot = snapshot_from_vfs(&instance_context, vfs, start_path)?; - vfs.set_watch_enabled(was_watch_enabled); log::debug!("Snapshot built in {:.1?}", snap_start.elapsed()); vfs.clear_prefetch_cache(); let patch_start = Instant::now(); - let watch_ignore_rules = root_project.path_ignore_rules(); - std::thread::scope(|s| { - s.spawn(|| { - use walkdir::WalkDir; + log::trace!("Computing initial patch set"); + let patch_set = compute_patch_set(snapshot, &tree, root_id); - let watch_start = Instant::now(); - let mut watch_count: usize = 0; + log::trace!("Applying initial patch set"); + let applied = apply_patch_set(&mut tree, patch_set); + let ref_path_entries = applied.ref_path_index_entries; + log::debug!("Patch computed + applied in {:.1?}", patch_start.elapsed()); - if cfg!(target_os = "macos") { - // macOS kqueue: one FD per watched path. Walk each $path - // root, skip globIgnorePaths subtrees, and watch every - // non-ignored file and directory non-recursively. File - // watches are required because kqueue directory watches - // only fire for structural changes, not child content edits. - let passes_ignore = |entry: &walkdir::DirEntry| -> bool { - if let Some(name) = entry.file_name().to_str() { - if matches!(name, ".git" | ".hg" | ".svn") { - return false; - } - } - watch_ignore_rules - .iter() - .all(|rule: &PathIgnoreRule| rule.passes(entry.path())) - }; - vfs.set_watch_recursive(false); - for root in &path_roots { - for entry in WalkDir::new(root) - .into_iter() - .filter_entry(&passes_ignore) - .flatten() - { - let _ = vfs.watch(entry.path()); - watch_count += 1; - } - } - vfs.set_watch_recursive(true); - } else { - // Linux/Windows: inotify uses a single FD for all watches, - // RDCW uses one handle per directory. Recursive watches are - // cheap and directory watches already cover child file - // events. globIgnorePaths filtering is handled by the - // change processor's event filter. - for root in &path_roots { - let _ = vfs.watch(root); - watch_count += 1; - } - } - log::debug!( - "Set up {} watches in {:.1?}", - watch_count, - watch_start.elapsed(), - ); - }); - - log::trace!("Computing initial patch set"); - let patch_set = compute_patch_set(snapshot, &tree, root_id); - - log::trace!("Applying initial patch set"); - apply_patch_set(&mut tree, patch_set); - }); - log::debug!("Watch + patch in {:.1?}", patch_start.elapsed()); - - Ok((root_project, tree, walked_paths)) + Ok((root_project, tree, walked_paths, ref_path_entries)) } /// Start a new serve session from the given in-memory filesystem and start @@ -500,7 +446,10 @@ impl ServeSession { let start_path = start_path.as_ref(); let start_time = Instant::now(); - let (root_project, tree, _walked_paths) = Self::init_tree(&vfs, start_path)?; + let t_init_start = Instant::now(); + let (root_project, tree, _walked_paths, ref_path_entries) = + Self::init_tree(&vfs, start_path)?; + let t_init_tree = Instant::now(); let session_id = SessionId::new(); let message_queue = MessageQueue::new(); @@ -511,29 +460,26 @@ impl ServeSession { let (tree_mutation_sender, tree_mutation_receiver) = crossbeam_channel::unbounded(); let suppressed_paths = Arc::new(Mutex::new(std::collections::HashMap::new())); - let ref_path_index = { - let mut index = crate::RefPathIndex::new(); - let tree_guard = tree.lock().unwrap(); - let mut ref_roots = Vec::new(); - collect_path_roots( - &root_project.tree, - root_project.folder_location(), - &mut ref_roots, - ); - for root in &ref_roots { - index.populate_from_dir(root, &tree_guard); - } - drop(tree_guard); - Arc::new(Mutex::new(index)) - }; + let ref_path_index = Arc::new(Mutex::new(crate::RefPathIndex::from_entries( + ref_path_entries, + ))); + let t_ref_index = Instant::now(); let git_repo_root = crate::git::git_repo_root(root_project.folder_location()); let initial_head_commit = git_repo_root .as_deref() .and_then(crate::git::git_head_commit); + let t_git = Instant::now(); let path_ignore_rules = root_project.path_ignore_rules(); + log::debug!( + "[PERF] ServeSession::new breakdown: init_tree={:.1?}, ref_path_index={:.1?}, git={:.1?}", + t_init_tree - t_init_start, + t_ref_index - t_init_tree, + t_git - t_ref_index, + ); + log::trace!("Starting ChangeProcessor"); let change_processor = ChangeProcessor::start( Arc::clone(&tree), @@ -576,7 +522,7 @@ impl ServeSession { let start_path = start_path.as_ref(); let start_time = Instant::now(); - let (root_project, tree, walked_paths) = Self::init_tree(&vfs, start_path)?; + let (root_project, tree, walked_paths, _ref_entries) = Self::init_tree(&vfs, start_path)?; Ok(Self { change_processor: None, diff --git a/src/snapshot/patch.rs b/src/snapshot/patch.rs index 235ed12d..d3551e42 100644 --- a/src/snapshot/patch.rs +++ b/src/snapshot/patch.rs @@ -1,6 +1,6 @@ //! Defines the data structures used for describing instance patches. -use std::collections::HashSet; +use std::{collections::HashSet, path::PathBuf}; use rbx_dom_weak::{ types::{Ref, Variant}, @@ -81,6 +81,11 @@ pub struct AppliedPatchSet { pub removed: Vec, pub added: Vec, pub updated: Vec, + + /// Collected during patch application: `(resolved_absolute_ref_path, meta_or_model_file_path)`. + /// Used to build `RefPathIndex` without re-walking the directory tree. + #[serde(skip)] + pub ref_path_index_entries: Vec<(String, PathBuf)>, } impl AppliedPatchSet { @@ -89,6 +94,7 @@ impl AppliedPatchSet { removed: Vec::new(), added: Vec::new(), updated: Vec::new(), + ref_path_index_entries: Vec::new(), } } diff --git a/src/snapshot/patch_apply.rs b/src/snapshot/patch_apply.rs index 150bfd6d..618864b0 100644 --- a/src/snapshot/patch_apply.rs +++ b/src/snapshot/patch_apply.rs @@ -3,6 +3,8 @@ use std::{ collections::{HashMap, HashSet}, mem::take, + path::PathBuf, + time::Instant, }; use rbx_dom_weak::{ @@ -24,14 +26,20 @@ use crate::{ /// tree in sync with Rojo's. #[profiling::function] pub fn apply_patch_set(tree: &mut RojoTree, patch_set: PatchSet) -> AppliedPatchSet { + let t0 = Instant::now(); let mut context = PatchApplyContext::default(); + let removal_count = patch_set.removed_instances.len(); + let addition_count = patch_set.added_instances.len(); + let update_count = patch_set.updated_instances.len(); + { profiling::scope!("removals"); for removed_id in patch_set.removed_instances { apply_remove_instance(&mut context, tree, removed_id); } } + let t1 = Instant::now(); { profiling::scope!("additions"); @@ -39,15 +47,30 @@ pub fn apply_patch_set(tree: &mut RojoTree, patch_set: PatchSet) -> AppliedPatch apply_add_child(&mut context, tree, add_patch.parent_id, add_patch.instance); } } + let t2 = Instant::now(); { profiling::scope!("updates"); - // Updates need to be applied after additions, which reduces the complexity - // of updates significantly. for update_patch in patch_set.updated_instances { apply_update_child(&mut context, tree, update_patch); } } + let t3 = Instant::now(); + + log::debug!( + "[PERF] patch_apply phases: removals={} in {:.1?}, additions={} in {:.1?}, updates={} in {:.1?}", + removal_count, t1 - t0, + addition_count, t2 - t1, + update_count, t3 - t2, + ); + log::debug!( + "[PERF] patch_apply context: snapshot_id_map={}, has_refs={}, attr_refs={}, path_refs={}, cleanup={}", + context.snapshot_id_to_instance_id.len(), + context.has_refs_to_rewrite.len(), + context.attribute_refs_to_rewrite.keys().count(), + context.path_refs_to_rewrite.keys().count(), + context.instances_needing_attr_cleanup.len(), + ); finalize_patch_application(context, tree) } @@ -92,6 +115,10 @@ struct PatchApplyContext { /// format and should not persist in the live game. instances_needing_attr_cleanup: HashSet, + /// Collected `(resolved_absolute_ref_path, meta_or_model_file_path)` entries + /// for building `RefPathIndex` without a redundant directory walk. + ref_path_index_entries: Vec<(String, PathBuf)>, + /// The current applied patch result, describing changes made to the tree. applied_patch_set: AppliedPatchSet, } @@ -107,10 +134,13 @@ struct PatchApplyContext { /// where we build up a map of snapshot IDs to instance IDs as they're created, /// then apply properties all at once at the end. #[profiling::function] -fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) -> AppliedPatchSet { +fn finalize_patch_application( + mut context: PatchApplyContext, + tree: &mut RojoTree, +) -> AppliedPatchSet { + let finalize_start = Instant::now(); + for id in context.has_refs_to_rewrite { - // This should always succeed since instances marked as added in our - // patch should be added without fail. let mut instance = tree .get_instance_mut(id) .expect("Invalid instance ID in deferred property map"); @@ -123,11 +153,13 @@ fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) - } } } + let t_refs_rewrite = Instant::now(); // IMPORTANT: Rojo_Target_* (legacy) must be resolved BEFORE Rojo_Ref_* // (path-based) so that path-based results overwrite legacy results when // both exist for the same property. Do not reorder these two blocks. let mut real_rewrites = Vec::new(); + let attr_ref_count = context.attribute_refs_to_rewrite.keys().count(); for (id, map) in context.attribute_refs_to_rewrite { for (prop_name, prop_value) in map { if let Some(target) = tree.get_specified_id(&RojoRef::new(prop_value)) { @@ -139,11 +171,28 @@ fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) - .expect("Invalid instance ID in deferred attribute ref map"); instance.properties_mut().extend(real_rewrites.drain(..)); } + let t_attr_refs = Instant::now(); // Handle path-based references (Rojo_Ref_) let mut path_rewrites = Vec::new(); + let mut path_ref_total = 0usize; + let mut path_ref_resolved = 0usize; + let mut path_ref_failed = 0usize; for (id, map) in context.path_refs_to_rewrite { + let source_abs = crate::ref_target_path_from_tree(tree, id); + let meta_file = find_meta_or_model_path(tree, id); + for (prop_name, path) in map { + path_ref_total += 1; + + if let Some(meta_path) = &meta_file { + let resolved = crate::resolve_ref_path_to_absolute(&path, &source_abs) + .unwrap_or_else(|| path.clone()); + context + .ref_path_index_entries + .push((resolved, meta_path.clone())); + } + if let Some(target) = tree.resolve_ref_path(&path, id) { log::trace!( "Resolved path reference {} -> {} (path: {})", @@ -151,8 +200,10 @@ fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) - target, path ); + path_ref_resolved += 1; path_rewrites.push((prop_name, Variant::Ref(target))); } else { + path_ref_failed += 1; log::debug!( "Could not resolve path reference {} at path '{}' - instance not found in tree", prop_name, @@ -167,9 +218,11 @@ fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) - instance.properties_mut().extend(path_rewrites.drain(..)); } } + let t_path_refs = Instant::now(); // Clean up Rojo ref attributes - they're only used for transport in the file // format and should not persist in the live game + let cleanup_count = context.instances_needing_attr_cleanup.len(); for id in context.instances_needing_attr_cleanup { let mut instance = match tree.get_instance_mut(id) { Some(inst) => inst, @@ -181,7 +234,6 @@ fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) - _ => continue, }; - // Remove all Rojo ref-related attributes let keys_to_remove: Vec = attributes .iter() .filter_map(|(name, _)| { @@ -200,8 +252,25 @@ fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) - attributes.remove(key); } } + let t_cleanup = Instant::now(); - context.applied_patch_set + log::debug!( + "[PERF] finalize_patch_application: refs_rewrite={:.1?}, attr_refs({})={:.1?}, path_refs({}, ok={}, fail={})={:.1?}, cleanup({})={:.1?}, total={:.1?}", + t_refs_rewrite - finalize_start, + attr_ref_count, + t_attr_refs - t_refs_rewrite, + path_ref_total, + path_ref_resolved, + path_ref_failed, + t_path_refs - t_attr_refs, + cleanup_count, + t_cleanup - t_path_refs, + t_cleanup - finalize_start, + ); + + let mut result = context.applied_patch_set; + result.ref_path_index_entries = context.ref_path_index_entries; + result } fn apply_remove_instance(context: &mut PatchApplyContext, tree: &mut RojoTree, removed_id: Ref) { @@ -324,6 +393,20 @@ fn apply_update_child(context: &mut PatchApplyContext, tree: &mut RojoTree, patc context.applied_patch_set.updated.push(applied_patch) } +fn find_meta_or_model_path(tree: &RojoTree, id: Ref) -> Option { + let instance = tree.get_instance(id)?; + instance + .metadata() + .relevant_paths + .iter() + .find(|p| { + p.file_name() + .and_then(|n| n.to_str()) + .is_some_and(crate::is_meta_or_model_name) + }) + .cloned() +} + /// Calculates manually-specified Ref properties and marks them in the provided /// `PatchApplyContext` to be rewritten at the end of the patch application /// process. diff --git a/src/snapshot/tree.rs b/src/snapshot/tree.rs index 39716437..57dd451a 100644 --- a/src/snapshot/tree.rs +++ b/src/snapshot/tree.rs @@ -125,28 +125,25 @@ impl RojoTree { // // We need to insert the NeedsPivotMigration property with a value of false on // every instance that inherits from Model for pivots to build correctly. - let hack_needs_pivot_migration = match snapshot.class_name.as_ref() { - // This is not a future proof way to do this but the last time a - // descendant of Model was added was in 2020 so it's probably fine. + let needs_pivot = match snapshot.class_name.as_ref() { "Model" | "Actor" | "Tool" | "HopperBin" | "Flag" | "WorldModel" | "Workspace" - | "Status" - if !snapshot - .properties - .contains_key(&ustr("NeedsPivotMigration")) => - { - vec![("NeedsPivotMigration", Variant::Bool(false))] - } - _ => Vec::new(), + | "Status" => !snapshot + .properties + .contains_key(&ustr("NeedsPivotMigration")), + _ => false, }; let is_script = is_script_class(snapshot.class_name.as_ref()); - let builder = InstanceBuilder::empty() + let mut builder = InstanceBuilder::empty() .with_class(snapshot.class_name) .with_name(snapshot.name.into_owned()) - .with_properties(hack_needs_pivot_migration) .with_properties(snapshot.properties); + if needs_pivot { + builder = builder.with_property("NeedsPivotMigration", Variant::Bool(false)); + } + let referent = self.inner.insert(parent_ref, builder); self.insert_metadata(referent, snapshot.metadata);