Slow function when parameter is Mandatory its Type is specified

Steps to reproduce

[Byte[]]$Value = [System.IO.File]::ReadAllBytes('C:\Some10MBfile')

#  ~1000ms for 10MB array
#  ~0.5ms for 1byte array
function MandType {
    Param (
        [parameter(Mandatory=$True)]
        [Byte[]]$Value
    )
}

#  ~0.3ms for 10MB array
function TypeOnly {
    Param (
        [parameter()]
        [Byte[]]$Value
    )
}

#  ~0.3 for 10MB array
function MandOnly {
    Param (
        [parameter(Mandatory=$True)]
        $Value
    )
}

(Measure-Command { MandType $Value}).TotalMilliseconds
(Measure-Command { MandType ([Byte[]]@(0))}).TotalMilliseconds
(Measure-Command { TypeOnly $Value}).TotalMilliseconds
(Measure-Command { MandOnly $Value}).TotalMilliseconds

Expected behavior

All versions of the above functions should behave identically and complete immediately, regardless of of parameter size as the functions perform no functions.

1.1
1.1
1.1
1.1

Actual behavior

When setting a parameter to Mandatory, specifying its Type, and passing a large array in, the function will pause for some amount of time before starting.

Environment data

PS C:\> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      6.0.0-beta.9
PSEdition                      Core
GitCommitId                    v6.0.0-beta.9
OS                             Microsoft Windows 10.0.16299
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0


PS C:\> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.16299.19
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.16299.19
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

I can repro this in Core on Windows, macOS, and Linux. Very odd.

also, just to be thorough I tested with function below just to see if there was a difference between explicit or implicit mandatory=$false and did not see the same issue.

function NoMandType {
    Param (
        [parameter(Mandatory=$false)]
        [Byte[]]$Value
    )
}

Mandatory causes the parameter binder to check for NULL elements in an array argument (code here), which causes this perf issue.

It also happens when the [ValidateNotNull] attribute is applied to the parameter.

function NotNulOrEmtpy {
     Param (
         [ValidateNotNull()]
         [Byte[]]$Value
     )
}
## Long delay
NotNulOrEmtpy -Value $Value

The parameter binder shouldn't blindly check null elements. In the case of this repro, the element type of the argument array is value type, and thus cannot have null element.

Dongbo Wang (@daxian-dbw)

Dongbo Wang (@daxian-dbw) Do we use new label bug instead of Issue-Bug?

What should the attributes be applied to? Argument or its elements? Collection or its elements?

IMO, [ValidateNotNull()] and [ValidateNotNullorEmpty()] would apply to the collection, not its elements... but... erm... maybe not all types. like [String[]] for example.. that one I could see users assuming it applies to elements. but for [Bytes[]] I would expect that to work on the collection, not the elements..

We could disable recursion here to set last parameter in false and in ValidateNullOrEmptyArgument add empty collection check.

For ValueFromPipeline arguments, it's important that Validate... applies to members of the collection because the recommended pattern is to always accept a collection so that the following work similarly:

$collection | Do-Something
Do-Something -InputObject $collection

It would be a little weird if Validate... did not apply to members of a collection if the parameter were not ValueFromPipeline.

I'd expect that we convert Do-Something -InputObject $collection to $collection | Do-Something if InputObject is a scalar and that we apply Validate... to members in this case, otherwise if InputObject is a collection we apply Validate... to the collection.

Ilya (@iSazonov)

Ilya (@iSazonov) - do you mean InputObject is typed as a collection, or that the value is a collection?

I believe conversions are performed before validation - as this example demonstrates:

class MyThing { [string]ToString() { return "" } }
function foo {
    param(
        [ValidateNotNullOrEmpty()]
        [Parameter(Mandatory, ValueFromPipeline)]
        [string[]]$InputObject)
    process { $InputObject.Count }
}
foo -InputObject ([MyThing]::new(),[MyThing]::new())

Because you git this error - showing clearly the conversion happened and validation failed on the empty strings:

foo : Cannot validate argument on parameter 'InputObject'. The argument is null or empty. Provide an argument that is
not null or empty, and then try the command again.
At line:1 char:18
+ foo -InputObject ([MyThing]::new(),[MyThing]::new())
+                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (:) [foo], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationError,foo

So when using the usual pattern for pipeline parameters - use a strongly typed collection for InputObject, it doesn't matter if you've passed a scalar or collection, the value will have been converted to a collection before validating.

I think we should skip checking elements for collections with a value-type element type. For collections with a value-type element type, the element cannot be $null, so in this case, just check if the collection is empty would be sufficient.

Jason Shirk (@lzybkr)

Jason Shirk (@lzybkr)
My thoughts was about:

function foo1 {
    param(
        [ValidateNotNullOrEmpty()]
        [Parameter(ValueFromPipeline)]
        [string[]]$InputObject)
    process { $InputObject.Count }
}

function foo2 {
    param(
        [ValidateNotNullOrEmpty()]
        [Parameter(ValueFromPipeline)]
        [string]$InputObject)
    process { $InputObject.Count }
}

foo1 -InputObject $scalar
$scalar | foo1
foo1 -InputObject $collection
$collection | foo1

foo2 -InputObject $scalar
$scalar | foo2
foo2 -InputObject $collection
$collection | foo2

- if we convert foo -InputObject $varto$var | foowe could use one code path (to work precisely similarly) and eliminate the internal cycle inProcessRecord()forInputObject`.

Jason Shirk (@lzybkr)

Jason Shirk (@lzybkr) Dongbo Wang (@daxian-dbw) We have an inconsistence with ValueFromPipeline arguments.

function MandType1 {
    Param (
        [parameter(Mandatory=$true, ValueFromPipeline)]
        [Byte[]]$Value
    )
}

function MandType2 {
    Param (
        [parameter(Mandatory=$true, ValueFromPipeline)]
        $Value
    )
}

# Validate every element in array
MandType1 -Value $Value
$Value | MandType1 

# Validate only whole $Value as Object
MandType2 -Value $Value
# Validate every element in array
$Value | MandType2

The not-null-or-empty check of a mandatory parameter is based on the parameter type, while the check for ValidateNotNullOrEmptyAttribute or ValidateNotNullAttribute (and other validate attributes too) are based on the argument only (the attribute itself doesn't have the information about the type of the parameter/variable/property it applies to).

I believe we want "works similarly", don't we?

Navigation Menu

Repository navigation

Slow function when parameter is Mandatory its Type is specified #5417

Steps to reproduce

Expected behavior

Actual behavior

Environment data

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Slow function when parameter is Mandatory its Type is specified #5417

Description

Steps to reproduce

Expected behavior

Actual behavior

Environment data

Activity

markekraus commented on Nov 11, 2017

daxian-dbw commented on Nov 11, 2017

iSazonov commented on Nov 11, 2017

iSazonov commented on Nov 11, 2017

markekraus commented on Nov 11, 2017

iSazonov commented on Nov 11, 2017

lzybkr commented on Nov 11, 2017

iSazonov commented on Nov 11, 2017

lzybkr commented on Nov 11, 2017

daxian-dbw commented on Nov 12, 2017

iSazonov commented on Nov 14, 2017

iSazonov commented on Nov 14, 2017

daxian-dbw commented on Nov 14, 2017

iSazonov commented on Nov 15, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions