Skip to content

Conversation

@BrucePay
Copy link
Collaborator

@BrucePay BrucePay commented Jun 21, 2018

PR Summary

Fix #6976.

The performance of -replace was greatly reduced when scriptblock support was added. This is because of the way the code selected what type of replacement to do. In the old code, the first choice was string. In the modified code, the last choice was string and an earlier clause used TryConvertTo() to see if the substitution object was (or could be transformed into) a MatchEvaluator object. It was this (very expensive) call to TryConvertTo() that was slowing everything down. I added a clause for strings (the most common case) to the beginning of the switch statement so it's fast again. I also added a call to PSObject.Base() to make sure the test for strings is correct.

PR Checklist

@BrucePay BrucePay requested a review from daxian-dbw June 21, 2018 19:48
Copy link
Member

@daxian-dbw daxian-dbw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@daxian-dbw daxian-dbw self-assigned this Jun 21, 2018
if (rList.Count > 1)
{
substitute = rList[1];
substitute = PSObject.Base(rList[1]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a comment why it is needed? what we break without this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, This is actually a pretty fundamental operation in PowerShell. Anywhere you receive something of type object, you have to deal with the case where it might be wrapped in a PSObject. So before you can use it as string or ScriptBlock, you have to get the base object. The PSObject.Base() method encapsulates this operation. If it's passed a PSObject, it returns the core object otherwise it just returns the object.

@iSazonov
Copy link
Collaborator

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Combining the -replace operator with a lookbehind assertion slowed down by a factor of 30+ between v6.0.2 and v6.1.0-preview.2

4 participants