From 273974b74fd39bbc218977887f8da670f61b14a4 Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Fri, 27 May 2022 02:54:33 -0400 Subject: [PATCH] SmallString implementation --- Cargo.lock | 64 +++++++++++++++++++++++---- Cargo.toml | 4 +- src/change_processor.rs | 1 + src/cli/build.rs | 1 + src/lib.rs | 1 + src/main.rs | 2 + src/serve_session.rs | 1 + src/small_string.rs | 1 + src/snapshot/instance_snapshot.rs | 38 +++++++++------- src/snapshot/patch.rs | 14 +++--- src/snapshot/patch_apply.rs | 25 +++++++---- src/snapshot/patch_compute.rs | 9 ++-- src/snapshot_middleware/csv.rs | 2 +- src/snapshot_middleware/json.rs | 2 +- src/snapshot_middleware/json_model.rs | 8 ++-- src/snapshot_middleware/lua.rs | 2 +- src/snapshot_middleware/meta_file.rs | 8 ++-- src/snapshot_middleware/mod.rs | 1 + src/snapshot_middleware/project.rs | 16 +++---- src/snapshot_middleware/txt.rs | 2 +- src/web/interface.rs | 9 ++-- 21 files changed, 142 insertions(+), 69 deletions(-) create mode 100644 src/small_string.rs diff --git a/Cargo.lock b/Cargo.lock index 326db2ba..b43045b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1492,6 +1492,26 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "profiling" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f61dcf0b917cd75d4521d7343d1ffff3d1583054133c9b5cbea3375c703c40d" +dependencies = [ + "profiling-procmacros", + "superluminal-perf", +] + +[[package]] +name = "profiling-procmacros" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "98eee3c112f2a6f784b6713fe1d7fb7d6506e066121c0a49371fdb976f72bae5" +dependencies = [ + "quote 1.0.18", + "syn", +] + [[package]] name = "quote" version = "0.6.13" @@ -1796,7 +1816,15 @@ dependencies = [ ] [[package]] -name = "rojo" +name = "rojo-insta-ext" +version = "0.1.0" +dependencies = [ + "serde", + "serde_yaml", +] + +[[package]] +name = "rojo-smallstring" version = "7.1.1" dependencies = [ "anyhow", @@ -1822,6 +1850,7 @@ dependencies = [ "opener", "paste", "pretty_assertions", + "profiling", "rbx_binary", "rbx_dom_weak", "rbx_reflection", @@ -1834,6 +1863,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml", + "smol_str", "tempfile", "termcolor", "thiserror", @@ -1843,14 +1873,6 @@ dependencies = [ "winreg 0.10.1", ] -[[package]] -name = "rojo-insta-ext" -version = "0.1.0" -dependencies = [ - "serde", - "serde_yaml", -] - [[package]] name = "rustc-demangle" version = "0.1.21" @@ -2015,6 +2037,15 @@ version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eb703cfe953bccee95685111adeedb76fabe4e97549a58d16f03ea7b9367bb32" +[[package]] +name = "smol_str" +version = "0.1.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7475118a28b7e3a2e157ce0131ba8c5526ea96e90ee601d9f6bb2e286a35ab44" +dependencies = [ + "serde", +] + [[package]] name = "snax" version = "0.2.0" @@ -2046,6 +2077,21 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2d67a5a62ba6e01cb2192ff309324cb4875d0c451d55fe2319433abe7a05a8ee" +[[package]] +name = "superluminal-perf" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "80ed8ddf5d2a4a849fa7dc75b3e0be740adb882fe7fee87e79584402ac9b1e60" +dependencies = [ + "superluminal-perf-sys", +] + +[[package]] +name = "superluminal-perf-sys" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0342a02bcc62538822a46f54294130677f026666c2e19d078fc213b7bc07ff16" + [[package]] name = "syn" version = "1.0.95" diff --git a/Cargo.toml b/Cargo.toml index 14b8a9ef..b2dd4dc1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "rojo" +name = "rojo-smallstring" version = "7.1.1" authors = ["Lucien Greathouse "] description = "Enables professional-grade development tools for Roblox developers" @@ -80,6 +80,8 @@ thiserror = "1.0.30" tokio = { version = "1.12.0", features = ["rt", "rt-multi-thread"] } uuid = { version = "1.0.0", features = ["v4", "serde"] } clap = { version = "3.1.18", features = ["derive"] } +smol_str = "0.1.23" +profiling = { version = "1.0.6", features = ["profile-with-superluminal"] } [target.'cfg(windows)'.dependencies] winreg = "0.10.1" diff --git a/src/change_processor.rs b/src/change_processor.rs index 7082a687..59246964 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -46,6 +46,7 @@ pub struct ChangeProcessor { impl ChangeProcessor { /// Spin up the ChangeProcessor, connecting it to the given tree, VFS, and /// outbound message queue. + #[profiling::function] pub fn start( tree: Arc>, vfs: Arc, diff --git a/src/cli/build.rs b/src/cli/build.rs index 081aef3a..24f83546 100644 --- a/src/cli/build.rs +++ b/src/cli/build.rs @@ -97,6 +97,7 @@ fn xml_encode_config() -> rbx_xml::EncodeOptions { rbx_xml::EncodeOptions::new().property_behavior(rbx_xml::EncodePropertyBehavior::WriteUnknown) } +#[profiling::function] fn write_model( session: &ServeSession, output: &Path, diff --git a/src/lib.rs b/src/lib.rs index 51195b6c..961915e2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,7 @@ mod project; mod resolution; mod serve_session; mod session_id; +mod small_string; mod snapshot; mod snapshot_middleware; mod web; diff --git a/src/main.rs b/src/main.rs index 10920bd3..a619d515 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,6 +6,8 @@ use clap::Parser; use librojo::cli::Options; fn main() { + profiling::register_thread!("Main Thread"); + panic::set_hook(Box::new(|panic_info| { // PanicInfo's payload is usually a &'static str or String. // See: https://doc.rust-lang.org/beta/std/panic/struct.PanicInfo.html#method.payload diff --git a/src/serve_session.rs b/src/serve_session.rs index a4eced44..8a0cd765 100644 --- a/src/serve_session.rs +++ b/src/serve_session.rs @@ -96,6 +96,7 @@ impl ServeSession { /// The project file is expected to be loaded out-of-band since it's /// currently loaded from the filesystem directly instead of through the /// in-memory filesystem layer. + #[profiling::function] pub fn new>(vfs: Vfs, start_path: P) -> Result { let start_path = start_path.as_ref(); let start_time = Instant::now(); diff --git a/src/small_string.rs b/src/small_string.rs new file mode 100644 index 00000000..a3b0d9be --- /dev/null +++ b/src/small_string.rs @@ -0,0 +1 @@ +pub use smol_str::SmolStr as SmallString; diff --git a/src/snapshot/instance_snapshot.rs b/src/snapshot/instance_snapshot.rs index 050dc067..6defa729 100644 --- a/src/snapshot/instance_snapshot.rs +++ b/src/snapshot/instance_snapshot.rs @@ -1,6 +1,6 @@ //! Defines the structure of an instance snapshot. -use std::{borrow::Cow, collections::HashMap}; +use std::collections::HashMap; use rbx_dom_weak::{ types::{Ref, Variant}, @@ -8,6 +8,8 @@ use rbx_dom_weak::{ }; use serde::{Deserialize, Serialize}; +use crate::small_string::SmallString; + use super::InstanceMetadata; /// A lightweight description of what an instance should look like. @@ -25,13 +27,13 @@ pub struct InstanceSnapshot { pub metadata: InstanceMetadata, /// Correpsonds to the Name property of the instance. - pub name: Cow<'static, str>, + pub name: SmallString, /// Corresponds to the ClassName property of the instance. - pub class_name: Cow<'static, str>, + pub class_name: SmallString, /// All other properties of the instance, weakly-typed. - pub properties: HashMap, + pub properties: HashMap, /// The children of the instance represented as more snapshots. /// @@ -44,37 +46,37 @@ impl InstanceSnapshot { Self { snapshot_id: None, metadata: InstanceMetadata::default(), - name: Cow::Borrowed("DEFAULT"), - class_name: Cow::Borrowed("DEFAULT"), + name: "DEFAULT".into(), + class_name: "DEFAULT".into(), properties: HashMap::new(), children: Vec::new(), } } - pub fn name(self, name: impl Into) -> Self { + pub fn name(self, name: impl Into) -> Self { Self { - name: Cow::Owned(name.into()), + name: name.into(), ..self } } - pub fn class_name(self, class_name: impl Into) -> Self { + pub fn class_name(self, class_name: impl Into) -> Self { Self { - class_name: Cow::Owned(class_name.into()), + class_name: class_name.into(), ..self } } pub fn property(mut self, key: K, value: V) -> Self where - K: Into, + K: Into, V: Into, { self.properties.insert(key.into(), value.into()); self } - pub fn properties(self, properties: impl Into>) -> Self { + pub fn properties(self, properties: impl Into>) -> Self { Self { properties: properties.into(), ..self @@ -112,12 +114,18 @@ impl InstanceSnapshot { .map(|id| Self::from_tree(tree, id)) .collect(); + let properties = instance + .properties + .iter() + .map(|(key, value)| (key.into(), value.clone())) + .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: SmallString::from(&instance.name), + class_name: SmallString::from(&instance.class), + properties, children, } } diff --git a/src/snapshot/patch.rs b/src/snapshot/patch.rs index 1d320a2c..e888d519 100644 --- a/src/snapshot/patch.rs +++ b/src/snapshot/patch.rs @@ -5,6 +5,8 @@ use std::collections::HashMap; use rbx_dom_weak::types::{Ref, Variant}; use serde::{Deserialize, Serialize}; +use crate::small_string::SmallString; + use super::{InstanceMetadata, InstanceSnapshot}; /// A set of different kinds of patches that can be applied to an WeakDom. @@ -40,12 +42,12 @@ pub struct PatchAdd { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct PatchUpdate { pub id: Ref, - pub changed_name: Option, - pub changed_class_name: Option, + pub changed_name: Option, + pub changed_class_name: Option, /// Contains all changed properties. If a property is assigned to `None`, /// then that property has been removed. - pub changed_properties: HashMap>, + pub changed_properties: HashMap>, /// Changed Rojo-specific metadata, if any of it changed. pub changed_metadata: Option, @@ -83,9 +85,9 @@ pub struct AppliedPatchUpdate { pub id: Ref, // TODO: Store previous values in order to detect application conflicts - pub changed_name: Option, - pub changed_class_name: Option, - pub changed_properties: HashMap>, + pub changed_name: Option, + pub changed_class_name: Option, + pub changed_properties: HashMap>, pub changed_metadata: Option, } diff --git a/src/snapshot/patch_apply.rs b/src/snapshot/patch_apply.rs index 4e40061c..db00fa75 100644 --- a/src/snapshot/patch_apply.rs +++ b/src/snapshot/patch_apply.rs @@ -4,6 +4,8 @@ use std::collections::HashMap; use rbx_dom_weak::types::{Ref, Variant}; +use crate::small_string::SmallString; + use super::{ patch::{AppliedPatchSet, AppliedPatchUpdate, PatchSet, PatchUpdate}, InstanceSnapshot, RojoTree, @@ -12,6 +14,7 @@ use super::{ /// Consumes the input `PatchSet`, applying all of its prescribed changes to the /// tree and returns an `AppliedPatchSet`, which can be used to keep another /// tree in sync with Rojo's. +#[profiling::function] pub fn apply_patch_set(tree: &mut RojoTree, patch_set: PatchSet) -> AppliedPatchSet { let mut context = PatchApplyContext::default(); @@ -68,7 +71,7 @@ struct PatchApplyContext { /// /// 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>, + added_instance_properties: HashMap>, /// The current applied patch result, describing changes made to the tree. applied_patch_set: AppliedPatchSet, @@ -100,7 +103,9 @@ fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) - } } - instance.properties_mut().insert(key, property_value); + instance + .properties_mut() + .insert(key.to_string(), property_value); } } @@ -164,13 +169,13 @@ fn apply_update_child(context: &mut PatchApplyContext, tree: &mut RojoTree, patc }; if let Some(name) = patch.changed_name { - *instance.name_mut() = name.clone(); - applied_patch.changed_name = Some(name); + *instance.name_mut() = name.to_string(); + applied_patch.changed_name = Some(name.into()); } if let Some(class_name) = patch.changed_class_name { - *instance.class_name_mut() = class_name.clone(); - applied_patch.changed_class_name = Some(class_name); + *instance.class_name_mut() = class_name.to_string(); + applied_patch.changed_class_name = Some(class_name.into()); } for (key, property_entry) in patch.changed_properties { @@ -195,13 +200,15 @@ fn apply_update_child(context: &mut PatchApplyContext, tree: &mut RojoTree, patc instance .properties_mut() - .insert(key.clone(), Variant::Ref(new_referent)); + .insert(key.to_string(), Variant::Ref(new_referent)); } Some(ref value) => { - instance.properties_mut().insert(key.clone(), value.clone()); + instance + .properties_mut() + .insert(key.to_string(), value.clone()); } None => { - instance.properties_mut().remove(&key); + instance.properties_mut().remove(key.as_str()); } } diff --git a/src/snapshot/patch_compute.rs b/src/snapshot/patch_compute.rs index 76f0b26c..7f5c6a29 100644 --- a/src/snapshot/patch_compute.rs +++ b/src/snapshot/patch_compute.rs @@ -10,6 +10,7 @@ use super::{ InstanceSnapshot, InstanceWithMeta, RojoTree, }; +#[profiling::function] pub fn compute_patch_set( snapshot: Option<&InstanceSnapshot>, tree: &RojoTree, @@ -102,13 +103,13 @@ fn compute_property_patches( let changed_name = if snapshot.name == instance.name() { None } else { - Some(snapshot.name.clone().into_owned()) + Some(snapshot.name.clone()) }; let changed_class_name = if snapshot.class_name == instance.class_name() { None } else { - Some(snapshot.class_name.clone().into_owned()) + Some(snapshot.class_name.clone()) }; let changed_metadata = if &snapshot.metadata == instance.metadata() { @@ -120,7 +121,7 @@ fn compute_property_patches( for (name, snapshot_value) in &snapshot.properties { visited_properties.insert(name.as_str()); - match instance.properties().get(name) { + match instance.properties().get(name.as_str()) { Some(instance_value) => { if snapshot_value != instance_value { changed_properties.insert(name.clone(), Some(snapshot_value.clone())); @@ -137,7 +138,7 @@ fn compute_property_patches( continue; } - changed_properties.insert(name.clone(), None); + changed_properties.insert(name.into(), None); } if changed_properties.is_empty() diff --git a/src/snapshot_middleware/csv.rs b/src/snapshot_middleware/csv.rs index a8f27180..a49949f8 100644 --- a/src/snapshot_middleware/csv.rs +++ b/src/snapshot_middleware/csv.rs @@ -30,7 +30,7 @@ pub fn snapshot_csv( .name(name) .class_name("LocalizationTable") .properties(hashmap! { - "Contents".to_owned() => table_contents.into(), + "Contents".into() => table_contents.into(), }) .metadata( InstanceMetadata::new() diff --git a/src/snapshot_middleware/json.rs b/src/snapshot_middleware/json.rs index 8c7f369e..744af96d 100644 --- a/src/snapshot_middleware/json.rs +++ b/src/snapshot_middleware/json.rs @@ -25,7 +25,7 @@ pub fn snapshot_json( let as_lua = json_to_lua(value).to_string(); let properties = hashmap! { - "Source".to_owned() => as_lua.into(), + "Source".into() => as_lua.into(), }; let meta_path = path.with_file_name(format!("{}.meta.json", name)); diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index 52071031..a69a4150 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, collections::HashMap, path::Path, str}; +use std::{collections::HashMap, path::Path, str}; use anyhow::Context; use memofs::Vfs; @@ -85,14 +85,14 @@ impl JsonModelCore { let mut properties = HashMap::with_capacity(self.properties.len()); for (key, unresolved) in self.properties { let value = unresolved.resolve(&class_name, &key)?; - properties.insert(key, value); + properties.insert(key.into(), value); } Ok(InstanceSnapshot { snapshot_id: None, metadata: Default::default(), - name: Cow::Owned(name), - class_name: Cow::Owned(class_name), + name: name.into(), + class_name: class_name.into(), properties, children, }) diff --git a/src/snapshot_middleware/lua.rs b/src/snapshot_middleware/lua.rs index 02bbc51d..9871f766 100644 --- a/src/snapshot_middleware/lua.rs +++ b/src/snapshot_middleware/lua.rs @@ -38,7 +38,7 @@ pub fn snapshot_lua( .name(instance_name) .class_name(class_name) .properties(hashmap! { - "Source".to_owned() => contents_str.into(), + "Source".into() => contents_str.into(), }) .metadata( InstanceMetadata::new() diff --git a/src/snapshot_middleware/meta_file.rs b/src/snapshot_middleware/meta_file.rs index 2715dbbe..f3485afb 100644 --- a/src/snapshot_middleware/meta_file.rs +++ b/src/snapshot_middleware/meta_file.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, collections::HashMap, path::PathBuf}; +use std::{collections::HashMap, path::PathBuf}; use anyhow::{format_err, Context}; use serde::{Deserialize, Serialize}; @@ -49,7 +49,7 @@ impl AdjacentMetadata { .resolve(&snapshot.class_name, &key) .with_context(|| format!("error applying meta file {}", path.display()))?; - snapshot.properties.insert(key, value); + snapshot.properties.insert(key.into(), value); } Ok(()) @@ -116,7 +116,7 @@ impl DirectoryMetadata { )); } - snapshot.class_name = Cow::Owned(class_name); + snapshot.class_name = class_name.into(); } Ok(()) @@ -136,7 +136,7 @@ impl DirectoryMetadata { .resolve(&snapshot.class_name, &key) .with_context(|| format!("error applying meta file {}", path.display()))?; - snapshot.properties.insert(key, value); + snapshot.properties.insert(key.into(), value); } Ok(()) diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 36729f8f..a8b348ee 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -40,6 +40,7 @@ pub use self::project::snapshot_project_node; /// The main entrypoint to the snapshot function. This function can be pointed /// at any path and will return something if Rojo knows how to deal with it. +#[profiling::function] pub fn snapshot_from_vfs( context: &InstanceContext, vfs: &Vfs, diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index 5650c99a..7293297a 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -6,6 +6,7 @@ use rbx_reflection::ClassTag; use crate::{ project::{PathNode, Project, ProjectNode}, + small_string::SmallString, snapshot::{ InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource, PathIgnoreRule, }, @@ -67,13 +68,10 @@ pub fn snapshot_project_node( ) -> anyhow::Result> { let project_folder = project_path.parent().unwrap(); - let class_name_from_project = node - .class_name - .as_ref() - .map(|name| Cow::Owned(name.clone())); + let class_name_from_project = node.class_name.as_ref().map(|name| SmallString::from(name)); let mut class_name_from_path = None; - let name = Cow::Owned(instance_name.to_owned()); + let name = SmallString::from(instance_name); let mut properties = HashMap::new(); let mut children = Vec::new(); let mut metadata = InstanceMetadata::default(); @@ -228,7 +226,7 @@ pub fn snapshot_project_node( _ => {} } - properties.insert(key.clone(), value); + properties.insert(key.into(), value); } // If the user specified $ignoreUnknownInstances, overwrite the existing @@ -262,7 +260,7 @@ pub fn snapshot_project_node( })) } -fn infer_class_name(name: &str, parent_class: Option<&str>) -> Option> { +fn infer_class_name(name: &str, parent_class: Option<&str>) -> Option { // If className wasn't defined from another source, we may be able // to infer one. @@ -275,13 +273,13 @@ fn infer_class_name(name: &str, parent_class: Option<&str>) -> Option contents_str.into(), + "Value".into() => contents_str.into(), }; let meta_path = path.with_file_name(format!("{}.meta.json", name)); diff --git a/src/web/interface.rs b/src/web/interface.rs index 28005057..abb1f02d 100644 --- a/src/web/interface.rs +++ b/src/web/interface.rs @@ -12,6 +12,7 @@ use serde::{Deserialize, Serialize}; use crate::{ session_id::SessionId, + small_string::SmallString, snapshot::{ AppliedPatchSet, InstanceMetadata as RojoInstanceMetadata, InstanceWithMeta, RojoTree, }, @@ -83,13 +84,13 @@ impl<'a> SubscribeMessage<'a> { #[serde(rename_all = "camelCase")] pub struct InstanceUpdate { pub id: Ref, - pub changed_name: Option, - pub changed_class_name: Option, + pub changed_name: Option, + pub changed_class_name: Option, - // TODO: Transform from HashMap> to something else, since + // TODO: Transform from HashMap<_, Option<_>> to something else, since // null will get lost when decoding from JSON in some languages. #[serde(default)] - pub changed_properties: HashMap>, + pub changed_properties: HashMap>, pub changed_metadata: Option, }