feat: add property filter caching and size hint for JSON5 serialization

- Introduced a `PropertyFilterCache` to optimize property filtering during syncback, reducing repeated superclass-chain walks in the reflection database.
- Added a `size_hint` method to `Json5Value` for estimating byte size during JSON5 serialization, improving performance by pre-allocating output strings based on value types.
- Updated `syncback` logic to utilize the new caching mechanism for filtering properties, enhancing efficiency in the syncback process.

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::<usize>(),
+            Json5Value::Object(map) => {
+                8 + map
+                    .iter()
+                    .map(|(k, v)| k.len() + v.size_hint() + 10)
+                    .sum::<usize>()
+            }
+        }
+    }
+
     /// 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<T: Serialize>(value: &T) -> anyhow::Result<Vec<u8>>
         .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())

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();

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;

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,64 +545,69 @@ pub fn syncback_loop_with_stats(
     let mut fs_snapshot = FsSnapshot::new();
     let mut instance_paths: HashMap<Ref, Vec<PathBuf>> = 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() {
+    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<WaveItem> = Vec::with_capacity(snapshots.len());
+        let mut next_snapshots: Vec<SyncbackSnapshot> = Vec::new();
+
+        'filter: for snapshot in snapshots.drain(..) {
             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:?}"
-                            );
-                            continue;
-                        }
-                    }
-                    _ => unreachable!("All Instances in both DOMs should have hashes"),
+                        (Some(old), Some(new)) if old == new => continue,
+                        _ => {}
                     }
                 }
             }
 
             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 {
+                let inst_path = snapshot.get_new_inst_path(snapshot.new);
                 for (glob, _pattern) in globs {
                     if glob.is_match(&inst_path) {
-                    log::debug!("Tree {inst_path} is blocked by ignoreTrees glob pattern");
-                    continue 'syncback;
+                        continue 'filter;
                     }
                 }
             }
 
             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,
+            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,
@@ -601,56 +618,47 @@ pub fn syncback_loop_with_stats(
                             .path
                             .file_name()
                             .and_then(|s| s.to_str())
-                    .context("Directory middleware should have a name in its path")?;
+                            .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);
-                // Record the fallback in stats instead of warning directly
                         stats.record_rbxm_fallback(&inst_path, &err.to_string());
-                let new_syncback_result = new_middleware
+                        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);
+                            .with_context(|| format!("Failed to syncback {inst_path}"))
                     }
-                new_syncback_result?
+                    Err(err) => {
+                        let inst_path = snapshot.get_new_inst_path(snapshot.new);
+                        Err(err).with_context(|| format!("Failed to syncback {inst_path}"))
                     }
-            Err(err) => anyhow::bail!("Failed to syncback {inst_path} because {err}"),
                 };
+                (snapshot, result)
+            })
+            .collect();
 
-        walk_syncback_ns += syncback_start.elapsed().as_nanos();
+        // Phase 3: Sequential merge of results.
+        for (snapshot, result) in results {
+            let syncback = result?;
 
             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());
                             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;
                             }
                         }
@@ -663,10 +671,6 @@ pub fn syncback_loop_with_stats(
                 }
             }
 
-        // 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<PathBuf> = syncback
                 .fs_snapshot
                 .added_files()
@@ -677,28 +681,26 @@ pub fn syncback_loop_with_stats(
                 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);
+            next_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

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<Ustr, UstrSet>,
+}
+
+type UstrSet = std::collections::HashSet<Ustr>;
+
+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>(

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<HashMap<Ref, String>>,
+    pub(super) ref_path_map: &'sync Mutex<HashMap<Ref, String>>,
+    /// Cached property filter results per ClassName, avoiding repeated
+    /// superclass-chain walks in the reflection database.
+    pub(super) prop_filter_cache: &'sync Mutex<PropertyFilterCache>,
 }
 
 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<UstrMap<&'sync Variant>> {
         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 {