Streamline bool.TryParse/Format by stephentoub · Pull Request #64782 · dotnet/runtime

stephentoub · 2022-02-04T02:26:30Z

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

ghost · 2022-02-04T02:26:34Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

ghost · 2022-02-04T21:20:25Z

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

Author:	stephentoub
Assignees:	stephentoub
Labels:	`area-System.Runtime`
Milestone:	-

danmoseley · 2022-02-06T00:17:27Z

Do you think that TrimWhiteSpaceAndNull can be improved also? It's on the "not a boolean" path.

stephentoub · 2022-02-07T03:35:53Z

Do you think that TrimWhiteSpaceAndNull can be improved also?

I don't see anything about the implementation that can be meaningfully improved without restoring to unsafe code, at least not until the JIT recognizes some sort of backward iteration pattern for bounds-check elimination.

We can, however, push that whole trimming path off into a separate function, making it a bit cheaper to invoke the main parsing routine, along with being able to add a few more checks to more quickly weed out bad inputs.

Method	Toolchain	value	Mean	Error	Ratio
TryParse	\main\corerun.exe	True	18.4991 ns	0.0338 ns	1.00
TryParse	\pr\corerun.exe	True	9.2014 ns	0.0485 ns	0.50

TryParse	\main\corerun.exe	0	15.6529 ns	0.0662 ns	1.00
TryParse	\pr\corerun.exe	0	1.2378 ns	0.0064 ns	0.08

TryParse	\main\corerun.exe	Bogus	17.7503 ns	0.0832 ns	1.00
TryParse	\pr\corerun.exe	Bogus	6.8170 ns	0.0342 ns	0.38

TryParse	\main\corerun.exe	False	7.8471 ns	0.0274 ns	1.00
TryParse	\pr\corerun.exe	False	1.4798 ns	0.0066 ns	0.19

TryParse	\main\corerun.exe	TRUE	8.1593 ns	0.0226 ns	1.00
TryParse	\pr\corerun.exe	TRUE	0.8097 ns	0.0060 ns	0.10

TryParse	\main\corerun.exe	false	7.9468 ns	0.0555 ns	1.00
TryParse	\pr\corerun.exe	false	1.4835 ns	0.0090 ns	0.19

TryParse	\main\corerun.exe	true	7.1882 ns	0.0645 ns	1.00
TryParse	\pr\corerun.exe	true	0.8110 ns	0.0068 ns	0.11

danmoseley · 2022-02-07T03:46:53Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

stephentoub · 2022-02-07T03:50:14Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

Have you seen any real-world cases of large inputs filled with whitespace and nulls being passed to bool.{Try}Parse?

uweigand · 2022-02-09T13:54:19Z

This commit introduced serious regressions on s390x, about 40 library test cases now fail.

The problem seems to be that this (and similar lines):

BinaryPrimitives.WriteUInt64LittleEndian(MemoryMarshal.AsBytes(destination), 0x65007500720054); // "True"

is incorrect on big-endian systems. The destination memory will be set up as the following bytes:

 54 00 72 00 75 00 65 00

while on a big-endian system, the string should actually be:

 00 54 00 72 00 75 00 65

I'm currently testing a fix.

uweigand · 2022-02-09T15:06:26Z

I'm currently testing a fix.

This is now #65078

* Fix regression after #64782

EgorBo · 2022-02-15T16:23:31Z

Improvements: dotnet/perf-autofiling-issues#3529

stephentoub · 2022-02-15T16:26:02Z

Improvements

Always nice when what you saw locally is mirrored by the lab subsequently :)

stephentoub · 2022-02-04T02:26:30Z

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

ghost · 2022-02-04T02:26:34Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

ghost · 2022-02-04T21:20:25Z

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

Author:	stephentoub
Assignees:	stephentoub
Labels:	`area-System.Runtime`
Milestone:	-

danmoseley · 2022-02-06T00:17:27Z

Do you think that TrimWhiteSpaceAndNull can be improved also? It's on the "not a boolean" path.

stephentoub · 2022-02-07T03:35:53Z

Do you think that TrimWhiteSpaceAndNull can be improved also?

I don't see anything about the implementation that can be meaningfully improved without restoring to unsafe code, at least not until the JIT recognizes some sort of backward iteration pattern for bounds-check elimination.

We can, however, push that whole trimming path off into a separate function, making it a bit cheaper to invoke the main parsing routine, along with being able to add a few more checks to more quickly weed out bad inputs.

Method	Toolchain	value	Mean	Error	Ratio
TryParse	\main\corerun.exe	True	18.4991 ns	0.0338 ns	1.00
TryParse	\pr\corerun.exe	True	9.2014 ns	0.0485 ns	0.50

TryParse	\main\corerun.exe	0	15.6529 ns	0.0662 ns	1.00
TryParse	\pr\corerun.exe	0	1.2378 ns	0.0064 ns	0.08

TryParse	\main\corerun.exe	Bogus	17.7503 ns	0.0832 ns	1.00
TryParse	\pr\corerun.exe	Bogus	6.8170 ns	0.0342 ns	0.38

TryParse	\main\corerun.exe	False	7.8471 ns	0.0274 ns	1.00
TryParse	\pr\corerun.exe	False	1.4798 ns	0.0066 ns	0.19

TryParse	\main\corerun.exe	TRUE	8.1593 ns	0.0226 ns	1.00
TryParse	\pr\corerun.exe	TRUE	0.8097 ns	0.0060 ns	0.10

TryParse	\main\corerun.exe	false	7.9468 ns	0.0555 ns	1.00
TryParse	\pr\corerun.exe	false	1.4835 ns	0.0090 ns	0.19

TryParse	\main\corerun.exe	true	7.1882 ns	0.0645 ns	1.00
TryParse	\pr\corerun.exe	true	0.8110 ns	0.0068 ns	0.11

danmoseley · 2022-02-07T03:46:53Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

stephentoub · 2022-02-07T03:50:14Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

Have you seen any real-world cases of large inputs filled with whitespace and nulls being passed to bool.{Try}Parse?

uweigand · 2022-02-09T13:54:19Z

This commit introduced serious regressions on s390x, about 40 library test cases now fail.

The problem seems to be that this (and similar lines):

BinaryPrimitives.WriteUInt64LittleEndian(MemoryMarshal.AsBytes(destination), 0x65007500720054); // "True"

is incorrect on big-endian systems. The destination memory will be set up as the following bytes:

 54 00 72 00 75 00 65 00

while on a big-endian system, the string should actually be:

 00 54 00 72 00 75 00 65

I'm currently testing a fix.

uweigand · 2022-02-09T15:06:26Z

I'm currently testing a fix.

This is now #65078

* Fix regression after #64782

EgorBo · 2022-02-15T16:23:31Z

Improvements: dotnet/perf-autofiling-issues#3529

stephentoub · 2022-02-15T16:26:02Z

Improvements

Always nice when what you saw locally is mirrored by the lab subsequently :)

stephentoub · 2022-02-04T02:26:30Z

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

ghost · 2022-02-04T02:26:34Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

ghost · 2022-02-04T21:20:25Z

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

Author:	stephentoub
Assignees:	stephentoub
Labels:	`area-System.Runtime`
Milestone:	-

danmoseley · 2022-02-06T00:17:27Z

Do you think that TrimWhiteSpaceAndNull can be improved also? It's on the "not a boolean" path.

stephentoub · 2022-02-07T03:35:53Z

Do you think that TrimWhiteSpaceAndNull can be improved also?

I don't see anything about the implementation that can be meaningfully improved without restoring to unsafe code, at least not until the JIT recognizes some sort of backward iteration pattern for bounds-check elimination.

We can, however, push that whole trimming path off into a separate function, making it a bit cheaper to invoke the main parsing routine, along with being able to add a few more checks to more quickly weed out bad inputs.

Method	Toolchain	value	Mean	Error	Ratio
TryParse	\main\corerun.exe	True	18.4991 ns	0.0338 ns	1.00
TryParse	\pr\corerun.exe	True	9.2014 ns	0.0485 ns	0.50

TryParse	\main\corerun.exe	0	15.6529 ns	0.0662 ns	1.00
TryParse	\pr\corerun.exe	0	1.2378 ns	0.0064 ns	0.08

TryParse	\main\corerun.exe	Bogus	17.7503 ns	0.0832 ns	1.00
TryParse	\pr\corerun.exe	Bogus	6.8170 ns	0.0342 ns	0.38

TryParse	\main\corerun.exe	False	7.8471 ns	0.0274 ns	1.00
TryParse	\pr\corerun.exe	False	1.4798 ns	0.0066 ns	0.19

TryParse	\main\corerun.exe	TRUE	8.1593 ns	0.0226 ns	1.00
TryParse	\pr\corerun.exe	TRUE	0.8097 ns	0.0060 ns	0.10

TryParse	\main\corerun.exe	false	7.9468 ns	0.0555 ns	1.00
TryParse	\pr\corerun.exe	false	1.4835 ns	0.0090 ns	0.19

TryParse	\main\corerun.exe	true	7.1882 ns	0.0645 ns	1.00
TryParse	\pr\corerun.exe	true	0.8110 ns	0.0068 ns	0.11

danmoseley · 2022-02-07T03:46:53Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

stephentoub · 2022-02-07T03:50:14Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

Have you seen any real-world cases of large inputs filled with whitespace and nulls being passed to bool.{Try}Parse?

uweigand · 2022-02-09T13:54:19Z

This commit introduced serious regressions on s390x, about 40 library test cases now fail.

The problem seems to be that this (and similar lines):

BinaryPrimitives.WriteUInt64LittleEndian(MemoryMarshal.AsBytes(destination), 0x65007500720054); // "True"

is incorrect on big-endian systems. The destination memory will be set up as the following bytes:

 54 00 72 00 75 00 65 00

while on a big-endian system, the string should actually be:

 00 54 00 72 00 75 00 65

I'm currently testing a fix.

uweigand · 2022-02-09T15:06:26Z

I'm currently testing a fix.

This is now #65078

* Fix regression after #64782

EgorBo · 2022-02-15T16:23:31Z

Improvements: dotnet/perf-autofiling-issues#3529

stephentoub · 2022-02-15T16:26:02Z

Improvements

Always nice when what you saw locally is mirrored by the lab subsequently :)

stephentoub · 2022-02-04T02:26:30Z

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

ghost · 2022-02-04T02:26:34Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

ghost · 2022-02-04T21:20:25Z

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

Author:	stephentoub
Assignees:	stephentoub
Labels:	`area-System.Runtime`
Milestone:	-

danmoseley · 2022-02-06T00:17:27Z

Do you think that TrimWhiteSpaceAndNull can be improved also? It's on the "not a boolean" path.

stephentoub · 2022-02-07T03:35:53Z

Do you think that TrimWhiteSpaceAndNull can be improved also?

I don't see anything about the implementation that can be meaningfully improved without restoring to unsafe code, at least not until the JIT recognizes some sort of backward iteration pattern for bounds-check elimination.

We can, however, push that whole trimming path off into a separate function, making it a bit cheaper to invoke the main parsing routine, along with being able to add a few more checks to more quickly weed out bad inputs.

Method	Toolchain	value	Mean	Error	Ratio
TryParse	\main\corerun.exe	True	18.4991 ns	0.0338 ns	1.00
TryParse	\pr\corerun.exe	True	9.2014 ns	0.0485 ns	0.50

TryParse	\main\corerun.exe	0	15.6529 ns	0.0662 ns	1.00
TryParse	\pr\corerun.exe	0	1.2378 ns	0.0064 ns	0.08

TryParse	\main\corerun.exe	Bogus	17.7503 ns	0.0832 ns	1.00
TryParse	\pr\corerun.exe	Bogus	6.8170 ns	0.0342 ns	0.38

TryParse	\main\corerun.exe	False	7.8471 ns	0.0274 ns	1.00
TryParse	\pr\corerun.exe	False	1.4798 ns	0.0066 ns	0.19

TryParse	\main\corerun.exe	TRUE	8.1593 ns	0.0226 ns	1.00
TryParse	\pr\corerun.exe	TRUE	0.8097 ns	0.0060 ns	0.10

TryParse	\main\corerun.exe	false	7.9468 ns	0.0555 ns	1.00
TryParse	\pr\corerun.exe	false	1.4835 ns	0.0090 ns	0.19

TryParse	\main\corerun.exe	true	7.1882 ns	0.0645 ns	1.00
TryParse	\pr\corerun.exe	true	0.8110 ns	0.0068 ns	0.11

danmoseley · 2022-02-07T03:46:53Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

stephentoub · 2022-02-07T03:50:14Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

Have you seen any real-world cases of large inputs filled with whitespace and nulls being passed to bool.{Try}Parse?

uweigand · 2022-02-09T13:54:19Z

This commit introduced serious regressions on s390x, about 40 library test cases now fail.

The problem seems to be that this (and similar lines):

BinaryPrimitives.WriteUInt64LittleEndian(MemoryMarshal.AsBytes(destination), 0x65007500720054); // "True"

is incorrect on big-endian systems. The destination memory will be set up as the following bytes:

 54 00 72 00 75 00 65 00

while on a big-endian system, the string should actually be:

 00 54 00 72 00 75 00 65

I'm currently testing a fix.

uweigand · 2022-02-09T15:06:26Z

I'm currently testing a fix.

This is now #65078

* Fix regression after #64782

EgorBo · 2022-02-15T16:23:31Z

Improvements: dotnet/perf-autofiling-issues#3529

stephentoub · 2022-02-15T16:26:02Z

Improvements

Always nice when what you saw locally is mirrored by the lab subsequently :)

stephentoub · 2022-02-04T02:26:30Z

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

ghost · 2022-02-04T02:26:34Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

ghost · 2022-02-04T21:20:25Z

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

Author:	stephentoub
Assignees:	stephentoub
Labels:	`area-System.Runtime`
Milestone:	-

danmoseley · 2022-02-06T00:17:27Z

Do you think that TrimWhiteSpaceAndNull can be improved also? It's on the "not a boolean" path.

stephentoub · 2022-02-07T03:35:53Z

Do you think that TrimWhiteSpaceAndNull can be improved also?

I don't see anything about the implementation that can be meaningfully improved without restoring to unsafe code, at least not until the JIT recognizes some sort of backward iteration pattern for bounds-check elimination.

We can, however, push that whole trimming path off into a separate function, making it a bit cheaper to invoke the main parsing routine, along with being able to add a few more checks to more quickly weed out bad inputs.

Method	Toolchain	value	Mean	Error	Ratio
TryParse	\main\corerun.exe	True	18.4991 ns	0.0338 ns	1.00
TryParse	\pr\corerun.exe	True	9.2014 ns	0.0485 ns	0.50

TryParse	\main\corerun.exe	0	15.6529 ns	0.0662 ns	1.00
TryParse	\pr\corerun.exe	0	1.2378 ns	0.0064 ns	0.08

TryParse	\main\corerun.exe	Bogus	17.7503 ns	0.0832 ns	1.00
TryParse	\pr\corerun.exe	Bogus	6.8170 ns	0.0342 ns	0.38

TryParse	\main\corerun.exe	False	7.8471 ns	0.0274 ns	1.00
TryParse	\pr\corerun.exe	False	1.4798 ns	0.0066 ns	0.19

TryParse	\main\corerun.exe	TRUE	8.1593 ns	0.0226 ns	1.00
TryParse	\pr\corerun.exe	TRUE	0.8097 ns	0.0060 ns	0.10

TryParse	\main\corerun.exe	false	7.9468 ns	0.0555 ns	1.00
TryParse	\pr\corerun.exe	false	1.4835 ns	0.0090 ns	0.19

TryParse	\main\corerun.exe	true	7.1882 ns	0.0645 ns	1.00
TryParse	\pr\corerun.exe	true	0.8110 ns	0.0068 ns	0.11

danmoseley · 2022-02-07T03:46:53Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

stephentoub · 2022-02-07T03:50:14Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

Have you seen any real-world cases of large inputs filled with whitespace and nulls being passed to bool.{Try}Parse?

uweigand · 2022-02-09T13:54:19Z

This commit introduced serious regressions on s390x, about 40 library test cases now fail.

The problem seems to be that this (and similar lines):

BinaryPrimitives.WriteUInt64LittleEndian(MemoryMarshal.AsBytes(destination), 0x65007500720054); // "True"

is incorrect on big-endian systems. The destination memory will be set up as the following bytes:

 54 00 72 00 75 00 65 00

while on a big-endian system, the string should actually be:

 00 54 00 72 00 75 00 65

I'm currently testing a fix.

uweigand · 2022-02-09T15:06:26Z

I'm currently testing a fix.

This is now #65078

* Fix regression after #64782

EgorBo · 2022-02-15T16:23:31Z

Improvements: dotnet/perf-autofiling-issues#3529

stephentoub · 2022-02-15T16:26:02Z

Improvements

Always nice when what you saw locally is mirrored by the lab subsequently :)

stephentoub · 2022-02-04T02:26:30Z

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

ghost · 2022-02-04T02:26:34Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

ghost · 2022-02-04T21:20:25Z

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

Author:	stephentoub
Assignees:	stephentoub
Labels:	`area-System.Runtime`
Milestone:	-

danmoseley · 2022-02-06T00:17:27Z

Do you think that TrimWhiteSpaceAndNull can be improved also? It's on the "not a boolean" path.

stephentoub · 2022-02-07T03:35:53Z

Do you think that TrimWhiteSpaceAndNull can be improved also?

I don't see anything about the implementation that can be meaningfully improved without restoring to unsafe code, at least not until the JIT recognizes some sort of backward iteration pattern for bounds-check elimination.

We can, however, push that whole trimming path off into a separate function, making it a bit cheaper to invoke the main parsing routine, along with being able to add a few more checks to more quickly weed out bad inputs.

Method	Toolchain	value	Mean	Error	Ratio
TryParse	\main\corerun.exe	True	18.4991 ns	0.0338 ns	1.00
TryParse	\pr\corerun.exe	True	9.2014 ns	0.0485 ns	0.50

TryParse	\main\corerun.exe	0	15.6529 ns	0.0662 ns	1.00
TryParse	\pr\corerun.exe	0	1.2378 ns	0.0064 ns	0.08

TryParse	\main\corerun.exe	Bogus	17.7503 ns	0.0832 ns	1.00
TryParse	\pr\corerun.exe	Bogus	6.8170 ns	0.0342 ns	0.38

TryParse	\main\corerun.exe	False	7.8471 ns	0.0274 ns	1.00
TryParse	\pr\corerun.exe	False	1.4798 ns	0.0066 ns	0.19

TryParse	\main\corerun.exe	TRUE	8.1593 ns	0.0226 ns	1.00
TryParse	\pr\corerun.exe	TRUE	0.8097 ns	0.0060 ns	0.10

TryParse	\main\corerun.exe	false	7.9468 ns	0.0555 ns	1.00
TryParse	\pr\corerun.exe	false	1.4835 ns	0.0090 ns	0.19

TryParse	\main\corerun.exe	true	7.1882 ns	0.0645 ns	1.00
TryParse	\pr\corerun.exe	true	0.8110 ns	0.0068 ns	0.11

danmoseley · 2022-02-07T03:46:53Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

stephentoub · 2022-02-07T03:50:14Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

Have you seen any real-world cases of large inputs filled with whitespace and nulls being passed to bool.{Try}Parse?

uweigand · 2022-02-09T13:54:19Z

This commit introduced serious regressions on s390x, about 40 library test cases now fail.

The problem seems to be that this (and similar lines):

BinaryPrimitives.WriteUInt64LittleEndian(MemoryMarshal.AsBytes(destination), 0x65007500720054); // "True"

is incorrect on big-endian systems. The destination memory will be set up as the following bytes:

 54 00 72 00 75 00 65 00

while on a big-endian system, the string should actually be:

 00 54 00 72 00 75 00 65

I'm currently testing a fix.

uweigand · 2022-02-09T15:06:26Z

I'm currently testing a fix.

This is now #65078

* Fix regression after #64782

EgorBo · 2022-02-15T16:23:31Z

Improvements: dotnet/perf-autofiling-issues#3529

stephentoub · 2022-02-15T16:26:02Z

Improvements

Always nice when what you saw locally is mirrored by the lab subsequently :)

stephentoub · 2022-02-04T02:26:30Z

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

ghost · 2022-02-04T02:26:34Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

ghost · 2022-02-04T21:20:25Z

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

Author:	stephentoub
Assignees:	stephentoub
Labels:	`area-System.Runtime`
Milestone:	-

danmoseley · 2022-02-06T00:17:27Z

Do you think that TrimWhiteSpaceAndNull can be improved also? It's on the "not a boolean" path.

stephentoub · 2022-02-07T03:35:53Z

Do you think that TrimWhiteSpaceAndNull can be improved also?

I don't see anything about the implementation that can be meaningfully improved without restoring to unsafe code, at least not until the JIT recognizes some sort of backward iteration pattern for bounds-check elimination.

We can, however, push that whole trimming path off into a separate function, making it a bit cheaper to invoke the main parsing routine, along with being able to add a few more checks to more quickly weed out bad inputs.

Method	Toolchain	value	Mean	Error	Ratio
TryParse	\main\corerun.exe	True	18.4991 ns	0.0338 ns	1.00
TryParse	\pr\corerun.exe	True	9.2014 ns	0.0485 ns	0.50

TryParse	\main\corerun.exe	0	15.6529 ns	0.0662 ns	1.00
TryParse	\pr\corerun.exe	0	1.2378 ns	0.0064 ns	0.08

TryParse	\main\corerun.exe	Bogus	17.7503 ns	0.0832 ns	1.00
TryParse	\pr\corerun.exe	Bogus	6.8170 ns	0.0342 ns	0.38

TryParse	\main\corerun.exe	False	7.8471 ns	0.0274 ns	1.00
TryParse	\pr\corerun.exe	False	1.4798 ns	0.0066 ns	0.19

TryParse	\main\corerun.exe	TRUE	8.1593 ns	0.0226 ns	1.00
TryParse	\pr\corerun.exe	TRUE	0.8097 ns	0.0060 ns	0.10

TryParse	\main\corerun.exe	false	7.9468 ns	0.0555 ns	1.00
TryParse	\pr\corerun.exe	false	1.4835 ns	0.0090 ns	0.19

TryParse	\main\corerun.exe	true	7.1882 ns	0.0645 ns	1.00
TryParse	\pr\corerun.exe	true	0.8110 ns	0.0068 ns	0.11

danmoseley · 2022-02-07T03:46:53Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

stephentoub · 2022-02-07T03:50:14Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

Have you seen any real-world cases of large inputs filled with whitespace and nulls being passed to bool.{Try}Parse?

uweigand · 2022-02-09T13:54:19Z

This commit introduced serious regressions on s390x, about 40 library test cases now fail.

The problem seems to be that this (and similar lines):

BinaryPrimitives.WriteUInt64LittleEndian(MemoryMarshal.AsBytes(destination), 0x65007500720054); // "True"

is incorrect on big-endian systems. The destination memory will be set up as the following bytes:

 54 00 72 00 75 00 65 00

while on a big-endian system, the string should actually be:

 00 54 00 72 00 75 00 65

I'm currently testing a fix.

uweigand · 2022-02-09T15:06:26Z

I'm currently testing a fix.

This is now #65078

* Fix regression after #64782

EgorBo · 2022-02-15T16:23:31Z

Improvements: dotnet/perf-autofiling-issues#3529

stephentoub · 2022-02-15T16:26:02Z

Improvements

Always nice when what you saw locally is mirrored by the lab subsequently :)

stephentoub · 2022-02-04T02:26:30Z

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

ghost · 2022-02-04T02:26:34Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

ghost · 2022-02-04T21:20:25Z

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

privatechar[]_true=newchar[]{'T','r','u','e'};privatechar[]_false=newchar[]{'F','a','l','s','e'};privatechar[]_somethingElse=newchar[]{'O','t','h','e','r'};[Benchmark]publicboolParseTrue()=>bool.TryParse(_true,out_);[Benchmark]publicboolParseFalse()=>bool.TryParse(_false,out_);[Benchmark]publicboolParseSomethingElse()=>bool.TryParse(_somethingElse,out_);[Benchmark]publicboolFormatTrue()=>true.TryFormat(_true,out_);[Benchmark]publicboolFormatFalse()=>false.TryFormat(_false,out_);

Method	Toolchain	Mean	Error	StdDev	Ratio
ParseTrue	\main\corerun.exe	7.311 ns	0.0171 ns	0.0142 ns	1.00
ParseTrue	\pr\corerun.exe	1.149 ns	0.0243 ns	0.0203 ns	0.16

ParseFalse	\main\corerun.exe	8.749 ns	0.0569 ns	0.0504 ns	1.00
ParseFalse	\pr\corerun.exe	2.309 ns	0.0572 ns	0.0477 ns	0.26

ParseSomethingElse	\main\corerun.exe	17.638 ns	0.1478 ns	0.1382 ns	1.00
ParseSomethingElse	\pr\corerun.exe	8.219 ns	0.0511 ns	0.0453 ns	0.47

FormatTrue	\main\corerun.exe	3.117 ns	0.0245 ns	0.0204 ns	1.00
FormatTrue	\pr\corerun.exe	2.000 ns	0.0287 ns	0.0269 ns	0.64

FormatFalse	\main\corerun.exe	2.101 ns	0.0363 ns	0.0322 ns	1.00
FormatFalse	\pr\corerun.exe	2.065 ns	0.0416 ns	0.0389 ns	0.98

Author:	stephentoub
Assignees:	stephentoub
Labels:	`area-System.Runtime`
Milestone:	-

danmoseley · 2022-02-06T00:17:27Z

Do you think that TrimWhiteSpaceAndNull can be improved also? It's on the "not a boolean" path.

stephentoub · 2022-02-07T03:35:53Z

Do you think that TrimWhiteSpaceAndNull can be improved also?

I don't see anything about the implementation that can be meaningfully improved without restoring to unsafe code, at least not until the JIT recognizes some sort of backward iteration pattern for bounds-check elimination.

We can, however, push that whole trimming path off into a separate function, making it a bit cheaper to invoke the main parsing routine, along with being able to add a few more checks to more quickly weed out bad inputs.

Method	Toolchain	value	Mean	Error	Ratio
TryParse	\main\corerun.exe	True	18.4991 ns	0.0338 ns	1.00
TryParse	\pr\corerun.exe	True	9.2014 ns	0.0485 ns	0.50

TryParse	\main\corerun.exe	0	15.6529 ns	0.0662 ns	1.00
TryParse	\pr\corerun.exe	0	1.2378 ns	0.0064 ns	0.08

TryParse	\main\corerun.exe	Bogus	17.7503 ns	0.0832 ns	1.00
TryParse	\pr\corerun.exe	Bogus	6.8170 ns	0.0342 ns	0.38

TryParse	\main\corerun.exe	False	7.8471 ns	0.0274 ns	1.00
TryParse	\pr\corerun.exe	False	1.4798 ns	0.0066 ns	0.19

TryParse	\main\corerun.exe	TRUE	8.1593 ns	0.0226 ns	1.00
TryParse	\pr\corerun.exe	TRUE	0.8097 ns	0.0060 ns	0.10

TryParse	\main\corerun.exe	false	7.9468 ns	0.0555 ns	1.00
TryParse	\pr\corerun.exe	false	1.4835 ns	0.0090 ns	0.19

TryParse	\main\corerun.exe	true	7.1882 ns	0.0645 ns	1.00
TryParse	\pr\corerun.exe	true	0.8110 ns	0.0068 ns	0.11

danmoseley · 2022-02-07T03:46:53Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

stephentoub · 2022-02-07T03:50:14Z

Assuming that once we get to trimming the result will usually be a failure, I wonder whether IndexOfAny(char[] { 'e', 'E' } might allow bailing out quickly if the input is large.

Have you seen any real-world cases of large inputs filled with whitespace and nulls being passed to bool.{Try}Parse?

uweigand · 2022-02-09T13:54:19Z

This commit introduced serious regressions on s390x, about 40 library test cases now fail.

The problem seems to be that this (and similar lines):

BinaryPrimitives.WriteUInt64LittleEndian(MemoryMarshal.AsBytes(destination), 0x65007500720054); // "True"

is incorrect on big-endian systems. The destination memory will be set up as the following bytes:

 54 00 72 00 75 00 65 00

while on a big-endian system, the string should actually be:

 00 54 00 72 00 75 00 65

I'm currently testing a fix.

uweigand · 2022-02-09T15:06:26Z

I'm currently testing a fix.

This is now #65078

* Fix regression after #64782

EgorBo · 2022-02-15T16:23:31Z

Improvements: dotnet/perf-autofiling-issues#3529

stephentoub · 2022-02-15T16:26:02Z

Improvements

Always nice when what you saw locally is mirrored by the lab subsequently :)

ghost assigned stephentoub Feb 4, 2022

stephentoub mentioned this pull request Feb 4, 2022
Inline System.Boolean Parse helper methods #64771
Closed

jkotas approved these changes Feb 4, 2022
View reviewed changes

danmoseley mentioned this pull request Feb 4, 2022
add boolean parsing variations dotnet/performance#2234
Merged

danmoseley reviewed Feb 4, 2022
View reviewed changes

Comment threadsrc/libraries/System.Private.CoreLib/src/System/Boolean.cs Outdated

danmoseley reviewed Feb 4, 2022
View reviewed changes

Comment threadsrc/libraries/System.Private.CoreLib/src/System/Boolean.cs Outdated

danmoseley approved these changes Feb 4, 2022
View reviewed changes

EgorBo reviewed Feb 4, 2022
View reviewed changes

Comment threadsrc/libraries/System.Private.CoreLib/src/System/Boolean.cs Outdated

This was referenced Feb 4, 2022
Test failure: System.Security.Cryptography.X509Certificates.Tests.CertificateCreation.CertificateRequestChainTests/CreateChain_Hybrid #25979
Closed
[wasm] sgen assertion with in System.Text.RegularExpressions.Tests#64769
Closed

jeffhandley added the area-System.Runtime label Feb 4, 2022

LoopedBard3 mentioned this pull request Feb 4, 2022
add boolean parsing variations dotnet/performance#2240
Closed

stephentoub force-pushed the boolparseformat branch from 45ffc11 to c7a32eaCompare February 5, 2022 19:26

stephentoub added 2 commits February 6, 2022 22:31

Streamline bool.TryParse/Format
f34b0b2

Factor out the failure path and add length checks
1fd41cf

stephentoub force-pushed the boolparseformat branch from c7a32ea to 1fd41cfCompare February 7, 2022 03:32

radical mentioned this pull request Feb 7, 2022
[wasm] RuntimeError: null function or function signature mismatch in System.Text.RegularExpressions.Tests#64916
Closed

stephentoub merged commit d2e5183 into dotnet:mainFeb 7, 2022

stephentoub deleted the boolparseformat branch February 7, 2022 22:37

uweigand mentioned this pull request Feb 9, 2022
Fix bool.TryParse/Format on big-endian systems #65078
Merged

rzikm mentioned this pull request Feb 10, 2022
RemoteExecutor: String 'True' was not recognized as a valid Boolean. #65143
Closed

jkotas pushed a commit that referenced this pull request Feb 10, 2022
Fix bool.TryParse/Format on big-endian systems (#65078)
b2eba58
* Fix regression after #64782

ghost locked as resolved and limited conversation to collaborators Mar 17, 2022