Use typeof() rather than caching Type in static

[benaadams]

From https://github.com/dotnet/corert/pull/7962/files#r373850627

It would be better to deleted these and convert the few places that use it to use typeof, e.g. if (type == typeof(Decimal)).

Caching these in statics is actually a deoptimization.

The JIT has optimization for operator== on System.Type that kicks in when one of the sides is typeof. It should compile this into a simple vtable compare.

/cc @jkotas

[benaadams]

@aspnet-hello benchmark

[pr-benchmarks]

Starting 'Default' pipelined plaintext benchmark with session ID 'e6f5186c786e4913a8386b101a867771'. This could take up to 30 minutes...

[pr-benchmarks]

Baseline

Starting baseline run on '6255c1ed960f5277d2e96ac2d0968c2c7e844ce2'...
RequestsPerSecond:           359,064
Max CPU (%):                 100
WorkingSet (MB):             83
Avg. Latency (ms):           6.39
Startup (ms):                425
First Request (ms):          155.72
Latency (ms):                0.46
Total Requests:              5,421,040
Duration: (ms)               15,100
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             21,006
Published Size (KB):         128,503
SDK:                         5.0.100-alpha.1.20063.3
Runtime:                     5.0.0-alpha.1.20078.2
ASP.NET Core:                5.0.0-alpha.1.20079.7

PR

Starting PR run on '8d1103bfff628a64c6c218dad87692e2bdcc4b8d'...
| Description |     RPS | CPU (%) | Memory (MB) | Avg. Latency (ms) | Startup (ms) | Build Time (ms) | Published Size (KB) | First Request (ms) | Latency (ms) | Errors | Ratio |
| ----------- | ------- | ------- | ----------- | ----------------- | ------------ | --------------- | ------------------- | ------------------ | ------------ | ------ | ----- |
|      Before | 359,064 |     100 |          83 |              6.39 |          425 |           21006 |              128503 |             155.72 |         0.46 |      0 |  1.00 |
|       After | 373,237 |      99 |          86 |              6.16 |          469 |            6504 |              128503 |             101.91 |         0.45 |      0 |  1.04 |

[benaadams]

Seems too big an improve?

[benaadams]

@aspnet-hello benchmark

[pr-benchmarks]

Starting 'Default' pipelined plaintext benchmark with session ID 'b260c8d0cf0b4e15a06b94925eb83335'. This could take up to 30 minutes...

[pr-benchmarks]

Baseline

Starting baseline run on '6255c1ed960f5277d2e96ac2d0968c2c7e844ce2'...
RequestsPerSecond:           362,308
Max CPU (%):                 100
WorkingSet (MB):             85
Avg. Latency (ms):           6.38
Startup (ms):                435
First Request (ms):          104.33
Latency (ms):                0.99
Total Requests:              5,462,790
Duration: (ms)               15,080
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             5,502
Published Size (KB):         128,503
SDK:                         5.0.100-alpha.1.20063.3
Runtime:                     5.0.0-alpha.1.20078.2
ASP.NET Core:                5.0.0-alpha.1.20079.7

PR

Starting PR run on '8d1103bfff628a64c6c218dad87692e2bdcc4b8d'...
| Description |     RPS | CPU (%) | Memory (MB) | Avg. Latency (ms) | Startup (ms) | Build Time (ms) | Published Size (KB) | First Request (ms) | Latency (ms) | Errors | Ratio |
| ----------- | ------- | ------- | ----------- | ----------------- | ------------ | --------------- | ------------------- | ------------------ | ------------ | ------ | ----- |
|      Before | 362,308 |     100 |          85 |              6.38 |          435 |            5502 |              128503 |             104.33 |         0.99 |      0 |  1.00 |
|       After | 364,301 |      99 |          86 |              6.33 |          408 |            5502 |              128503 |             104.34 |         0.52 |      0 |  1.01 |

[jkotas]

The caching in statics would help a tiny bit here, but it is probably a drop in a bucket given the total cost of the enumerator.

[davidfowl]

Nobody really calls this anyways.

[jkotas]

Have you looked at the disassembly to verify that the optimization kicks in as expected?

[jkotas]

The optimization is not as good as I though. It is not obvious whether this change makes things better.

You may want to keep the caching it statics, but use object.ReferenceEquals instead of the overloaded operator ==.

[benaadams]

The optimization is not as good as I though.

Should be resolved by dotnet/runtime#31686?

[MichalStrehovsky]

Should be resolved by dotnet/runtime#31686?

Yes, I would expect it to handle that.

[ghost]

Hello @halter73!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.