Git dependencies, skip dropping ServeSession

Cargo.lock

@@ -1633,11 +1633,11 @@ dependencies = [
 [[package]]
 name = "rbx_binary"
 version = "0.6.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "68f424adb7a0a24ab4bd153be141035f1404ae40affed902fd2721b42cca7f86"
+source = "git+https://github.com/rojo-rbx/rbx-dom#4fa99b4190009f417ce63c86b2c6725e32c0090e"
 dependencies = [
  "log",
  "lz4",
+ "profiling",
  "rbx_dom_weak",
  "rbx_reflection",
  "rbx_reflection_database",
@@ -1647,8 +1647,7 @@ dependencies = [
 [[package]]
 name = "rbx_dom_weak"
 version = "2.3.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6f7f524fb18f30d7065c82c4e87f747705679329810207e96169c6d4ec922d1f"
+source = "git+https://github.com/rojo-rbx/rbx-dom#4fa99b4190009f417ce63c86b2c6725e32c0090e"
 dependencies = [
  "rbx_types",
  "serde",
@@ -1657,8 +1656,7 @@ dependencies = [
 [[package]]
 name = "rbx_reflection"
 version = "4.2.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a8915748c8a3b2a92540e4e35e99ebd548df2d62b0a6cf38ae5d0081f0e611d5"
+source = "git+https://github.com/rojo-rbx/rbx-dom#4fa99b4190009f417ce63c86b2c6725e32c0090e"
 dependencies = [
  "rbx_types",
  "serde",
@@ -1667,8 +1665,7 @@ dependencies = [
 [[package]]
 name = "rbx_reflection_database"
 version = "0.2.4+roblox-504"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b41e8da85aa697cd04cef48e6dd7d96992786d2e322bafe1d3cc93045f4de1e1"
+source = "git+https://github.com/rojo-rbx/rbx-dom#4fa99b4190009f417ce63c86b2c6725e32c0090e"
 dependencies = [
  "lazy_static",
  "rbx_reflection",
@@ -1679,8 +1676,7 @@ dependencies = [
 [[package]]
 name = "rbx_types"
 version = "1.3.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d637383aa560cb675b7ea7a7778b945dab065ccc7c158f77b5455e27efadc6df"
+source = "git+https://github.com/rojo-rbx/rbx-dom#4fa99b4190009f417ce63c86b2c6725e32c0090e"
 dependencies = [
  "base64 0.11.0",
  "bitflags",
@@ -1694,8 +1690,7 @@ dependencies = [
 [[package]]
 name = "rbx_xml"
 version = "0.12.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "67387cd246cdec9251dd2451672541499ae6ce0a47c768b3ea9ee0a1becda9dd"
+source = "git+https://github.com/rojo-rbx/rbx-dom#4fa99b4190009f417ce63c86b2c6725e32c0090e"
 dependencies = [
  "base64 0.11.0",
  "log",

Cargo.toml

@@ -50,11 +50,17 @@ memofs = { version = "0.2.0", path = "crates/memofs" }
 # rbx_reflection_database = { path = "../rbx-dom/rbx_reflection_database" }
 # rbx_xml = { path = "../rbx-dom/rbx_xml" }
 
-rbx_binary = "0.6.4"
-rbx_dom_weak = "2.3.0"
-rbx_reflection = "4.2.0"
-rbx_reflection_database = "0.2.2"
-rbx_xml = "0.12.3"
+rbx_binary = { git = "https://github.com/rojo-rbx/rbx-dom" }
+rbx_dom_weak = { git = "https://github.com/rojo-rbx/rbx-dom" }
+rbx_reflection = { git = "https://github.com/rojo-rbx/rbx-dom" }
+rbx_reflection_database = { git = "https://github.com/rojo-rbx/rbx-dom" }
+rbx_xml = { git = "https://github.com/rojo-rbx/rbx-dom" }
+
+# rbx_binary = "0.6.4"
+# rbx_dom_weak = "2.3.0"
+# rbx_reflection = "4.2.0"
+# rbx_reflection_database = "0.2.2"
+# rbx_xml = "0.12.3"
 
 anyhow = "1.0.44"
 backtrace = "0.3.61"

src/change_processor.rs

@@ -291,7 +291,7 @@ fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: Ref) -> Option<
                     }
                 };
 
-                let patch_set = compute_patch_set(snapshot.as_ref(), &tree, id);
+                let patch_set = compute_patch_set(snapshot, &tree, id);
                 apply_patch_set(tree, patch_set)
             }
             Ok(None) => {
@@ -334,7 +334,7 @@ fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: Ref) -> Option<
                 }
             };
 
-            let patch_set = compute_patch_set(snapshot.as_ref(), &tree, id);
+            let patch_set = compute_patch_set(snapshot, &tree, id);
             apply_patch_set(tree, patch_set)
         }
     };

src/cli/build.rs

@@ -1,5 +1,6 @@
 use std::{
     io::{BufWriter, Write},
+    mem::ManuallyDrop,
     path::{Path, PathBuf},
 };
 
@@ -61,6 +62,14 @@ impl BuildCommand {
             }
         }
 
+        // Never drop ServeSession, because it's VERY expensive to drop and
+        // we're about to exit anyways.
+        //
+        // This is kind of evil; if this function is ever called outside of the
+        // context of a CLI, this will leak a large object forever. However, the
+        // performance benefits of leaking it outweigh the cost at this time.
+        let _session = ManuallyDrop::new(session);
+
         Ok(())
     }
 }

src/serve_session.rs

@@ -130,7 +130,7 @@ impl ServeSession {
         let snapshot = snapshot_from_vfs(&instance_context, &vfs, &start_path)?;
 
         log::trace!("Computing initial patch set");
-        let patch_set = compute_patch_set(snapshot.as_ref(), &tree, root_id);
+        let patch_set = compute_patch_set(snapshot, &tree, root_id);
 
         log::trace!("Applying initial patch set");
         apply_patch_set(&mut tree, patch_set);

src/snapshot/instance_snapshot.rs

@@ -4,7 +4,7 @@ use std::{borrow::Cow, collections::HashMap};
 
 use rbx_dom_weak::{
     types::{Ref, Variant},
-    WeakDom,
+    Instance, WeakDom,
 };
 use serde::{Deserialize, Serialize};
 
@@ -103,22 +103,28 @@ impl InstanceSnapshot {
     }
 
     #[profiling::function]
-    pub fn from_tree(tree: &WeakDom, id: Ref) -> Self {
-        let instance = tree.get_by_ref(id).expect("instance did not exist in tree");
+    pub fn from_tree(tree: WeakDom, id: Ref) -> Self {
+        let (_, mut raw_tree) = tree.into_raw();
+        Self::from_raw_tree(&mut raw_tree, id)
+    }
+
+    fn from_raw_tree(raw_tree: &mut HashMap<Ref, Instance>, id: Ref) -> Self {
+        let instance = raw_tree
+            .remove(&id)
+            .expect("instance did not exist in tree");
 
         let children = instance
             .children()
             .iter()
-            .copied()
-            .map(|id| Self::from_tree(tree, id))
+            .map(|&id| Self::from_raw_tree(raw_tree, id))
             .collect();
 
         Self {
             snapshot_id: Some(id),
             metadata: InstanceMetadata::default(),
-            name: Cow::Owned(instance.name.clone()),
-            class_name: Cow::Owned(instance.class.clone()),
-            properties: instance.properties.clone(),
+            name: Cow::Owned(instance.name),
+            class_name: Cow::Owned(instance.class),
+            properties: instance.properties,
             children,
         }
     }

src/snapshot/patch_apply.rs

@@ -1,6 +1,9 @@
 //! Defines the algorithm for applying generated patches.
 
-use std::collections::HashMap;
+use std::{
+    collections::{HashMap, HashSet},
+    mem::take,
+};
 
 use rbx_dom_weak::types::{Ref, Variant};
 
@@ -16,19 +19,28 @@ use super::{
 pub fn apply_patch_set(tree: &mut RojoTree, patch_set: PatchSet) -> AppliedPatchSet {
     let mut context = PatchApplyContext::default();
 
+    {
+        profiling::scope!("removals");
         for removed_id in patch_set.removed_instances {
             apply_remove_instance(&mut context, tree, removed_id);
         }
+    }
 
+    {
+        profiling::scope!("additions");
         for add_patch in patch_set.added_instances {
             apply_add_child(&mut context, tree, add_patch.parent_id, add_patch.instance);
         }
+    }
 
+    {
+        profiling::scope!("updates");
         // Updates need to be applied after additions, which reduces the complexity
         // of updates significantly.
         for update_patch in patch_set.updated_instances {
             apply_update_child(&mut context, tree, update_patch);
         }
+    }
 
     finalize_patch_application(context, tree)
 }
@@ -56,20 +68,9 @@ struct PatchApplyContext {
     /// eachother.
     snapshot_id_to_instance_id: HashMap<Ref, Ref>,
 
-    /// The properties of instances added by the current `PatchSet`.
-    ///
-    /// Instances added to the tree can refer to eachother via Ref properties,
-    /// but we need to make sure they're correctly transformed from snapshot
-    /// space into tree space (via `snapshot_id_to_instance_id`).
-    ///
-    /// It's not possible to do that transformation for refs that refer to added
-    /// instances until all the instances have actually been inserted into the
-    /// tree. For simplicity, we defer application of _all_ properties on added
-    /// instances instead of just Refs.
-    ///
-    /// This doesn't affect updated instances, since they're always applied
-    /// after we've added all the instances from the patch.
-    added_instance_properties: HashMap<Ref, HashMap<String, Variant>>,
+    /// Tracks all of the instances added by this patch that have refs that need
+    /// to be rewritten.
+    has_refs_to_rewrite: HashSet<Ref>,
 
     /// The current applied patch result, describing changes made to the tree.
     applied_patch_set: AppliedPatchSet,
@@ -85,23 +86,22 @@ struct PatchApplyContext {
 /// The remaining Ref properties need to be handled during patch application,
 /// where we build up a map of snapshot IDs to instance IDs as they're created,
 /// then apply properties all at once at the end.
+#[profiling::function]
 fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) -> AppliedPatchSet {
-    for (id, properties) in context.added_instance_properties {
+    for id in context.has_refs_to_rewrite {
         // This should always succeed since instances marked as added in our
         // patch should be added without fail.
         let mut instance = tree
             .get_instance_mut(id)
             .expect("Invalid instance ID in deferred property map");
 
-        for (key, mut property_value) in properties {
-            if let Variant::Ref(referent) = property_value {
+        for value in instance.properties_mut().values_mut() {
+            if let Variant::Ref(referent) = value {
                 if let Some(&instance_referent) = context.snapshot_id_to_instance_id.get(&referent)
                 {
-                    property_value = Variant::Ref(instance_referent);
+                    *value = Variant::Ref(instance_referent);
                 }
             }
-
-            instance.properties_mut().insert(key, property_value);
         }
     }
 
@@ -117,24 +117,24 @@ fn apply_add_child(
     context: &mut PatchApplyContext,
     tree: &mut RojoTree,
     parent_id: Ref,
-    snapshot: InstanceSnapshot,
+    mut snapshot: InstanceSnapshot,
 ) {
     let snapshot_id = snapshot.snapshot_id;
-    let properties = snapshot.properties;
-    let children = snapshot.children;
+    let children = take(&mut snapshot.children);
 
-    // Property application is deferred until after all children
-    // are constructed. This helps apply referents correctly.
-    let remaining_snapshot = InstanceSnapshot::new()
-        .name(snapshot.name)
-        .class_name(snapshot.class_name)
-        .metadata(snapshot.metadata)
-        .snapshot_id(snapshot.snapshot_id);
+    // If an object we're adding has a non-null referent, we'll note this
+    // instance down as needing to be revisited later.
+    let has_refs = snapshot.properties.values().any(|value| match value {
+        Variant::Ref(value) => value.is_some(),
+        _ => false,
+    });
 
-    let id = tree.insert_instance(parent_id, remaining_snapshot);
+    let id = tree.insert_instance(parent_id, snapshot);
     context.applied_patch_set.added.push(id);
 
-    context.added_instance_properties.insert(id, properties);
+    if has_refs {
+        context.has_refs_to_rewrite.insert(id);
+    }
 
     if let Some(snapshot_id) = snapshot_id {
         context.snapshot_id_to_instance_id.insert(snapshot_id, id);

src/snapshot/patch_compute.rs

@@ -1,7 +1,10 @@
 //! Defines the algorithm for computing a roughly-minimal patch set given an
 //! existing instance tree and an instance snapshot.
 
-use std::collections::{HashMap, HashSet};
+use std::{
+    collections::{HashMap, HashSet},
+    mem::take,
+};
 
 use rbx_dom_weak::types::{Ref, Variant};
 
@@ -11,11 +14,7 @@ use super::{
 };
 
 #[profiling::function]
-pub fn compute_patch_set(
-    snapshot: Option<&InstanceSnapshot>,
-    tree: &RojoTree,
-    id: Ref,
-) -> PatchSet {
+pub fn compute_patch_set(snapshot: Option<InstanceSnapshot>, tree: &RojoTree, id: Ref) -> PatchSet {
     let mut patch_set = PatchSet::new();
 
     if let Some(snapshot) = snapshot {
@@ -75,7 +74,7 @@ fn rewrite_refs_in_snapshot(context: &ComputePatchContext, snapshot: &mut Instan
 
 fn compute_patch_set_internal(
     context: &mut ComputePatchContext,
-    snapshot: &InstanceSnapshot,
+    mut snapshot: InstanceSnapshot,
     tree: &RojoTree,
     id: Ref,
     patch_set: &mut PatchSet,
@@ -88,12 +87,12 @@ fn compute_patch_set_internal(
         .get_instance(id)
         .expect("Instance did not exist in tree");
 
-    compute_property_patches(snapshot, &instance, patch_set);
-    compute_children_patches(context, snapshot, tree, id, patch_set);
+    compute_property_patches(&mut snapshot, &instance, patch_set);
+    compute_children_patches(context, &mut snapshot, tree, id, patch_set);
 }
 
 fn compute_property_patches(
-    snapshot: &InstanceSnapshot,
+    snapshot: &mut InstanceSnapshot,
     instance: &InstanceWithMeta,
     patch_set: &mut PatchSet,
 ) {
@@ -103,32 +102,32 @@ fn compute_property_patches(
     let changed_name = if snapshot.name == instance.name() {
         None
     } else {
-        Some(snapshot.name.clone().into_owned())
+        Some(take(&mut snapshot.name).into_owned())
     };
 
     let changed_class_name = if snapshot.class_name == instance.class_name() {
         None
     } else {
-        Some(snapshot.class_name.clone().into_owned())
+        Some(take(&mut snapshot.class_name).into_owned())
     };
 
     let changed_metadata = if &snapshot.metadata == instance.metadata() {
         None
     } else {
-        Some(snapshot.metadata.clone())
+        Some(take(&mut snapshot.metadata))
     };
 
-    for (name, snapshot_value) in &snapshot.properties {
-        visited_properties.insert(name.as_str());
+    for (name, snapshot_value) in take(&mut snapshot.properties) {
+        visited_properties.insert(name.clone());
 
-        match instance.properties().get(name) {
+        match instance.properties().get(&name) {
             Some(instance_value) => {
-                if snapshot_value != instance_value {
-                    changed_properties.insert(name.clone(), Some(snapshot_value.clone()));
+                if &snapshot_value != instance_value {
+                    changed_properties.insert(name, Some(snapshot_value));
                 }
             }
             None => {
-                changed_properties.insert(name.clone(), Some(snapshot_value.clone()));
+                changed_properties.insert(name, Some(snapshot_value));
             }
         }
     }
@@ -160,7 +159,7 @@ fn compute_property_patches(
 
 fn compute_children_patches(
     context: &mut ComputePatchContext,
-    snapshot: &InstanceSnapshot,
+    snapshot: &mut InstanceSnapshot,
     tree: &RojoTree,
     id: Ref,
     patch_set: &mut PatchSet,
@@ -173,7 +172,7 @@ fn compute_children_patches(
 
     let mut paired_instances = vec![false; instance_children.len()];
 
-    for snapshot_child in snapshot.children.iter() {
+    for snapshot_child in take(&mut snapshot.children) {
         let matching_instance =
             instance_children
                 .iter()
@@ -210,7 +209,7 @@ fn compute_children_patches(
             None => {
                 patch_set.added_instances.push(PatchAdd {
                     parent_id: id,
-                    instance: snapshot_child.clone(),
+                    instance: snapshot_child,
                 });
             }
         }
@@ -258,7 +257,7 @@ mod test {
             children: Vec::new(),
         };
 
-        let patch_set = compute_patch_set(Some(&snapshot), &tree, root_id);
+        let patch_set = compute_patch_set(Some(snapshot), &tree, root_id);
 
         let expected_patch_set = PatchSet {
             updated_instances: vec![PatchUpdate {
@@ -308,7 +307,7 @@ mod test {
             class_name: Cow::Borrowed("foo"),
         };
 
-        let patch_set = compute_patch_set(Some(&snapshot), &tree, root_id);
+        let patch_set = compute_patch_set(Some(snapshot), &tree, root_id);
 
         let expected_patch_set = PatchSet {
             added_instances: vec![PatchAdd {

src/snapshot/tests/compute.rs

@@ -23,7 +23,7 @@ fn set_name_and_class_name() {
         children: Vec::new(),
     };
 
-    let patch_set = compute_patch_set(Some(&snapshot), &tree, tree.get_root_id());
+    let patch_set = compute_patch_set(Some(snapshot), &tree, tree.get_root_id());
     let patch_value = redactions.redacted_yaml(patch_set);
 
     assert_yaml_snapshot!(patch_value);
@@ -47,7 +47,7 @@ fn set_property() {
         children: Vec::new(),
     };
 
-    let patch_set = compute_patch_set(Some(&snapshot), &tree, tree.get_root_id());
+    let patch_set = compute_patch_set(Some(snapshot), &tree, tree.get_root_id());
     let patch_value = redactions.redacted_yaml(patch_set);
 
     assert_yaml_snapshot!(patch_value);
@@ -78,7 +78,7 @@ fn remove_property() {
         children: Vec::new(),
     };
 
-    let patch_set = compute_patch_set(Some(&snapshot), &tree, tree.get_root_id());
+    let patch_set = compute_patch_set(Some(snapshot), &tree, tree.get_root_id());
     let patch_value = redactions.redacted_yaml(patch_set);
 
     assert_yaml_snapshot!(patch_value);
@@ -107,7 +107,7 @@ fn add_child() {
         }],
     };
 
-    let patch_set = compute_patch_set(Some(&snapshot), &tree, tree.get_root_id());
+    let patch_set = compute_patch_set(Some(snapshot), &tree, tree.get_root_id());
     let patch_value = redactions.redacted_yaml(patch_set);
 
     assert_yaml_snapshot!(patch_value);
@@ -139,7 +139,7 @@ fn remove_child() {
         children: Vec::new(),
     };
 
-    let patch_set = compute_patch_set(Some(&snapshot), &tree, tree.get_root_id());
+    let patch_set = compute_patch_set(Some(snapshot), &tree, tree.get_root_id());
     let patch_value = redactions.redacted_yaml(patch_set);
 
     assert_yaml_snapshot!(patch_value);

src/snapshot/tree.rs

@@ -87,8 +87,9 @@ impl RojoTree {
     }
 
     pub fn insert_instance(&mut self, parent_ref: Ref, snapshot: InstanceSnapshot) -> Ref {
-        let builder = InstanceBuilder::new(snapshot.class_name.to_owned())
-            .with_name(snapshot.name.to_owned())
+        let builder = InstanceBuilder::empty()
+            .with_class(snapshot.class_name.into_owned())
+            .with_name(snapshot.name.into_owned())
             .with_properties(snapshot.properties);
 
         let referent = self.inner.insert(parent_ref, builder);

src/snapshot_middleware/rbxm.rs

@@ -22,7 +22,8 @@ pub fn snapshot_rbxm(
     let children = root_instance.children();
 
     if children.len() == 1 {
-        let snapshot = InstanceSnapshot::from_tree(&temp_tree, children[0])
+        let child = children[0];
+        let snapshot = InstanceSnapshot::from_tree(temp_tree, child)
             .name(name)
             .metadata(
                 InstanceMetadata::new()

src/snapshot_middleware/rbxmx.rs

@@ -24,7 +24,8 @@ pub fn snapshot_rbxmx(
     let children = root_instance.children();
 
     if children.len() == 1 {
-        let snapshot = InstanceSnapshot::from_tree(&temp_tree, children[0])
+        let child = children[0];
+        let snapshot = InstanceSnapshot::from_tree(temp_tree, child)
             .name(name)
             .metadata(
                 InstanceMetadata::new()