From 6692a2d56fd42c1f8639d6d2deba71712eadc5f8 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Sat, 11 Apr 2026 09:09:44 -0700 Subject: [PATCH] Perf optimizations around `clone()` usage --- dsc/src/subcommand.rs | 2 +- dsc/src/util.rs | 6 +- .../src/schema_utility_extensions.rs | 64 +++++++++---------- lib/dsc-lib-registry/src/lib.rs | 6 +- lib/dsc-lib/src/dscresources/dscresource.rs | 14 ++-- lib/dsc-lib/src/functions/intersection.rs | 20 +++--- lib/dsc-lib/src/functions/lambda_helpers.rs | 14 ++-- lib/dsc-lib/src/functions/shallow_merge.rs | 4 +- lib/dsc-lib/src/functions/user_function.rs | 33 ++++++++-- 9 files changed, 92 insertions(+), 71 deletions(-) diff --git a/dsc/src/subcommand.rs b/dsc/src/subcommand.rs index 25a725b32..fc6f70370 100644 --- a/dsc/src/subcommand.rs +++ b/dsc/src/subcommand.rs @@ -819,7 +819,7 @@ pub fn list_resources( // if description, tags, or write_table is specified, pull resource manifest if it exists if let Some(ref manifest) = resource.manifest { // if description is specified, skip if resource description does not contain it - if description.is_some() && (manifest.description.is_none() | !manifest.description.clone().unwrap_or_default().to_lowercase().contains(&description.unwrap_or(&String::new()).to_lowercase())) { + if description.is_some() && (manifest.description.is_none() | !manifest.description.as_ref().unwrap_or(&String::new()).to_lowercase().contains(&description.unwrap_or(&String::new()).to_lowercase())) { continue; } diff --git a/dsc/src/util.rs b/dsc/src/util.rs index 1b3c3d81a..0ee1e66f6 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -367,10 +367,10 @@ pub fn enable_tracing(trace_level_arg: Option<&TraceLevel>, trace_format_arg: Op // command-line args override setting value, but not policy if !policy_is_used { if let Some(v) = trace_level_arg { - tracing_setting.level = v.clone(); + tracing_setting.level = *v; } if let Some(v) = trace_format_arg { - tracing_setting.format = v.clone(); + tracing_setting.format = *v; } } @@ -443,7 +443,7 @@ pub fn get_input(input: Option<&String>, file: Option<&String>) -> String { error!("{}", t!("util.inputIsFile")); exit(EXIT_INVALID_INPUT); } - input.clone() + input.to_owned() } else if let Some(path) = file { debug!("{} {path}", t!("util.readingInputFromFile")); // check if need to read from STDIN diff --git a/lib/dsc-lib-jsonschema/src/schema_utility_extensions.rs b/lib/dsc-lib-jsonschema/src/schema_utility_extensions.rs index 57fd02040..b223820ed 100644 --- a/lib/dsc-lib-jsonschema/src/schema_utility_extensions.rs +++ b/lib/dsc-lib-jsonschema/src/schema_utility_extensions.rs @@ -680,26 +680,26 @@ pub trait SchemaUtilityExtensions { //************************ $id keyword functions *************************// /// Retrieves the `$id` values for any entries in the `$defs` keyword. - /// + /// /// A compound schema resource document, also called a "bundled schema", includes referenced /// schema resources in the `$defs` keyword. A schema resource always defines the `$id` keyword. - /// + /// /// This method retrieves those IDs and returns a hashset containing the unique values. - /// + /// /// Optionally, you can recursively search for schema resource IDs to handle cases where the /// a bundled schema resource may itself have bundled resources. - /// + /// /// If the schema doesn't have any bundled schema resources, this method returns an empty /// hashset. - /// + /// /// # Examples - /// + /// /// This example demonstrates the difference between invoking the method for the top-level /// only and recursively returning nested bundled schema resources. - /// + /// /// ```rust /// use std::collections::HashSet; - /// + /// /// use schemars::json_schema; /// use serde_json::json; /// use dsc_lib_jsonschema::schema_utility_extensions::SchemaUtilityExtensions; @@ -725,7 +725,7 @@ pub trait SchemaUtilityExtensions { /// } /// } /// }); - /// + /// /// let non_recursive_result: HashSet<&str> = [ /// "https://contoso.com/schemas/example/string.json", /// "https://contoso.com/schemas/example/object.json" @@ -734,7 +734,7 @@ pub trait SchemaUtilityExtensions { /// schema.get_bundled_schema_resource_ids(false), /// non_recursive_result /// ); - /// + /// /// let recursive_result: HashSet<&str> = [ /// "https://contoso.com/schemas/example/string.json", /// "https://contoso.com/schemas/example/object.json", @@ -1474,20 +1474,20 @@ pub trait SchemaUtilityExtensions { //************************ $ref keyword functions ************************// /// Retrieves the value for every `$ref` keyword from the [`Schema`] as a [`HashSet`] of /// unique values. - /// - /// + /// + /// /// This method recurses through a given schema for the `$ref` keyword and inserts the value /// into a hashset to return. If the schema doesn't define any references, this method returns /// an empty hashset. - /// + /// /// # Examples - /// + /// /// This example shows how the method returns a unique set of references by recursively /// searching the schema for the `$ref` keyword. - /// + /// /// ```rust /// use std::collections::HashSet; - /// + /// /// use schemars::json_schema; /// use dsc_lib_jsonschema::schema_utility_extensions::SchemaUtilityExtensions; /// @@ -1499,12 +1499,12 @@ pub trait SchemaUtilityExtensions { /// "age": { "$ref": "/schemas/example/properties/age.json" }, /// }, /// }); - /// + /// /// let references: HashSet<&str> = [ /// "/schemas/example/properties/name.json", /// "/schemas/example/properties/age.json" /// ].into(); - /// + /// /// assert_eq!( /// schema.get_references(), /// references @@ -1559,7 +1559,7 @@ pub trait SchemaUtilityExtensions { /// "nestedBar": { "$ref": "#/$defs/bar" }, /// }, /// }, - /// + /// /// }, /// }); /// @@ -1576,17 +1576,17 @@ pub trait SchemaUtilityExtensions { fn get_references_to_bundled_schema_resource(&self, resource_id: &str) -> HashSet<&str>; /// Searches the schema for instances of the `$ref` keyword defined as a /// given value and replaces each instance with a new value. - /// + /// /// This method simplifies replacing references programmatically, especially /// for converting references to use the canonical ID of a bundled schema /// resource. - /// + /// /// # Examples - /// + /// /// This example replaces the reference to `#/$defs/name.json` with the /// canonical ID URI for the referenced schema resource, which is bundled /// in the schema. - /// + /// /// ```rust /// use schemars::json_schema; /// use dsc_lib_jsonschema::schema_utility_extensions::SchemaUtilityExtensions; @@ -1604,12 +1604,12 @@ pub trait SchemaUtilityExtensions { /// } /// } /// }); - /// + /// /// schema.replace_references( /// "#/$defs/name", /// "https://contoso.com/schemas/example/properties/name.json" /// ); - /// + /// /// assert_eq!( /// schema.get_references().into_iter().nth(0).unwrap(), /// "https://contoso.com/schemas/example/properties/name.json" @@ -1617,11 +1617,11 @@ pub trait SchemaUtilityExtensions { /// ``` fn replace_references(&mut self, find_value: &str, new_value: &str); /// Checks whether a given reference maps to a bundled schema resource. - /// + /// /// This method takes the value of a `$ref` keyword and searches for a matching entry in the /// `$defs` keyword. The method returns `true` if the reference resolves to an entry in /// `$defs` and otherwise false. - /// + /// /// The reference can be any of the following: /// /// - A URI identifier fragment, like `#/$defs/foo` @@ -1629,9 +1629,9 @@ pub trait SchemaUtilityExtensions { /// - A site-relative URL for the referenced schema, like `/schemas/example.json`. The function /// can only resolve site-relative URLs when the schema itself defines `$id` with an absolute /// URL, because it uses the current schema's `$id` as the base URL. - /// + /// /// # Examples - /// + /// /// ```rust /// use schemars::json_schema; /// use dsc_lib_jsonschema::schema_utility_extensions::SchemaUtilityExtensions; @@ -1645,7 +1645,7 @@ pub trait SchemaUtilityExtensions { /// } /// } /// }); - /// + /// /// // Resolving reference as pointer /// assert_eq!(schema.reference_is_for_bundled_resource("#/$defs/name"), true); /// // Resolving reference as site-relative URI @@ -1991,7 +1991,7 @@ impl SchemaUtilityExtensions for Schema { if k.as_str() == old_key { (new_key.to_string(), v.clone()) } else { - (k.clone(), v.clone()) + (std::mem::take(k), v.clone()) } }).collect(); } @@ -2148,7 +2148,7 @@ impl SchemaUtilityExtensions for Schema { let Some(def_key) = self.get_bundled_schema_resource_defs_key(resource_id) else { return HashSet::new(); }; - + let matching_references = &mut vec![ format!("#/$defs/{def_key}"), resource_id.to_string(), diff --git a/lib/dsc-lib-registry/src/lib.rs b/lib/dsc-lib-registry/src/lib.rs index 48e18affc..f48fd7e81 100644 --- a/lib/dsc-lib-registry/src/lib.rs +++ b/lib/dsc-lib-registry/src/lib.rs @@ -30,13 +30,15 @@ impl RegistryHelper { /// /// * `RegistryError` - The error that occurred. pub fn new_from_json(config: &str) -> Result { - let registry: Registry = match serde_json::from_str(config) { + let mut registry: Registry = match serde_json::from_str(config) { Ok(config) => config, Err(e) => return Err(RegistryError::Json(e)), }; - let key_path = registry.key_path.clone(); + let key_path = std::mem::take(&mut registry.key_path); let (hive, subkey) = get_hive_from_path(&key_path)?; + // Restore the key_path since we moved it out + registry.key_path = key_path; Ok( Self { config: registry, diff --git a/lib/dsc-lib/src/dscresources/dscresource.rs b/lib/dsc-lib/src/dscresources/dscresource.rs index ef2eacc78..caedad5cd 100644 --- a/lib/dsc-lib/src/dscresources/dscresource.rs +++ b/lib/dsc-lib/src/dscresources/dscresource.rs @@ -117,7 +117,7 @@ impl DscResource { } } - fn create_config_for_adapter(self, adapter: &FullyQualifiedTypeName, input: &str) -> Result { + fn create_config_for_adapter(&self, adapter: &FullyQualifiedTypeName, input: &str) -> Result { // create new configuration with adapter and use this as the resource let mut configuration = Configuration::new(); let mut property_map = Map::new(); @@ -144,7 +144,7 @@ impl DscResource { } fn invoke_get_with_adapter(&self, adapter: &FullyQualifiedTypeName, target_resource: &DscResource, filter: &str) -> Result { - let mut configurator = self.clone().create_config_for_adapter(adapter, filter)?; + let mut configurator = self.create_config_for_adapter(adapter, filter)?; let mut adapter = Self::get_adapter_resource(&mut configurator, adapter)?; if get_adapter_input_kind(&adapter)? == AdapterInputKind::Single { adapter.target_resource = Some(Box::new(target_resource.clone())); @@ -168,7 +168,7 @@ impl DscResource { } fn invoke_set_with_adapter(&self, adapter: &FullyQualifiedTypeName, target_resource: &DscResource, desired: &str, skip_test: bool, execution_type: &ExecutionKind) -> Result { - let mut configurator = self.clone().create_config_for_adapter(adapter, desired)?; + let mut configurator = self.create_config_for_adapter(adapter, desired)?; let mut adapter = Self::get_adapter_resource(&mut configurator, adapter)?; if get_adapter_input_kind(&adapter)? == AdapterInputKind::Single { adapter.target_resource = Some(Box::new(target_resource.clone())); @@ -201,7 +201,7 @@ impl DscResource { } fn invoke_test_with_adapter(&self, adapter: &FullyQualifiedTypeName, target_resource: &DscResource, expected: &str) -> Result { - let mut configurator = self.clone().create_config_for_adapter(adapter, expected)?; + let mut configurator = self.create_config_for_adapter(adapter, expected)?; let mut adapter = Self::get_adapter_resource(&mut configurator, adapter)?; if get_adapter_input_kind(&adapter)? == AdapterInputKind::Single { adapter.target_resource = Some(Box::new(target_resource.clone())); @@ -235,7 +235,7 @@ impl DscResource { } fn invoke_delete_with_adapter(&self, adapter: &FullyQualifiedTypeName, target_resource: &DscResource, filter: &str, execution_type: &ExecutionKind) -> Result { - let mut configurator = self.clone().create_config_for_adapter(adapter, filter)?; + let mut configurator = self.create_config_for_adapter(adapter, filter)?; let mut adapter = Self::get_adapter_resource(&mut configurator, adapter)?; if get_adapter_input_kind(&adapter)? == AdapterInputKind::Single { if adapter.capabilities.contains(&Capability::Delete) { @@ -250,7 +250,7 @@ impl DscResource { } fn invoke_export_with_adapter(&self, adapter: &FullyQualifiedTypeName, target_resource: &DscResource,input: &str) -> Result { - let mut configurator = self.clone().create_config_for_adapter(adapter, input)?; + let mut configurator = self.create_config_for_adapter(adapter, input)?; let mut adapter = Self::get_adapter_resource(&mut configurator, adapter)?; if get_adapter_input_kind(&adapter)? == AdapterInputKind::Single { adapter.target_resource = Some(Box::new(target_resource.clone())); @@ -280,7 +280,7 @@ impl DscResource { } fn invoke_schema_with_adapter(&self, adapter: &FullyQualifiedTypeName, target_resource: &DscResource) -> Result { - let mut configurator = self.clone().create_config_for_adapter(adapter, "")?; + let mut configurator = self.create_config_for_adapter(adapter, "")?; let mut adapter = Self::get_adapter_resource(&mut configurator, adapter)?; if get_adapter_input_kind(&adapter)? == AdapterInputKind::Single { adapter.target_resource = Some(Box::new(target_resource.clone())); diff --git a/lib/dsc-lib/src/functions/intersection.rs b/lib/dsc-lib/src/functions/intersection.rs index 292529ea3..d48bf3f65 100644 --- a/lib/dsc-lib/src/functions/intersection.rs +++ b/lib/dsc-lib/src/functions/intersection.rs @@ -30,13 +30,13 @@ impl Function for Intersection { fn invoke(&self, args: &[Value], _context: &Context) -> Result { debug!("{}", t!("functions.intersection.invoked")); - + if let Some(first_array) = args[0].as_array() { let mut result = Vec::new(); - + for item in first_array { let mut found_in_all = true; - + for arg in &args[1..] { if let Some(array) = arg.as_array() { if !array.contains(item) { @@ -47,21 +47,21 @@ impl Function for Intersection { return Err(DscError::Parser(t!("functions.intersection.invalidArgType").to_string())); } } - + if found_in_all && !result.contains(item) { result.push(item.clone()); } } - + return Ok(Value::Array(result)); } if let Some(first_object) = args[0].as_object() { let mut result = Map::new(); - + for (key, value) in first_object { let mut found_in_all = true; - + for arg in &args[1..] { if let Some(object) = arg.as_object() { if let Some(other_value) = object.get(key) { @@ -77,12 +77,12 @@ impl Function for Intersection { return Err(DscError::Parser(t!("functions.intersection.invalidArgType").to_string())); } } - + if found_in_all { - result.insert(key.clone(), value.clone()); + result.insert(key.to_string(), value.clone()); } } - + return Ok(Value::Object(result)); } diff --git a/lib/dsc-lib/src/functions/lambda_helpers.rs b/lib/dsc-lib/src/functions/lambda_helpers.rs index ec3b6561e..7d340c850 100644 --- a/lib/dsc-lib/src/functions/lambda_helpers.rs +++ b/lib/dsc-lib/src/functions/lambda_helpers.rs @@ -36,16 +36,16 @@ pub fn get_lambda<'a>( func_name: &str, ) -> Result>, DscError> { let lambdas = context.lambdas.borrow(); - + if !lambdas.contains_key(lambda_id) { return Err(DscError::Parser(t!("functions.lambdaNotFound", name = func_name, id = lambda_id).to_string())); } - + let lambda = lambdas.get(lambda_id).unwrap(); if lambda.parameters.is_empty() || lambda.parameters.len() > 2 { return Err(DscError::Parser(t!("functions.lambdaTooManyParams", name = func_name).to_string())); } - + Ok(lambdas) } @@ -78,21 +78,21 @@ where for (index, element) in array.iter().enumerate() { let mut lambda_context = context.clone(); - + lambda_context.lambda_variables.insert( - lambda.parameters[0].clone(), + lambda.parameters[0].to_string(), element.clone() ); if lambda.parameters.len() == 2 { lambda_context.lambda_variables.insert( - lambda.parameters[1].clone(), + lambda.parameters[1].to_string(), Value::Number(serde_json::Number::from(index)) ); } let result = lambda.body.invoke(&dispatcher, &lambda_context)?; - + if let Some(value) = apply(result, element)? { results.push(value); } diff --git a/lib/dsc-lib/src/functions/shallow_merge.rs b/lib/dsc-lib/src/functions/shallow_merge.rs index 1d236d3f1..6656624dc 100644 --- a/lib/dsc-lib/src/functions/shallow_merge.rs +++ b/lib/dsc-lib/src/functions/shallow_merge.rs @@ -36,10 +36,10 @@ impl Function for ShallowMerge { value = item ).to_string()) })?; - + for (key, value) in obj { // Shallow merge: replace the entire value, even if it's a nested object - result.insert(key.clone(), value.clone()); + result.insert(key.to_string(), value.clone()); } } diff --git a/lib/dsc-lib/src/functions/user_function.rs b/lib/dsc-lib/src/functions/user_function.rs index fd76a97c6..442f35a70 100644 --- a/lib/dsc-lib/src/functions/user_function.rs +++ b/lib/dsc-lib/src/functions/user_function.rs @@ -24,17 +24,36 @@ use serde_json::Value; pub fn invoke_user_function(name: &str, args: &[Value], context: &Context) -> Result { if let Some(function_definition) = context.user_functions.get(name) { validate_parameters(name, function_definition, args)?; - let mut user_context = context.clone(); - user_context.process_mode = ProcessMode::UserFunction; - // can only use its own parameters and not the global ones - user_context.parameters.clear(); - // cannot call other user functions - user_context.user_functions.clear(); + let mut user_context = Context { + process_mode: ProcessMode::UserFunction, + parameters: HashMap::new(), + user_functions: HashMap::new(), + // Copy other necessary fields by reference or shallow copy + copy: context.copy.clone(), + copy_current_loop_name: context.copy_current_loop_name.clone(), + dsc_version: context.dsc_version.clone(), + execution_type: context.execution_type, + extensions: context.extensions.clone(), + lambda_raw_args: std::cell::RefCell::new(None), + lambda_variables: HashMap::new(), + lambdas: std::cell::RefCell::new(HashMap::new()), + operation: context.operation, + outputs: context.outputs.clone(), + processing_parameter_defaults: context.processing_parameter_defaults, + process_expressions: context.process_expressions, + references: context.references.clone(), + restart_required: context.restart_required.clone(), + security_context: context.security_context.clone(), + start_datetime: context.start_datetime, + stdout: context.stdout.clone(), + system_root: context.system_root.clone(), + variables: context.variables.clone(), + }; for (i, arg) in args.iter().enumerate() { let Some(params) = &function_definition.parameters else { return Err(DscError::Parser(t!("functions.userFunction.expectedNoParameters", name = name).to_string())); }; - user_context.parameters.insert(params[i].name.clone(), (arg.clone(), params[i].r#type.clone())); + user_context.parameters.insert(params[i].name.to_string(), (arg.clone(), params[i].r#type.clone())); } let mut parser = Statement::new()?; let result = parser.parse_and_execute(&function_definition.output.value, &user_context)?;