diff --git a/src/json.rs b/src/json.rs index 3abeccfd..98c7e012 100644 --- a/src/json.rs +++ b/src/json.rs @@ -187,6 +187,23 @@ enum Json5Value { } impl Json5Value { + /// Rough byte-size estimate for pre-allocating the output string. + fn size_hint(&self) -> usize { + match self { + Json5Value::Null => 4, + Json5Value::Bool(_) => 5, + Json5Value::Number(s) => s.len(), + Json5Value::String(s) => s.len() + 2, + Json5Value::Array(items) => 8 + items.iter().map(|v| v.size_hint() + 6).sum::(), + Json5Value::Object(map) => { + 8 + map + .iter() + .map(|(k, v)| k.len() + v.size_hint() + 10) + .sum::() + } + } + } + /// Write this value to a string with proper JSON5 formatting and indentation. fn write_to(&self, output: &mut String, indent: usize) { let indent_str = " ".repeat(indent); @@ -682,8 +699,7 @@ pub fn to_vec_pretty_sorted(value: &T) -> anyhow::Result> .serialize(Json5ValueSerializer) .map_err(|e| anyhow::anyhow!("Failed to serialize: {}", e))?; - // Write the tree to string with pretty formatting - let mut output = String::new(); + let mut output = String::with_capacity(tree.size_hint()); tree.write_to(&mut output, 0); output.push('\n'); Ok(output.into_bytes()) diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index 51ee8e62..ff7fc380 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -13,7 +13,7 @@ use crate::{ json, resolution::UnresolvedValue, snapshot::{InstanceContext, InstanceSnapshot}, - syncback::{filter_properties_preallocated, FsSnapshot, SyncbackReturn, SyncbackSnapshot}, + syncback::{FsSnapshot, SyncbackReturn, SyncbackSnapshot}, RojoRef, }; @@ -126,7 +126,7 @@ fn json_model_from_pair<'sync>( .get_new_instance(new) .expect("all new referents passed to json_model_from_pair should exist"); - filter_properties_preallocated(snapshot.project(), new_inst, prop_buffer); + snapshot.filter_properties_cached(new_inst, prop_buffer); let mut properties = IndexMap::new(); let mut attributes = IndexMap::new(); diff --git a/src/syncback/fs_snapshot.rs b/src/syncback/fs_snapshot.rs index daa1524d..3eb518dd 100644 --- a/src/syncback/fs_snapshot.rs +++ b/src/syncback/fs_snapshot.rs @@ -338,7 +338,8 @@ impl FsSnapshot { let full_path = base_path.join(path); if let Some(cache) = git_cache { - if cache.file_matches_index(path, contents) { + let rel = path.strip_prefix(base_path).unwrap_or(path); + if cache.file_matches_index(rel, contents) { skipped_files.fetch_add(1, Ordering::Relaxed); git_skipped.fetch_add(1, Ordering::Relaxed); return; diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index 3cec1484..189c7ff9 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -41,6 +41,7 @@ pub use fs_snapshot::FsSnapshot; pub use hash::*; pub use property_filter::{ filter_properties, filter_properties_preallocated, should_property_serialize, + PropertyFilterCache, }; pub use snapshot::{inst_path, SyncbackData, SyncbackSnapshot}; pub use stats::SyncbackStats; @@ -387,6 +388,15 @@ pub fn syncback_loop_with_stats( if !dir.is_dir() { continue; } + // If the pre-walked set already contains paths under this dir, + // the prefetch covered it — just insert the dir itself. + let already_covered = filtered.iter().any(|p| p.starts_with(dir)); + if already_covered { + if is_valid_path(&ignore_patterns, project_path, dir) { + filtered.insert(dir.clone()); + } + continue; + } for entry in walkdir::WalkDir::new(dir) .follow_links(true) .into_iter() @@ -508,7 +518,8 @@ pub fn syncback_loop_with_stats( ); let phase_timer = std::time::Instant::now(); - let ref_path_map = std::cell::RefCell::new(HashMap::new()); + let ref_path_map = std::sync::Mutex::new(HashMap::new()); + let prop_filter_cache = std::sync::Mutex::new(PropertyFilterCache::new(project)); let syncback_data = SyncbackData { vfs, old_tree, @@ -517,6 +528,7 @@ pub fn syncback_loop_with_stats( incremental, stats, ref_path_map: &ref_path_map, + prop_filter_cache: &prop_filter_cache, }; // Always start with old reference for the Project middleware. @@ -533,172 +545,162 @@ pub fn syncback_loop_with_stats( let mut fs_snapshot = FsSnapshot::new(); let mut instance_paths: HashMap> = HashMap::new(); let mut walk_count: usize = 0; - let mut walk_syncback_ns: u128 = 0; - let mut walk_merge_ns: u128 = 0; - let mut walk_path_ns: u128 = 0; - 'syncback: while let Some(snapshot) = snapshots.pop() { - walk_count += 1; - let path_start = std::time::Instant::now(); - let inst_path = snapshot.get_new_inst_path(snapshot.new); - walk_path_ns += path_start.elapsed().as_nanos(); - // In incremental mode, we can quickly check that two subtrees are identical - // and if they are, skip reconciling them. In clean mode, we always process - // all instances to ensure fresh structure. - if incremental { - if let Some(old_ref) = snapshot.old { - match (old_hashes.get(&old_ref), new_hashes.get(&snapshot.new)) { - (Some(old), Some(new)) => { - if old == new { - log::trace!( - "Skipping {inst_path} due to it being identically hashed as {old:?}" - ); + use rayon::prelude::*; + + struct WaveItem<'sync> { + snapshot: SyncbackSnapshot<'sync>, + middleware: Middleware, + } + + while !snapshots.is_empty() { + // Phase 1: Sequential pre-filter to build this wave's work items. + let mut wave: Vec = Vec::with_capacity(snapshots.len()); + let mut next_snapshots: Vec = Vec::new(); + + 'filter: for snapshot in snapshots.drain(..) { + walk_count += 1; + + if incremental { + if let Some(old_ref) = snapshot.old { + match (old_hashes.get(&old_ref), new_hashes.get(&snapshot.new)) { + (Some(old), Some(new)) if old == new => continue, + _ => {} + } + } + } + + if !is_valid_path(&ignore_patterns, project_path, &snapshot.path) { + continue; + } + if let Some(ref globs) = tree_globs { + let inst_path = snapshot.get_new_inst_path(snapshot.new); + for (glob, _pattern) in globs { + if glob.is_match(&inst_path) { + continue 'filter; + } + } + } + + let middleware = get_best_middleware(&snapshot); + + if matches!(middleware, Middleware::Json | Middleware::Toml) { + continue; + } + + wave.push(WaveItem { + snapshot, + middleware, + }); + } + + if wave.is_empty() { + break; + } + + // Phase 2: Parallel middleware execution. + let results: Vec<_> = wave + .into_par_iter() + .map(|item| { + let WaveItem { + snapshot, + middleware, + } = item; + let result = match middleware.syncback(&snapshot) { + Ok(syncback) => Ok(syncback), + Err(err) if middleware == Middleware::Dir => { + let new_middleware = match env::var(DEBUG_MODEL_FORMAT_VAR) { + Ok(value) if value == "1" => Middleware::Rbxmx, + Ok(value) if value == "2" => Middleware::JsonModel, + _ => Middleware::Rbxm, + }; + let file_name = snapshot + .path + .file_name() + .and_then(|s| s.to_str()) + .unwrap_or(""); + let mut path = snapshot.path.clone(); + path.set_file_name(format!( + "{file_name}.{}", + extension_for_middleware(new_middleware) + )); + let inst_path = snapshot.get_new_inst_path(snapshot.new); + let new_snapshot = snapshot.with_new_path(path, snapshot.new, snapshot.old); + stats.record_rbxm_fallback(&inst_path, &err.to_string()); + new_middleware + .syncback(&new_snapshot) + .with_context(|| format!("Failed to syncback {inst_path}")) + } + Err(err) => { + let inst_path = snapshot.get_new_inst_path(snapshot.new); + Err(err).with_context(|| format!("Failed to syncback {inst_path}")) + } + }; + (snapshot, result) + }) + .collect(); + + // Phase 3: Sequential merge of results. + for (snapshot, result) in results { + let syncback = result?; + + if !syncback.removed_children.is_empty() { + 'remove: for inst in &syncback.removed_children { + let path = inst.metadata().instigating_source.as_ref().unwrap().path(); + let inst_path = snapshot.get_old_inst_path(inst.id()); + if let Some(name) = path.file_name().and_then(|n| n.to_str()) { + if name.starts_with('.') && name != ".gitkeep" { continue; } } - _ => unreachable!("All Instances in both DOMs should have hashes"), - } - } - } - - if !is_valid_path(&ignore_patterns, project_path, &snapshot.path) { - log::debug!("Skipping {inst_path} because its path matches ignore pattern"); - continue; - } - // Check ignoreTrees with glob pattern support - if let Some(ref globs) = tree_globs { - for (glob, _pattern) in globs { - if glob.is_match(&inst_path) { - log::debug!("Tree {inst_path} is blocked by ignoreTrees glob pattern"); - continue 'syncback; - } - } - } - - let middleware = get_best_middleware(&snapshot); - - log::trace!( - "Middleware for {inst_path} is {:?} (path is {})", - middleware, - snapshot.path.display() - ); - - if matches!(middleware, Middleware::Json | Middleware::Toml) { - log::warn!("Cannot syncback {middleware:?} at {inst_path}, skipping"); - continue; - } - - let syncback_start = std::time::Instant::now(); - let syncback = match middleware.syncback(&snapshot) { - Ok(syncback) => syncback, - Err(err) if middleware == Middleware::Dir => { - let new_middleware = match env::var(DEBUG_MODEL_FORMAT_VAR) { - Ok(value) if value == "1" => Middleware::Rbxmx, - Ok(value) if value == "2" => Middleware::JsonModel, - _ => Middleware::Rbxm, - }; - let file_name = snapshot - .path - .file_name() - .and_then(|s| s.to_str()) - .context("Directory middleware should have a name in its path")?; - let mut path = snapshot.path.clone(); - path.set_file_name(format!( - "{file_name}.{}", - extension_for_middleware(new_middleware) - )); - let new_snapshot = snapshot.with_new_path(path, snapshot.new, snapshot.old); - // Record the fallback in stats instead of warning directly - stats.record_rbxm_fallback(&inst_path, &err.to_string()); - let new_syncback_result = new_middleware - .syncback(&new_snapshot) - .with_context(|| format!("Failed to syncback {inst_path}")); - if new_syncback_result.is_ok() && snapshot.old_inst().is_some() { - // We need to remove the old FS representation if we're - // reserializing it as an rbxm. - fs_snapshot.remove_dir(&snapshot.path); - } - new_syncback_result? - } - Err(err) => anyhow::bail!("Failed to syncback {inst_path} because {err}"), - }; - - walk_syncback_ns += syncback_start.elapsed().as_nanos(); - - if !syncback.removed_children.is_empty() { - log::debug!( - "removed children for {inst_path}: {}", - syncback.removed_children.len() - ); - 'remove: for inst in &syncback.removed_children { - let path = inst.metadata().instigating_source.as_ref().unwrap().path(); - let inst_path = snapshot.get_old_inst_path(inst.id()); - if let Some(name) = path.file_name().and_then(|n| n.to_str()) { - if name.starts_with('.') && name != ".gitkeep" { - log::debug!("Skipping removing {} (hidden path)", path.display()); + if !is_valid_path(&ignore_patterns, project_path, path) { continue; } - } - if !is_valid_path(&ignore_patterns, project_path, path) { - log::debug!( - "Skipping removing {} because its matches an ignore pattern", - path.display() - ); - continue; - } - // Check ignoreTrees with glob pattern support - if let Some(ref globs) = tree_globs { - for (glob, _pattern) in globs { - if glob.is_match(&inst_path) { - log::debug!("Skipping removing {inst_path} because its path is blocked by ignoreTrees glob pattern"); - continue 'remove; + if let Some(ref globs) = tree_globs { + for (glob, _pattern) in globs { + if glob.is_match(&inst_path) { + continue 'remove; + } } } - } - if path.is_dir() { - fs_snapshot.remove_dir(path) - } else { - fs_snapshot.remove_file(path) + if path.is_dir() { + fs_snapshot.remove_dir(path) + } else { + fs_snapshot.remove_file(path) + } } } + + let files_for_instance: Vec = syncback + .fs_snapshot + .added_files() + .into_iter() + .map(|p| p.to_path_buf()) + .collect(); + if !files_for_instance.is_empty() { + instance_paths.insert(snapshot.new, files_for_instance); + } + + fs_snapshot.merge_with_filter(syncback.fs_snapshot, |path| { + is_valid_path(&ignore_patterns, project_path, path) + }); + + next_snapshots.extend(syncback.children); } - // TODO provide replacement snapshots for e.g. two way sync - - // Collect file paths for this instance before merging into the main snapshot. - // This builds the instance-to-path map used for sourcemap generation. - let files_for_instance: Vec = syncback - .fs_snapshot - .added_files() - .into_iter() - .map(|p| p.to_path_buf()) - .collect(); - if !files_for_instance.is_empty() { - instance_paths.insert(snapshot.new, files_for_instance); - } - - let merge_start = std::time::Instant::now(); - fs_snapshot.merge_with_filter(syncback.fs_snapshot, |path| { - is_valid_path(&ignore_patterns, project_path, path) - }); - walk_merge_ns += merge_start.elapsed().as_nanos(); - - snapshots.extend(syncback.children); + snapshots = next_snapshots; } log::debug!( - "[PERF] main walk loop: {:.3}s ({} instances, syncback={:.3}s, merge={:.3}s, path={:.3}s)", + "[PERF] main walk loop: {:.3}s ({} instances)", phase_timer.elapsed().as_secs_f64(), walk_count, - walk_syncback_ns as f64 / 1e9, - walk_merge_ns as f64 / 1e9, - walk_path_ns as f64 / 1e9, ); let phase_timer = std::time::Instant::now(); { use ref_properties::tentative_fs_path_public; - let final_map = ref_path_map.borrow(); + let final_map = ref_path_map.lock().unwrap(); let mut substitutions: Vec<(String, String)> = Vec::new(); for (placeholder, (source_ref, target_ref)) in &placeholder_map { let source_abs = final_map diff --git a/src/syncback/property_filter.rs b/src/syncback/property_filter.rs index 1f622f47..b8a8c593 100644 --- a/src/syncback/property_filter.rs +++ b/src/syncback/property_filter.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; + use rbx_dom_weak::{types::Variant, Instance, Ustr, UstrMap}; use rbx_reflection::{PropertyKind, PropertySerialization, Scriptability}; @@ -5,6 +7,128 @@ use crate::{variant_eq::variant_eq, Project}; use super::SyncbackStats; +/// Per-class cache of which properties should be skipped during syncback +/// filtering. Eliminates repeated superclass-chain walks in the reflection +/// database for serialization and scriptability checks. +pub struct PropertyFilterCache { + sync_unscriptable: bool, + /// ClassName -> set of property names that FAIL the static checks + /// (DoesNotSerialize or Scriptability::None when sync_unscriptable=false). + /// Properties in this set should be skipped. + skip_sets: HashMap, +} + +type UstrSet = std::collections::HashSet; + +impl PropertyFilterCache { + pub fn new(project: &Project) -> Self { + let sync_unscriptable = project + .syncback_rules + .as_ref() + .and_then(|s| s.sync_unscriptable) + .unwrap_or(false); + Self { + sync_unscriptable, + skip_sets: HashMap::new(), + } + } + + /// Returns the set of property names to skip for a given class. + /// Builds and caches the set on first access. + fn skip_set_for(&mut self, class_name: &Ustr) -> &UstrSet { + if self.skip_sets.contains_key(class_name) { + return &self.skip_sets[class_name]; + } + let sync_unscriptable = self.sync_unscriptable; + let database = rbx_reflection_database::get().unwrap(); + let class_data = database.classes.get(class_name.as_str()); + let mut skip = UstrSet::new(); + + if let Some(class_data) = class_data { + // Walk all properties known to the reflection DB for this class + // (including inherited) and mark those that fail static checks. + let mut current = Some(class_data); + while let Some(data) = current { + for (prop_name, prop_data) in &data.properties { + let ustr_name = Ustr::from(prop_name); + if skip.contains(&ustr_name) { + continue; + } + let should_skip_serialize = match &prop_data.kind { + PropertyKind::Alias { alias_for } => { + !should_property_serialize(class_name.as_str(), alias_for) + } + PropertyKind::Canonical { serialization } => { + matches!(serialization, PropertySerialization::DoesNotSerialize) + } + _ => false, + }; + if should_skip_serialize { + skip.insert(ustr_name); + continue; + } + if !sync_unscriptable && matches!(prop_data.scriptability, Scriptability::None) + { + skip.insert(ustr_name); + } + } + current = data + .superclass + .as_ref() + .and_then(|s| database.classes.get(&**s)); + } + } + + self.skip_sets.entry(*class_name).or_insert(skip) + } + + /// Cached version of `filter_properties_preallocated`. Fills `allocation` + /// with properties that pass all static and value-dependent checks. + pub fn filter_properties<'inst>( + &mut self, + inst: &'inst Instance, + allocation: &mut Vec<(Ustr, &'inst Variant)>, + stats: Option<&SyncbackStats>, + ) { + let database = rbx_reflection_database::get().unwrap(); + let class_data = database.classes.get(inst.class.as_str()); + + if class_data.is_none() { + if let Some(stats) = stats { + stats.record_unknown_class(&inst.class); + } + } + + let skip = self.skip_set_for(&inst.class); + + if let Some(class_data) = class_data { + let defaults = &class_data.default_properties; + for (name, value) in &inst.properties { + if matches!(value, Variant::Ref(_) | Variant::UniqueId(_)) { + continue; + } + if skip.contains(name) { + continue; + } + if let Some(default) = defaults.get(name.as_str()) { + if !variant_eq(value, default) { + allocation.push((*name, value)); + } + } else { + allocation.push((*name, value)); + } + } + } else { + for (name, value) in &inst.properties { + if matches!(value, Variant::Ref(_) | Variant::UniqueId(_)) { + continue; + } + allocation.push((*name, value)); + } + } + } +} + /// Returns a map of properties from `inst` that are both allowed under the /// user-provided settings, are not their default value, and serialize. pub fn filter_properties<'inst>( diff --git a/src/syncback/snapshot.rs b/src/syncback/snapshot.rs index 87318625..598a6656 100644 --- a/src/syncback/snapshot.rs +++ b/src/syncback/snapshot.rs @@ -1,8 +1,8 @@ use indexmap::IndexMap; use memofs::Vfs; -use std::cell::RefCell; use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; +use std::sync::Mutex; use crate::{ glob::Glob, @@ -15,9 +15,7 @@ use rbx_dom_weak::{ Instance, Ustr, UstrMap, WeakDom, }; -use super::{ - get_best_middleware, name_for_inst, property_filter::filter_properties, SyncbackStats, -}; +use super::{get_best_middleware, name_for_inst, PropertyFilterCache, SyncbackStats}; #[derive(Clone, Copy)] pub struct SyncbackData<'sync> { @@ -33,7 +31,10 @@ pub struct SyncbackData<'sync> { /// Records (Ref → ref-path-string) for each instance as filenames are /// assigned during the syncback walk. Used after the walk to fix /// Rojo_Ref_* attribute paths with correct dedup suffixes. - pub(super) ref_path_map: &'sync RefCell>, + pub(super) ref_path_map: &'sync Mutex>, + /// Cached property filter results per ClassName, avoiding repeated + /// superclass-chain walks in the reflection database. + pub(super) prop_filter_cache: &'sync Mutex, } impl<'sync> SyncbackData<'sync> { @@ -147,7 +148,8 @@ impl<'sync> SyncbackSnapshot<'sync> { let parent_path = self .data .ref_path_map - .borrow() + .lock() + .unwrap() .get(&self.new) .cloned() .unwrap_or_default(); @@ -158,7 +160,8 @@ impl<'sync> SyncbackSnapshot<'sync> { }; self.data .ref_path_map - .borrow_mut() + .lock() + .unwrap() .insert(child_ref, child_path); } @@ -196,9 +199,10 @@ impl<'sync> SyncbackSnapshot<'sync> { pub fn get_path_filtered_properties(&self, new_ref: Ref) -> Option> { let inst = self.get_new_instance(new_ref)?; - // The only filtering we have to do is filter out properties that are - // special-cased in some capacity. - let properties = filter_properties(self.data.project, inst) + let mut buf = Vec::with_capacity(inst.properties.len()); + self.filter_properties_cached(inst, &mut buf); + + let properties = buf .into_iter() .filter(|(name, _)| !filter_out_property(inst, name)) .collect(); @@ -256,6 +260,20 @@ impl<'sync> SyncbackSnapshot<'sync> { self.data.project } + /// Fills `allocation` with filtered properties using the shared cache. + #[inline] + pub fn filter_properties_cached<'inst>( + &self, + inst: &'inst Instance, + allocation: &mut Vec<(rbx_dom_weak::Ustr, &'inst Variant)>, + ) { + self.data + .prop_filter_cache + .lock() + .unwrap() + .filter_properties(inst, allocation, None); + } + /// Returns the underlying VFS being used for syncback. #[inline] pub fn vfs(&self) -> &'sync Vfs {