From a823db0297ad3e3f43a8b4d7e8961b7674433904 Mon Sep 17 00:00:00 2001 From: Joe Date: Wed, 18 Feb 2026 08:11:00 -0600 Subject: [PATCH] Refactor encoding logic in ChangeBatcher and improve matching depth handling - Simplified the `isReadable` condition in `encodeService` for better readability. - Updated the `ServiceChunk` structure to conditionally include properties and references only if they exist, enhancing data efficiency. - Introduced a `MAX_SCORING_DEPTH` constant in the matching module to limit recursion depth, improving performance and preventing potential stack overflow issues. - Refactored the `computePairCost` and `matchChildren` functions to incorporate the new depth parameter, ensuring accurate cost calculations during matching. These changes enhance the clarity and efficiency of the encoding and matching processes, contributing to overall performance improvements. --- plugin/src/ChangeBatcher/encodeService.lua | 14 +++++++---- plugin/src/Reconciler/matching.lua | 28 +++++++++++++++------- src/syncback/matching.rs | 1 + src/syncback/property_filter.rs | 6 +---- tests/tests/live_syncback.rs | 6 ++--- 5 files changed, 34 insertions(+), 21 deletions(-) diff --git a/plugin/src/ChangeBatcher/encodeService.lua b/plugin/src/ChangeBatcher/encodeService.lua index 1f186b60..9cce3c7e 100644 --- a/plugin/src/ChangeBatcher/encodeService.lua +++ b/plugin/src/ChangeBatcher/encodeService.lua @@ -24,8 +24,7 @@ return function(service: Instance) continue end - local isReadable = propertyMeta.scriptability == "ReadWrite" - or propertyMeta.scriptability == "Read" + local isReadable = propertyMeta.scriptability == "ReadWrite" or propertyMeta.scriptability == "Read" local doesSerialize = propertyMeta.serialization ~= "DoesNotSerialize" if isReadable and doesSerialize then @@ -66,10 +65,15 @@ return function(service: Instance) end end - return { + local chunk = { className = service.ClassName, data = data, - properties = properties, - refs = refs, } + if next(properties) then + chunk.properties = properties + end + if next(refs) then + chunk.refs = refs + end + return chunk end diff --git a/plugin/src/Reconciler/matching.lua b/plugin/src/Reconciler/matching.lua index 8dc0bbd6..0371424e 100644 --- a/plugin/src/Reconciler/matching.lua +++ b/plugin/src/Reconciler/matching.lua @@ -25,6 +25,7 @@ local RbxDom = require(Packages.RbxDom) local trueEquals = require(script.Parent.trueEquals) local UNMATCHED_PENALTY = 10000 +local MAX_SCORING_DEPTH = 3 -- ================================================================ -- Types @@ -341,7 +342,8 @@ local function computePairCost( virtualId: string, studioInstance: Instance, virtualInstances: VirtualInstances, - bestSoFar: number + bestSoFar: number, + depth: number ): number local vc = session.costCache[virtualId] if vc then @@ -365,7 +367,7 @@ local function computePairCost( local sCache = cacheStudio(studioInstance, classKeys, vCache.extraProps, refPropNames) local cost = countOwnDiffs(vCache, sCache, classKeys) - if cost >= bestSoFar then + if cost >= bestSoFar or depth >= MAX_SCORING_DEPTH then return cost end @@ -378,8 +380,15 @@ local function computePairCost( elseif #studioKids == 0 then cost += #validVChildren * UNMATCHED_PENALTY else - local childResult = - matchChildren(session, validVChildren, studioKids, virtualInstances, virtualId, studioInstance) + local childResult = matchChildren( + session, + validVChildren, + studioKids, + virtualInstances, + virtualId, + studioInstance, + depth + 1 + ) cost += childResult.totalCost end end @@ -400,8 +409,10 @@ matchChildren = function( studioChildren: { Instance }, virtualInstances: VirtualInstances, parentVirtualId: string?, - parentStudioInstance: Instance? + parentStudioInstance: Instance?, + depth: number? ): MatchResult + depth = depth or 0 if parentVirtualId and parentStudioInstance then local pc = session.matchCache[parentVirtualId] if pc then @@ -522,7 +533,7 @@ matchChildren = function( pairIdx += 1 local cost = countOwnDiffs(vCache, sCache, classKeys) - if cost < bestSoFar then + if cost < bestSoFar and depth < MAX_SCORING_DEPTH then local validVChildren = vCache.validChildren local studioKids = sCache.children @@ -538,7 +549,8 @@ matchChildren = function( studioKids, virtualInstances, virtualChildren[vi], - sCache.instance + sCache.instance, + depth + 1 ) cost += childResult.totalCost end @@ -591,7 +603,7 @@ matchChildren = function( local totalCost = 0 for _, pair in ipairs(matched) do - totalCost += computePairCost(session, pair.virtualId, pair.studioInstance, virtualInstances, math.huge) + totalCost += computePairCost(session, pair.virtualId, pair.studioInstance, virtualInstances, math.huge, depth) end totalCost += (#unmatchedVirtual + #unmatchedStudio) * UNMATCHED_PENALTY diff --git a/src/syncback/matching.rs b/src/syncback/matching.rs index 10714f53..130c4c7a 100644 --- a/src/syncback/matching.rs +++ b/src/syncback/matching.rs @@ -251,6 +251,7 @@ struct ScoringMatchResult { /// Match children by reference for scoring purposes (non-consuming). /// Groups by (Name, ClassName), instant-matches 1:1 groups, and scores /// ambiguous groups using `compute_change_count` (mutually recursive). +#[allow(clippy::too_many_arguments)] fn match_children_for_scoring( new_children: &[Ref], old_children: &[Ref], diff --git a/src/syncback/property_filter.rs b/src/syncback/property_filter.rs index ddb1fad5..1f622f47 100644 --- a/src/syncback/property_filter.rs +++ b/src/syncback/property_filter.rs @@ -62,11 +62,7 @@ pub fn filter_properties_with_stats<'inst>( } if !sync_unscriptable { let mut current = class_data; - loop { - let data = match current { - Some(d) => d, - None => break, - }; + while let Some(data) = current { if let Some(prop_data) = data.properties.get(prop_name.as_str()) { if matches!(prop_data.scriptability, Scriptability::None) { return true; diff --git a/tests/tests/live_syncback.rs b/tests/tests/live_syncback.rs index eeb2150b..140dc2f9 100644 --- a/tests/tests/live_syncback.rs +++ b/tests/tests/live_syncback.rs @@ -316,7 +316,7 @@ fn parity_multiple_services_with_properties() { #[test] fn parity_camera_not_synced() { let camera_child = InstanceBuilder::new("Camera").with_name("Camera"); - let chunks = vec![make_service_chunk_full( + let chunks = [make_service_chunk_full( "Workspace", vec![], vec![("CurrentCamera", "Camera", "Camera")], @@ -698,7 +698,7 @@ fn roundtrip_build_syncback_rebuild() { .iter() .map(|&r| { let c = dom_a.get_by_ref(r).unwrap(); - (c.name.clone(), c.class.clone(), r) + (c.name.clone(), c.class, r) }) .collect(); children_a.sort_by(|a, b| (&a.0, &a.1).cmp(&(&b.0, &b.1))); @@ -708,7 +708,7 @@ fn roundtrip_build_syncback_rebuild() { .iter() .map(|&r| { let c = dom_b.get_by_ref(r).unwrap(); - (c.name.clone(), c.class.clone(), r) + (c.name.clone(), c.class, r) }) .collect(); children_b.sort_by(|a, b| (&a.0, &a.1).cmp(&(&b.0, &b.1)));