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.
This commit is contained in:
Joe 2026-02-18 08:11:00 -06:00
parent e0fd9077e8
commit a823db0297
5 changed files with 34 additions and 21 deletions

View file

@ -24,8 +24,7 @@ return function(service: Instance)
continue continue
end end
local isReadable = propertyMeta.scriptability == "ReadWrite" local isReadable = propertyMeta.scriptability == "ReadWrite" or propertyMeta.scriptability == "Read"
or propertyMeta.scriptability == "Read"
local doesSerialize = propertyMeta.serialization ~= "DoesNotSerialize" local doesSerialize = propertyMeta.serialization ~= "DoesNotSerialize"
if isReadable and doesSerialize then if isReadable and doesSerialize then
@ -66,10 +65,15 @@ return function(service: Instance)
end end
end end
return { local chunk = {
className = service.ClassName, className = service.ClassName,
data = data, 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 end

View file

@ -25,6 +25,7 @@ local RbxDom = require(Packages.RbxDom)
local trueEquals = require(script.Parent.trueEquals) local trueEquals = require(script.Parent.trueEquals)
local UNMATCHED_PENALTY = 10000 local UNMATCHED_PENALTY = 10000
local MAX_SCORING_DEPTH = 3
-- ================================================================ -- ================================================================
-- Types -- Types
@ -341,7 +342,8 @@ local function computePairCost(
virtualId: string, virtualId: string,
studioInstance: Instance, studioInstance: Instance,
virtualInstances: VirtualInstances, virtualInstances: VirtualInstances,
bestSoFar: number bestSoFar: number,
depth: number
): number ): number
local vc = session.costCache[virtualId] local vc = session.costCache[virtualId]
if vc then if vc then
@ -365,7 +367,7 @@ local function computePairCost(
local sCache = cacheStudio(studioInstance, classKeys, vCache.extraProps, refPropNames) local sCache = cacheStudio(studioInstance, classKeys, vCache.extraProps, refPropNames)
local cost = countOwnDiffs(vCache, sCache, classKeys) local cost = countOwnDiffs(vCache, sCache, classKeys)
if cost >= bestSoFar then if cost >= bestSoFar or depth >= MAX_SCORING_DEPTH then
return cost return cost
end end
@ -378,8 +380,15 @@ local function computePairCost(
elseif #studioKids == 0 then elseif #studioKids == 0 then
cost += #validVChildren * UNMATCHED_PENALTY cost += #validVChildren * UNMATCHED_PENALTY
else else
local childResult = local childResult = matchChildren(
matchChildren(session, validVChildren, studioKids, virtualInstances, virtualId, studioInstance) session,
validVChildren,
studioKids,
virtualInstances,
virtualId,
studioInstance,
depth + 1
)
cost += childResult.totalCost cost += childResult.totalCost
end end
end end
@ -400,8 +409,10 @@ matchChildren = function(
studioChildren: { Instance }, studioChildren: { Instance },
virtualInstances: VirtualInstances, virtualInstances: VirtualInstances,
parentVirtualId: string?, parentVirtualId: string?,
parentStudioInstance: Instance? parentStudioInstance: Instance?,
depth: number?
): MatchResult ): MatchResult
depth = depth or 0
if parentVirtualId and parentStudioInstance then if parentVirtualId and parentStudioInstance then
local pc = session.matchCache[parentVirtualId] local pc = session.matchCache[parentVirtualId]
if pc then if pc then
@ -522,7 +533,7 @@ matchChildren = function(
pairIdx += 1 pairIdx += 1
local cost = countOwnDiffs(vCache, sCache, classKeys) local cost = countOwnDiffs(vCache, sCache, classKeys)
if cost < bestSoFar then if cost < bestSoFar and depth < MAX_SCORING_DEPTH then
local validVChildren = vCache.validChildren local validVChildren = vCache.validChildren
local studioKids = sCache.children local studioKids = sCache.children
@ -538,7 +549,8 @@ matchChildren = function(
studioKids, studioKids,
virtualInstances, virtualInstances,
virtualChildren[vi], virtualChildren[vi],
sCache.instance sCache.instance,
depth + 1
) )
cost += childResult.totalCost cost += childResult.totalCost
end end
@ -591,7 +603,7 @@ matchChildren = function(
local totalCost = 0 local totalCost = 0
for _, pair in ipairs(matched) do 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 end
totalCost += (#unmatchedVirtual + #unmatchedStudio) * UNMATCHED_PENALTY totalCost += (#unmatchedVirtual + #unmatchedStudio) * UNMATCHED_PENALTY

View file

@ -251,6 +251,7 @@ struct ScoringMatchResult {
/// Match children by reference for scoring purposes (non-consuming). /// Match children by reference for scoring purposes (non-consuming).
/// Groups by (Name, ClassName), instant-matches 1:1 groups, and scores /// Groups by (Name, ClassName), instant-matches 1:1 groups, and scores
/// ambiguous groups using `compute_change_count` (mutually recursive). /// ambiguous groups using `compute_change_count` (mutually recursive).
#[allow(clippy::too_many_arguments)]
fn match_children_for_scoring( fn match_children_for_scoring(
new_children: &[Ref], new_children: &[Ref],
old_children: &[Ref], old_children: &[Ref],

View file

@ -62,11 +62,7 @@ pub fn filter_properties_with_stats<'inst>(
} }
if !sync_unscriptable { if !sync_unscriptable {
let mut current = class_data; let mut current = class_data;
loop { while let Some(data) = current {
let data = match current {
Some(d) => d,
None => break,
};
if let Some(prop_data) = data.properties.get(prop_name.as_str()) { if let Some(prop_data) = data.properties.get(prop_name.as_str()) {
if matches!(prop_data.scriptability, Scriptability::None) { if matches!(prop_data.scriptability, Scriptability::None) {
return true; return true;

View file

@ -316,7 +316,7 @@ fn parity_multiple_services_with_properties() {
#[test] #[test]
fn parity_camera_not_synced() { fn parity_camera_not_synced() {
let camera_child = InstanceBuilder::new("Camera").with_name("Camera"); let camera_child = InstanceBuilder::new("Camera").with_name("Camera");
let chunks = vec![make_service_chunk_full( let chunks = [make_service_chunk_full(
"Workspace", "Workspace",
vec![], vec![],
vec![("CurrentCamera", "Camera", "Camera")], vec![("CurrentCamera", "Camera", "Camera")],
@ -698,7 +698,7 @@ fn roundtrip_build_syncback_rebuild() {
.iter() .iter()
.map(|&r| { .map(|&r| {
let c = dom_a.get_by_ref(r).unwrap(); let c = dom_a.get_by_ref(r).unwrap();
(c.name.clone(), c.class.clone(), r) (c.name.clone(), c.class, r)
}) })
.collect(); .collect();
children_a.sort_by(|a, b| (&a.0, &a.1).cmp(&(&b.0, &b.1))); 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() .iter()
.map(|&r| { .map(|&r| {
let c = dom_b.get_by_ref(r).unwrap(); let c = dom_b.get_by_ref(r).unwrap();
(c.name.clone(), c.class.clone(), r) (c.name.clone(), c.class, r)
}) })
.collect(); .collect();
children_b.sort_by(|a, b| (&a.0, &a.1).cmp(&(&b.0, &b.1))); children_b.sort_by(|a, b| (&a.0, &a.1).cmp(&(&b.0, &b.1)));