mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-10-23 22:11:38 +00:00
Enhance matching module with reference identity and caching improvements
- Introduced a new `RefIdentity` type to store reference details (name and className) in the matching process. - Updated `VCache` and `SCache` structures to include references, improving the handling of reference comparisons during matching. - Modified `cacheVirtual` and `cacheStudio` functions to support reference caching, enhancing performance and accuracy in matching. - Improved `countOwnDiffs` function to account for reference mismatches, ensuring more precise cost calculations. - Added comprehensive tests to validate the new reference handling and caching mechanisms. These changes significantly enhance the efficiency and accuracy of the matching process in the application.
This commit is contained in:
parent
bf4aef6e94
commit
b687c8862d
12 changed files with 1401 additions and 61 deletions
|
|
@ -83,6 +83,7 @@ impl ChangeProcessor {
|
|||
suppressed_paths: Arc<Mutex<std::collections::HashMap<PathBuf, (usize, usize)>>>,
|
||||
ref_path_index: Arc<Mutex<crate::RefPathIndex>>,
|
||||
project_root: PathBuf,
|
||||
project_file_path: PathBuf,
|
||||
critical_error_receiver: Option<Receiver<memofs::WatcherCriticalError>>,
|
||||
git_repo_root: Option<PathBuf>,
|
||||
) -> Self {
|
||||
|
|
@ -95,6 +96,8 @@ impl ChangeProcessor {
|
|||
// Canonicalize project_root so path comparisons work with the
|
||||
// \\?\ prefix that std::fs::canonicalize adds on Windows.
|
||||
let project_root = std::fs::canonicalize(&project_root).unwrap_or(project_root);
|
||||
let project_file_path =
|
||||
std::fs::canonicalize(&project_file_path).unwrap_or(project_file_path);
|
||||
let task = JobThreadContext {
|
||||
tree,
|
||||
vfs,
|
||||
|
|
@ -102,6 +105,7 @@ impl ChangeProcessor {
|
|||
pending_recovery: Mutex::new(Vec::new()),
|
||||
suppressed_paths,
|
||||
project_root,
|
||||
project_file_path,
|
||||
ref_path_index,
|
||||
git_repo_root,
|
||||
};
|
||||
|
|
@ -240,6 +244,11 @@ struct JobThreadContext {
|
|||
/// Root directory of the project, used to display relative paths in logs.
|
||||
project_root: PathBuf,
|
||||
|
||||
/// Path to the project file (e.g. `plugin.project.json`). Used by
|
||||
/// `reconcile_tree` so that re-snapshotting goes through the project
|
||||
/// middleware instead of walking the directory as a generic Folder.
|
||||
project_file_path: PathBuf,
|
||||
|
||||
/// Index of meta/model files that contain `Rojo_Ref_*` attributes.
|
||||
/// Shared with ApiService for efficient rename path updates.
|
||||
ref_path_index: Arc<Mutex<crate::RefPathIndex>>,
|
||||
|
|
@ -928,7 +937,8 @@ impl JobThreadContext {
|
|||
let start = Instant::now();
|
||||
let instance_context = InstanceContext::new();
|
||||
|
||||
let snapshot = match snapshot_from_vfs(&instance_context, &self.vfs, &self.project_root) {
|
||||
let snapshot = match snapshot_from_vfs(&instance_context, &self.vfs, &self.project_file_path)
|
||||
{
|
||||
Ok(s) => s,
|
||||
Err(e) => {
|
||||
log::error!("Tree reconciliation snapshot error: {:?}", e);
|
||||
|
|
|
|||
|
|
@ -193,6 +193,7 @@ impl ServeSession {
|
|||
Arc::clone(&suppressed_paths),
|
||||
Arc::clone(&ref_path_index),
|
||||
root_project.folder_location().to_path_buf(),
|
||||
root_project.file_location.clone(),
|
||||
critical_error_receiver,
|
||||
git_repo_root.clone(),
|
||||
);
|
||||
|
|
@ -366,7 +367,7 @@ impl ServeSession {
|
|||
/// Does NOT apply corrections — the tree is left unchanged.
|
||||
pub fn check_tree_freshness(&self) -> TreeFreshnessReport {
|
||||
let start = Instant::now();
|
||||
let start_path = self.root_project.folder_location();
|
||||
let start_path: &Path = &self.root_project.file_location;
|
||||
let instance_context = InstanceContext::new();
|
||||
|
||||
let snapshot = match snapshot_from_vfs(&instance_context, &self.vfs, start_path) {
|
||||
|
|
@ -429,7 +430,7 @@ impl ServeSession {
|
|||
}
|
||||
|
||||
let start = Instant::now();
|
||||
let start_path = self.root_project.folder_location();
|
||||
let start_path: &Path = &self.root_project.file_location;
|
||||
let instance_context = InstanceContext::new();
|
||||
|
||||
let snapshot = match snapshot_from_vfs(&instance_context, &self.vfs, start_path) {
|
||||
|
|
|
|||
|
|
@ -1308,4 +1308,136 @@ mod tests {
|
|||
elapsed
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn total_cost_zero_for_identical() {
|
||||
let snaps = vec![
|
||||
make_snapshot("Alpha", "Folder"),
|
||||
make_snapshot("Beta", "Folder"),
|
||||
];
|
||||
let (tree, children) = make_tree_with_children(&[("Alpha", "Folder"), ("Beta", "Folder")]);
|
||||
let result = match_forward(snaps, &children, &tree, &MatchingSession::new());
|
||||
assert_eq!(result.matched.len(), 2);
|
||||
assert_eq!(result.total_cost, 0, "Identical instances should have zero cost");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn total_cost_nonzero_for_different() {
|
||||
use rbx_dom_weak::types::Variant;
|
||||
|
||||
let snaps = vec![make_snapshot_with_props(
|
||||
"Part",
|
||||
"Part",
|
||||
vec![("Transparency", Variant::Float32(0.5))],
|
||||
)];
|
||||
let (tree, children) = make_tree_from_snapshots(vec![make_snapshot_with_props(
|
||||
"Part",
|
||||
"Part",
|
||||
vec![("Transparency", Variant::Float32(0.0))],
|
||||
)]);
|
||||
let result = match_forward(snaps, &children, &tree, &MatchingSession::new());
|
||||
assert_eq!(result.matched.len(), 1);
|
||||
assert!(
|
||||
result.total_cost > 0,
|
||||
"Different properties should produce nonzero cost, got {}",
|
||||
result.total_cost
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn total_cost_includes_unmatched() {
|
||||
let snaps = vec![
|
||||
make_snapshot("A", "Folder"),
|
||||
make_snapshot("B", "Folder"),
|
||||
make_snapshot("C", "Folder"),
|
||||
];
|
||||
let (tree, children) = make_tree_with_children(&[("A", "Folder"), ("B", "Folder")]);
|
||||
let result = match_forward(snaps, &children, &tree, &MatchingSession::new());
|
||||
assert_eq!(result.matched.len(), 2);
|
||||
assert_eq!(result.unmatched_snapshot.len(), 1);
|
||||
assert!(
|
||||
result.total_cost >= UNMATCHED_PENALTY,
|
||||
"Should include penalty for unmatched snapshot, got {}",
|
||||
result.total_cost
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn total_cost_sums_correctly() {
|
||||
use rbx_dom_weak::types::Variant;
|
||||
|
||||
let snaps = vec![
|
||||
make_snapshot("A", "Folder"),
|
||||
make_snapshot_with_props("B", "Part", vec![("Transparency", Variant::Float32(0.5))]),
|
||||
make_snapshot("C", "Folder"),
|
||||
];
|
||||
let tree_children_snaps = vec![
|
||||
make_snapshot("A", "Folder"),
|
||||
make_snapshot_with_props("B", "Part", vec![("Transparency", Variant::Float32(0.0))]),
|
||||
];
|
||||
let (tree, children) = make_tree_from_snapshots(tree_children_snaps);
|
||||
let result = match_forward(snaps, &children, &tree, &MatchingSession::new());
|
||||
|
||||
assert_eq!(result.matched.len(), 2);
|
||||
assert_eq!(result.unmatched_snapshot.len(), 1);
|
||||
// total = matched costs (A=0 + B>=1) + unmatched (C=10000)
|
||||
assert!(
|
||||
result.total_cost >= UNMATCHED_PENALTY + 1,
|
||||
"Should be at least penalty + 1 diff, got {}",
|
||||
result.total_cost
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn session_cache_consistent() {
|
||||
let snaps1 = vec![
|
||||
make_snapshot("X", "Folder"),
|
||||
make_snapshot("Y", "Folder"),
|
||||
];
|
||||
let snaps2 = vec![
|
||||
make_snapshot("X", "Folder"),
|
||||
make_snapshot("Y", "Folder"),
|
||||
];
|
||||
let (tree, children) = make_tree_with_children(&[("X", "Folder"), ("Y", "Folder")]);
|
||||
let session = MatchingSession::new();
|
||||
|
||||
let r1 = match_forward(snaps1, &children, &tree, &session);
|
||||
let r2 = match_forward(snaps2, &children, &tree, &session);
|
||||
|
||||
assert_eq!(r1.matched.len(), r2.matched.len());
|
||||
assert_eq!(r1.total_cost, r2.total_cost);
|
||||
assert_eq!(
|
||||
r1.unmatched_snapshot.len(),
|
||||
r2.unmatched_snapshot.len()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn session_cache_does_not_corrupt() {
|
||||
use rbx_dom_weak::types::Variant;
|
||||
|
||||
let session = MatchingSession::new();
|
||||
|
||||
// Group A: identical folders
|
||||
let snaps_a = vec![make_snapshot("A", "Folder")];
|
||||
let (tree_a, children_a) = make_tree_with_children(&[("A", "Folder")]);
|
||||
let ra = match_forward(snaps_a, &children_a, &tree_a, &session);
|
||||
assert_eq!(ra.total_cost, 0);
|
||||
|
||||
// Group B: parts with property diff
|
||||
let snaps_b = vec![make_snapshot_with_props(
|
||||
"B",
|
||||
"Part",
|
||||
vec![("Transparency", Variant::Float32(1.0))],
|
||||
)];
|
||||
let (tree_b, children_b) = make_tree_from_snapshots(vec![make_snapshot_with_props(
|
||||
"B",
|
||||
"Part",
|
||||
vec![("Transparency", Variant::Float32(0.0))],
|
||||
)]);
|
||||
let rb = match_forward(snaps_b, &children_b, &tree_b, &session);
|
||||
|
||||
assert_eq!(ra.total_cost, 0, "Group A should still be zero");
|
||||
assert!(rb.total_cost > 0, "Group B should have nonzero cost");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -758,4 +758,150 @@ mod tests {
|
|||
assert_eq!(a, b);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn total_cost_zero_identical() {
|
||||
let (new_dom, new_root, _, _) = build_test_dom();
|
||||
let (old_dom, old_root, _, _) = build_test_dom();
|
||||
|
||||
let new_children: Vec<Ref> = new_dom.get_by_ref(new_root).unwrap().children().to_vec();
|
||||
let old_children: Vec<Ref> = old_dom.get_by_ref(old_root).unwrap().children().to_vec();
|
||||
|
||||
let result = match_children(
|
||||
&new_children,
|
||||
&old_children,
|
||||
&new_dom,
|
||||
&old_dom,
|
||||
None,
|
||||
None,
|
||||
&MatchingSession::new(),
|
||||
);
|
||||
assert_eq!(result.matched.len(), 2);
|
||||
assert_eq!(
|
||||
result.total_cost, 0,
|
||||
"Identical instances should have zero cost"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn total_cost_nonzero_different() {
|
||||
use rbx_dom_weak::types::Variant;
|
||||
|
||||
let mut new_dom = WeakDom::new(InstanceBuilder::new("DataModel"));
|
||||
let new_root = new_dom.root_ref();
|
||||
new_dom.insert(
|
||||
new_root,
|
||||
InstanceBuilder::new("Part")
|
||||
.with_name("P")
|
||||
.with_property("Transparency", Variant::Float32(0.5)),
|
||||
);
|
||||
|
||||
let mut old_dom = WeakDom::new(InstanceBuilder::new("DataModel"));
|
||||
let old_root = old_dom.root_ref();
|
||||
old_dom.insert(
|
||||
old_root,
|
||||
InstanceBuilder::new("Part")
|
||||
.with_name("P")
|
||||
.with_property("Transparency", Variant::Float32(0.0)),
|
||||
);
|
||||
|
||||
let new_children: Vec<Ref> = new_dom.get_by_ref(new_root).unwrap().children().to_vec();
|
||||
let old_children: Vec<Ref> = old_dom.get_by_ref(old_root).unwrap().children().to_vec();
|
||||
|
||||
let result = match_children(
|
||||
&new_children,
|
||||
&old_children,
|
||||
&new_dom,
|
||||
&old_dom,
|
||||
None,
|
||||
None,
|
||||
&MatchingSession::new(),
|
||||
);
|
||||
assert_eq!(result.matched.len(), 1);
|
||||
assert!(
|
||||
result.total_cost > 0,
|
||||
"Different properties should produce nonzero cost, got {}",
|
||||
result.total_cost
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn total_cost_includes_unmatched() {
|
||||
let mut new_dom = WeakDom::new(InstanceBuilder::new("DataModel"));
|
||||
let new_root = new_dom.root_ref();
|
||||
new_dom.insert(
|
||||
new_root,
|
||||
InstanceBuilder::new("Folder").with_name("A"),
|
||||
);
|
||||
new_dom.insert(
|
||||
new_root,
|
||||
InstanceBuilder::new("Folder").with_name("B"),
|
||||
);
|
||||
new_dom.insert(
|
||||
new_root,
|
||||
InstanceBuilder::new("Folder").with_name("C"),
|
||||
);
|
||||
|
||||
let mut old_dom = WeakDom::new(InstanceBuilder::new("DataModel"));
|
||||
let old_root = old_dom.root_ref();
|
||||
old_dom.insert(
|
||||
old_root,
|
||||
InstanceBuilder::new("Folder").with_name("A"),
|
||||
);
|
||||
old_dom.insert(
|
||||
old_root,
|
||||
InstanceBuilder::new("Folder").with_name("B"),
|
||||
);
|
||||
|
||||
let new_children: Vec<Ref> = new_dom.get_by_ref(new_root).unwrap().children().to_vec();
|
||||
let old_children: Vec<Ref> = old_dom.get_by_ref(old_root).unwrap().children().to_vec();
|
||||
|
||||
let result = match_children(
|
||||
&new_children,
|
||||
&old_children,
|
||||
&new_dom,
|
||||
&old_dom,
|
||||
None,
|
||||
None,
|
||||
&MatchingSession::new(),
|
||||
);
|
||||
assert_eq!(result.matched.len(), 2);
|
||||
assert_eq!(result.unmatched_new.len(), 1);
|
||||
assert!(
|
||||
result.total_cost >= UNMATCHED_PENALTY,
|
||||
"Should include penalty for unmatched, got {}",
|
||||
result.total_cost
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn session_cache_consistent_syncback() {
|
||||
let (new_dom, new_root, _, _) = build_test_dom();
|
||||
let (old_dom, old_root, _, _) = build_test_dom();
|
||||
|
||||
let new_children: Vec<Ref> = new_dom.get_by_ref(new_root).unwrap().children().to_vec();
|
||||
let old_children: Vec<Ref> = old_dom.get_by_ref(old_root).unwrap().children().to_vec();
|
||||
let session = MatchingSession::new();
|
||||
|
||||
let r1 = match_children(
|
||||
&new_children,
|
||||
&old_children,
|
||||
&new_dom,
|
||||
&old_dom,
|
||||
None,
|
||||
None,
|
||||
&session,
|
||||
);
|
||||
let r2 = match_children(
|
||||
&new_children,
|
||||
&old_children,
|
||||
&new_dom,
|
||||
&old_dom,
|
||||
None,
|
||||
None,
|
||||
&session,
|
||||
);
|
||||
assert_eq!(r1.matched.len(), r2.matched.len());
|
||||
assert_eq!(r1.total_cost, r2.total_cost);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue