[mono][aot] Prefer concrete instances instead of gshared ones for met… by vargaz · Pull Request #78182 · dotnet/runtime

vargaz · 2022-11-10T18:11:50Z

…hods containing type equality checks.

For example, calls to Vector.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as:

 get => (typeof(T) == typeof(byte)) ||
(typeof(T) == typeof(double)) ||

and T_BYTE could be instantiated with an enum whose basetype is byte.

Fixes some of the issues in #78163.

lambdageek · 2022-11-10T18:52:11Z

@vargaz Should we also prefer instances for op_Inequality ?

vargaz · 2022-11-10T19:01:07Z

Maybe, it wasn't a problem in practice so far.

…hods containing type equality checks. For example, calls to Vector<T>.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as: ``` get => (typeof(T) == typeof(byte)) || (typeof(T) == typeof(double)) || ``` and T_BYTE could be instantiated with an enum whose basetype is byte. Fixes some of the issues in dotnet#78163.

Fixesdotnet#78290.

Fixes#78290.

SamMonoRT · 2022-12-01T21:42:54Z

@vargaz - would this change contribute to size increase for blazor-wasm app (Interpreter) one. We are seeing size regressions in all wasm apps(New blank template, pizza app, aot-wasm, interpreter-wasm) in the same commit range.

vargaz · 2022-12-01T22:35:10Z

No, it should only affect AOT apps.

radekdoulik · 2022-12-02T10:22:20Z

That might be same regression as here https://radekdoulik.github.io/WasmPerformanceMeasurements/?startDate=2022-11-09T20%3A56%3A48.000Z&endDate=2022-11-12T14%3A17%3A58.000Z&tasks=Size

It increases managed size by 0.192 %

adiff output of corlib https://gist.github.com/radekdoulik/a7b2743a4fae5d9e6711717ea46359cd for this commit range 78322ae...3e94fdc

kotlarmilos · 2022-12-02T10:38:48Z

@radekdoulik is this main branch? There was a regression backported to .net7 in 403aa58...1c341de for pizza app.

vargaz · 2022-11-10T18:11:50Z

…hods containing type equality checks.

For example, calls to Vector.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as:

 get => (typeof(T) == typeof(byte)) ||
(typeof(T) == typeof(double)) ||

and T_BYTE could be instantiated with an enum whose basetype is byte.

Fixes some of the issues in #78163.

lambdageek · 2022-11-10T18:52:11Z

@vargaz Should we also prefer instances for op_Inequality ?

vargaz · 2022-11-10T19:01:07Z

Maybe, it wasn't a problem in practice so far.

…hods containing type equality checks. For example, calls to Vector<T>.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as: ``` get => (typeof(T) == typeof(byte)) || (typeof(T) == typeof(double)) || ``` and T_BYTE could be instantiated with an enum whose basetype is byte. Fixes some of the issues in dotnet#78163.

Fixesdotnet#78290.

Fixes#78290.

SamMonoRT · 2022-12-01T21:42:54Z

@vargaz - would this change contribute to size increase for blazor-wasm app (Interpreter) one. We are seeing size regressions in all wasm apps(New blank template, pizza app, aot-wasm, interpreter-wasm) in the same commit range.

vargaz · 2022-12-01T22:35:10Z

No, it should only affect AOT apps.

radekdoulik · 2022-12-02T10:22:20Z

That might be same regression as here https://radekdoulik.github.io/WasmPerformanceMeasurements/?startDate=2022-11-09T20%3A56%3A48.000Z&endDate=2022-11-12T14%3A17%3A58.000Z&tasks=Size

It increases managed size by 0.192 %

adiff output of corlib https://gist.github.com/radekdoulik/a7b2743a4fae5d9e6711717ea46359cd for this commit range 78322ae...3e94fdc

kotlarmilos · 2022-12-02T10:38:48Z

@radekdoulik is this main branch? There was a regression backported to .net7 in 403aa58...1c341de for pizza app.

vargaz · 2022-11-10T18:11:50Z

…hods containing type equality checks.

For example, calls to Vector.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as:

 get => (typeof(T) == typeof(byte)) ||
(typeof(T) == typeof(double)) ||

and T_BYTE could be instantiated with an enum whose basetype is byte.

Fixes some of the issues in #78163.

lambdageek · 2022-11-10T18:52:11Z

@vargaz Should we also prefer instances for op_Inequality ?

vargaz · 2022-11-10T19:01:07Z

Maybe, it wasn't a problem in practice so far.

…hods containing type equality checks. For example, calls to Vector<T>.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as: ``` get => (typeof(T) == typeof(byte)) || (typeof(T) == typeof(double)) || ``` and T_BYTE could be instantiated with an enum whose basetype is byte. Fixes some of the issues in dotnet#78163.

Fixesdotnet#78290.

Fixes#78290.

SamMonoRT · 2022-12-01T21:42:54Z

@vargaz - would this change contribute to size increase for blazor-wasm app (Interpreter) one. We are seeing size regressions in all wasm apps(New blank template, pizza app, aot-wasm, interpreter-wasm) in the same commit range.

vargaz · 2022-12-01T22:35:10Z

No, it should only affect AOT apps.

radekdoulik · 2022-12-02T10:22:20Z

That might be same regression as here https://radekdoulik.github.io/WasmPerformanceMeasurements/?startDate=2022-11-09T20%3A56%3A48.000Z&endDate=2022-11-12T14%3A17%3A58.000Z&tasks=Size

It increases managed size by 0.192 %

adiff output of corlib https://gist.github.com/radekdoulik/a7b2743a4fae5d9e6711717ea46359cd for this commit range 78322ae...3e94fdc

kotlarmilos · 2022-12-02T10:38:48Z

@radekdoulik is this main branch? There was a regression backported to .net7 in 403aa58...1c341de for pizza app.

vargaz · 2022-11-10T18:11:50Z

…hods containing type equality checks.

For example, calls to Vector.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as:

 get => (typeof(T) == typeof(byte)) ||
(typeof(T) == typeof(double)) ||

and T_BYTE could be instantiated with an enum whose basetype is byte.

Fixes some of the issues in #78163.

lambdageek · 2022-11-10T18:52:11Z

@vargaz Should we also prefer instances for op_Inequality ?

vargaz · 2022-11-10T19:01:07Z

Maybe, it wasn't a problem in practice so far.

…hods containing type equality checks. For example, calls to Vector<T>.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as: ``` get => (typeof(T) == typeof(byte)) || (typeof(T) == typeof(double)) || ``` and T_BYTE could be instantiated with an enum whose basetype is byte. Fixes some of the issues in dotnet#78163.

Fixesdotnet#78290.

Fixes#78290.

SamMonoRT · 2022-12-01T21:42:54Z

@vargaz - would this change contribute to size increase for blazor-wasm app (Interpreter) one. We are seeing size regressions in all wasm apps(New blank template, pizza app, aot-wasm, interpreter-wasm) in the same commit range.

vargaz · 2022-12-01T22:35:10Z

No, it should only affect AOT apps.

radekdoulik · 2022-12-02T10:22:20Z

That might be same regression as here https://radekdoulik.github.io/WasmPerformanceMeasurements/?startDate=2022-11-09T20%3A56%3A48.000Z&endDate=2022-11-12T14%3A17%3A58.000Z&tasks=Size

It increases managed size by 0.192 %

adiff output of corlib https://gist.github.com/radekdoulik/a7b2743a4fae5d9e6711717ea46359cd for this commit range 78322ae...3e94fdc

kotlarmilos · 2022-12-02T10:38:48Z

@radekdoulik is this main branch? There was a regression backported to .net7 in 403aa58...1c341de for pizza app.

vargaz · 2022-11-10T18:11:50Z

…hods containing type equality checks.

For example, calls to Vector.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as:

 get => (typeof(T) == typeof(byte)) ||
(typeof(T) == typeof(double)) ||

and T_BYTE could be instantiated with an enum whose basetype is byte.

Fixes some of the issues in #78163.

lambdageek · 2022-11-10T18:52:11Z

@vargaz Should we also prefer instances for op_Inequality ?

vargaz · 2022-11-10T19:01:07Z

Maybe, it wasn't a problem in practice so far.

…hods containing type equality checks. For example, calls to Vector<T>.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as: ``` get => (typeof(T) == typeof(byte)) || (typeof(T) == typeof(double)) || ``` and T_BYTE could be instantiated with an enum whose basetype is byte. Fixes some of the issues in dotnet#78163.

Fixesdotnet#78290.

Fixes#78290.

SamMonoRT · 2022-12-01T21:42:54Z

@vargaz - would this change contribute to size increase for blazor-wasm app (Interpreter) one. We are seeing size regressions in all wasm apps(New blank template, pizza app, aot-wasm, interpreter-wasm) in the same commit range.

vargaz · 2022-12-01T22:35:10Z

No, it should only affect AOT apps.

radekdoulik · 2022-12-02T10:22:20Z

That might be same regression as here https://radekdoulik.github.io/WasmPerformanceMeasurements/?startDate=2022-11-09T20%3A56%3A48.000Z&endDate=2022-11-12T14%3A17%3A58.000Z&tasks=Size

It increases managed size by 0.192 %

adiff output of corlib https://gist.github.com/radekdoulik/a7b2743a4fae5d9e6711717ea46359cd for this commit range 78322ae...3e94fdc

kotlarmilos · 2022-12-02T10:38:48Z

@radekdoulik is this main branch? There was a regression backported to .net7 in 403aa58...1c341de for pizza app.

vargaz · 2022-11-10T18:11:50Z

…hods containing type equality checks.

For example, calls to Vector.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as:

 get => (typeof(T) == typeof(byte)) ||
(typeof(T) == typeof(double)) ||

and T_BYTE could be instantiated with an enum whose basetype is byte.

Fixes some of the issues in #78163.

lambdageek · 2022-11-10T18:52:11Z

@vargaz Should we also prefer instances for op_Inequality ?

vargaz · 2022-11-10T19:01:07Z

Maybe, it wasn't a problem in practice so far.

…hods containing type equality checks. For example, calls to Vector<T>.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as: ``` get => (typeof(T) == typeof(byte)) || (typeof(T) == typeof(double)) || ``` and T_BYTE could be instantiated with an enum whose basetype is byte. Fixes some of the issues in dotnet#78163.

Fixesdotnet#78290.

Fixes#78290.

SamMonoRT · 2022-12-01T21:42:54Z

@vargaz - would this change contribute to size increase for blazor-wasm app (Interpreter) one. We are seeing size regressions in all wasm apps(New blank template, pizza app, aot-wasm, interpreter-wasm) in the same commit range.

vargaz · 2022-12-01T22:35:10Z

No, it should only affect AOT apps.

radekdoulik · 2022-12-02T10:22:20Z

That might be same regression as here https://radekdoulik.github.io/WasmPerformanceMeasurements/?startDate=2022-11-09T20%3A56%3A48.000Z&endDate=2022-11-12T14%3A17%3A58.000Z&tasks=Size

It increases managed size by 0.192 %

adiff output of corlib https://gist.github.com/radekdoulik/a7b2743a4fae5d9e6711717ea46359cd for this commit range 78322ae...3e94fdc

kotlarmilos · 2022-12-02T10:38:48Z

@radekdoulik is this main branch? There was a regression backported to .net7 in 403aa58...1c341de for pizza app.

vargaz · 2022-11-10T18:11:50Z

…hods containing type equality checks.

For example, calls to Vector.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as:

 get => (typeof(T) == typeof(byte)) ||
(typeof(T) == typeof(double)) ||

and T_BYTE could be instantiated with an enum whose basetype is byte.

Fixes some of the issues in #78163.

lambdageek · 2022-11-10T18:52:11Z

@vargaz Should we also prefer instances for op_Inequality ?

vargaz · 2022-11-10T19:01:07Z

Maybe, it wasn't a problem in practice so far.

…hods containing type equality checks. For example, calls to Vector<T>.IsSupported cannot be optimized away in a method where T is T_BYTE because its written as: ``` get => (typeof(T) == typeof(byte)) || (typeof(T) == typeof(double)) || ``` and T_BYTE could be instantiated with an enum whose basetype is byte. Fixes some of the issues in dotnet#78163.

Fixesdotnet#78290.

Fixes#78290.

SamMonoRT · 2022-12-01T21:42:54Z

@vargaz - would this change contribute to size increase for blazor-wasm app (Interpreter) one. We are seeing size regressions in all wasm apps(New blank template, pizza app, aot-wasm, interpreter-wasm) in the same commit range.

vargaz · 2022-12-01T22:35:10Z

No, it should only affect AOT apps.

radekdoulik · 2022-12-02T10:22:20Z

That might be same regression as here https://radekdoulik.github.io/WasmPerformanceMeasurements/?startDate=2022-11-09T20%3A56%3A48.000Z&endDate=2022-11-12T14%3A17%3A58.000Z&tasks=Size

It increases managed size by 0.192 %

adiff output of corlib https://gist.github.com/radekdoulik/a7b2743a4fae5d9e6711717ea46359cd for this commit range 78322ae...3e94fdc

kotlarmilos · 2022-12-02T10:38:48Z

@radekdoulik is this main branch? There was a regression backported to .net7 in 403aa58...1c341de for pizza app.

vargaz requested review from SamMonoRT and lambdageek as code owners November 10, 2022 18:11

ghost added the area-Codegen-JIT-mono label Nov 10, 2022

ghost assigned vargaz Nov 10, 2022

lambdageek approved these changes Nov 10, 2022
View reviewed changes

vargaz force-pushed the op-equal-gshared branch from 24dc1c2 to ec3a0baCompare November 10, 2022 19:02

vargaz force-pushed the op-equal-gshared branch from ec3a0ba to d4ec04eCompare November 10, 2022 20:14

build-analysisBot mentioned this pull request Nov 11, 2022
dotnet build returns exit code 1 even when the build passes #74328
Closed

Avoid an assert when compiling Vector<object> instances.
7a4195a

vargaz merged commit 486682a into dotnet:mainNov 11, 2022

vargaz deleted the op-equal-gshared branch November 11, 2022 21:15

vargaz added a commit to vargaz/runtime that referenced this pull request Nov 13, 2022
[mono] Fix some asserts caused bydotnet#78182.
ce13eec
Fixesdotnet#78290.

vargaz added a commit that referenced this pull request Nov 14, 2022
[mono] Fix some asserts caused by#78182. (#78291)
4ff7eca
Fixes#78290.

radekdoulik mentioned this pull request Nov 16, 2022
[Perf] Linux/x64: 38 Regressions on 11/12/2022 10:57:02 AM dotnet/perf-autofiling-issues#9704
Closed

kotlarmilos mentioned this pull request Nov 16, 2022
[Perf] Linux/x64: 25 Regressions on 11/2/2022 4:25:40 PM #78812
Closed

This was referenced Nov 22, 2022
[Perf] Linux/x64: 21 Improvements on 11/14/2022 9:30:20 AM dotnet/perf-autofiling-issues#10013
Open
[Perf] Linux/x64: 11 Improvements on 11/14/2022 9:30:20 AM dotnet/perf-autofiling-issues#9827
Open

kotlarmilos mentioned this pull request Nov 25, 2022
[mono][perf] iOS disk size regression on 11 Nov 2022 #78851
Closed

ivanpovazan mentioned this pull request Dec 19, 2022
MAUI iOS sample app size regression compared to .NET7 #79825
Closed

ghost locked as resolved and limited conversation to collaborators Jan 1, 2023