Perf optimizations around `clone()` usage by SteveL-MSFT · Pull Request #1481 · PowerShell/DSC

SteveL-MSFT · 2026-04-11T16:10:21Z

PR Summary

Some perf optimizations around clone() usage

Copilot

Pull request overview

This PR aims to reduce allocations/copies by trimming clone() usage across DSC function helpers, CLI utilities, and schema/registry helpers.

Changes:

Refactors user-function invocation to build a smaller Context instead of cloning the whole context.
Replaces some String key clones with to_string() / move-based patterns in merge/intersection/schema utilities.
Removes unnecessary DscResource cloning when creating adapter configurations; small clone-related tweaks in CLI utilities.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
lib/dsc-lib/src/functions/user_function.rs	Reworks user-function context creation to avoid cloning the entire `Context`.
lib/dsc-lib/src/functions/shallow_merge.rs	Minor key handling tweak during shallow merge insertion.
lib/dsc-lib/src/functions/lambda_helpers.rs	Uses `to_string()` for lambda parameter names during insertion.
lib/dsc-lib/src/functions/intersection.rs	Uses `to_string()` for map key insertion in object intersection.
lib/dsc-lib/src/dscresources/dscresource.rs	Avoids cloning `DscResource` when creating adapter configurators.
lib/dsc-lib-registry/src/lib.rs	Attempts to avoid cloning `key_path` by moving it out temporarily.
lib/dsc-lib-jsonschema/src/schema_utility_extensions.rs	Doc whitespace cleanup; tries to avoid key cloning during `$defs` rename.
dsc/src/util.rs	Replaces some `.clone()` with deref copies; minor string clone tweak.
dsc/src/subcommand.rs	Adjusts description filtering to avoid cloning manifest description.

Copilot · 2026-04-11T16:15:34Z

lib/dsc-lib/src/functions/user_function.rs

+        let mut user_context = Context {
+            process_mode: ProcessMode::UserFunction,
+            parameters: HashMap::new(),
+            user_functions: HashMap::new(),


invoke_user_function now builds a Context with a struct literal but uses HashMap::new() without importing std::collections::HashMap, which will fail to compile. Add the missing import (or fully-qualify std::collections::HashMap::new() everywhere).

Copilot · 2026-04-11T16:15:34Z

lib/dsc-lib/src/functions/user_function.rs

+            lambda_variables: HashMap::new(),
+            lambdas: std::cell::RefCell::new(HashMap::new()),


Behavior regression risk: previously the user-function context was context.clone() with only parameters and user_functions cleared, so user functions could still access existing lambda_variables/lambdas (e.g., via lambdaVariables() or by receiving a lambda id from the caller). The new code initializes lambda_variables and lambdas as empty, which will break user functions invoked inside lambdas or ones that operate on caller-provided lambda ids. Preserve the previous behavior by cloning these fields from context (and only clearing the fields you intend to isolate).

Suggested change

lambda_variables: HashMap::new(),

lambdas: std::cell::RefCell::new(HashMap::new()),

lambda_variables: context.lambda_variables.clone(),

lambdas: std::cell::RefCell::new(context.lambdas.borrow().clone()),

Copilot · 2026-04-11T16:15:35Z

dsc/src/subcommand.rs

+                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;


This uses the bitwise OR operator (|) on booleans, which forces evaluation of both sides and defeats short-circuiting (and any perf win from avoiding clones). Use || here, and consider restructuring to avoid repeated to_lowercase() allocations (e.g., compute the lowercased filter once when description is Some).

Suggested change

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;

if let Some(description_filter) = description {

let description_filter_lower = description_filter.to_lowercase();

let matches_description = manifest.description

.as_ref()

.map(|resource_description| resource_description.to_lowercase().contains(&description_filter_lower))

.unwrap_or(false);

if !matches_description {

continue;

}

Copilot · 2026-04-11T16:15:35Z

lib/dsc-lib/src/functions/user_function.rs

+            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,


The struct literal copies execution_type from &Context (execution_type: context.execution_type). ExecutionKind is Clone but not Copy, so this likely won’t compile; use context.execution_type.clone() instead (or make the enum Copy if that’s acceptable).

Suggested change

execution_type: context.execution_type,

execution_type: context.execution_type.clone(),

Copilot · 2026-04-11T16:15:35Z

lib/dsc-lib/src/functions/user_function.rs

+            lambda_raw_args: std::cell::RefCell::new(None),
+            lambda_variables: HashMap::new(),
+            lambdas: std::cell::RefCell::new(HashMap::new()),
+            operation: context.operation,


The struct literal copies operation from &Context (operation: context.operation). Operation/Option<Operation> are Clone but not Copy, so this likely won’t compile; use context.operation.clone().

Suggested change

operation: context.operation,

operation: context.operation.clone(),

Copilot · 2026-04-11T16:15:35Z

lib/dsc-lib/src/functions/user_function.rs

+            references: context.references.clone(),
+            restart_required: context.restart_required.clone(),
+            security_context: context.security_context.clone(),
+            start_datetime: context.start_datetime,


start_datetime: context.start_datetime moves a DateTime<Local> out of &Context, which should be a compile error because it’s not Copy. Use context.start_datetime.clone() (or otherwise derive the intended start time for the user-function context).

Suggested change

start_datetime: context.start_datetime,

start_datetime: context.start_datetime.clone(),

Copilot · 2026-04-11T16:15:36Z

lib/dsc-lib-jsonschema/src/schema_utility_extensions.rs

                (new_key.to_string(), v.clone())
            } else {
-                (k.clone(), v.clone())
+                (std::mem::take(k), v.clone())


std::mem::take(k) requires k: &mut String, but when iterating a serde_json::Map you don’t get mutable access to keys. This should be a compile error; prefer keeping k.clone() or rebuild the map by taking/replacing the whole defs map and iterating by value.

Suggested change

(std::mem::take(k), v.clone())

(k.clone(), v.clone())

Copilot · 2026-04-11T16:15:36Z

dsc/src/util.rs

        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;


TraceLevel and TraceFormat only derive Clone (not Copy), so assigning from &TraceLevel / &TraceFormat with *v won’t compile. Keep v.clone() here, or add Copy to the enum definitions if changing the public API is acceptable.

Copilot · 2026-04-11T16:15:36Z

lib/dsc-lib-registry/src/lib.rs

        };
-        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;


Using std::mem::take(&mut registry.key_path) and then restoring it adds mutation/complexity. You can avoid the clone by borrowing registry.key_path.as_str() to compute (hive, subkey) before moving registry into Self { config: registry, ... }.

Perf optimizations around clone() usage

6692a2d

Copilot AI review requested due to automatic review settings April 11, 2026 16:10

Copilot started reviewing on behalf of SteveL-MSFT April 11, 2026 16:11 View session

Copilot AI reviewed Apr 11, 2026

View reviewed changes

SteveL-MSFT marked this pull request as draft April 11, 2026 16:17

SteveL-MSFT closed this Apr 11, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Perf optimizations around `clone()` usage#1481

Perf optimizations around `clone()` usage#1481
SteveL-MSFT wants to merge 1 commit intoPowerShell:mainfrom
SteveL-MSFT:clone-optimizations

SteveL-MSFT commented Apr 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 11, 2026

Uh oh!

Copilot AI Apr 11, 2026

Uh oh!

Copilot AI Apr 11, 2026

Uh oh!

Copilot AI Apr 11, 2026

Uh oh!

Copilot AI Apr 11, 2026

Uh oh!

Copilot AI Apr 11, 2026

Uh oh!

Copilot AI Apr 11, 2026

Uh oh!

Copilot AI Apr 11, 2026

Uh oh!

Copilot AI Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		lambda_variables: HashMap::new(),
		lambdas: std::cell::RefCell::new(HashMap::new()),

		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;

-                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;
+                if let Some(description_filter) = description {
+                    let description_filter_lower = description_filter.to_lowercase();
+                    let matches_description = manifest.description
+                        .as_ref()
+                        .map(|resource_description| resource_description.to_lowercase().contains(&description_filter_lower))
+                        .unwrap_or(false);
+                    if !matches_description {
+                        continue;
+                    }

	execution_type: context.execution_type,
	execution_type: context.execution_type.clone(),

	operation: context.operation,
	operation: context.operation.clone(),

	start_datetime: context.start_datetime,
	start_datetime: context.start_datetime.clone(),

Conversation

SteveL-MSFT commented Apr 11, 2026

PR Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants