[mono] remove 2 extra members from System.Object #44081

lambdageek · 2020-10-30T18:59:48Z

move GetRawData to an extension method in RuntimeHelpers and fixup the intrinsics
replace Object.CloneInternal by MarshalAsAttribute.CloneInternal at its one use site
re-enable System.Reflection.Tests.TypeInfoTests.FindMembers and fix [netcore] Make System.Reflection.Tests.TypeInfoTests.FindMembers Pass mono/mono#15029

Dotnet-GitSync-Bot · 2020-10-30T18:59:51Z

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.

Make it into an extension method like CoreCLR.

CoffeeFlux · 2020-10-30T20:40:21Z

Relevant: mono/mono#17295

CoffeeFlux · 2020-10-30T20:40:59Z

Also related: mono/mono#15029

vargaz · 2020-10-30T22:35:46Z

This might depend on optimizations that are missing in the mono JIT/interpreter to get the same performance.

src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs

stephentoub

Yay sharing more stuff.

CoffeeFlux · 2020-10-31T03:24:37Z

Filip seemed to think the codegen ended up being the same, but I'd like to be sure of that before we merge this.

lambdageek · 2020-11-02T17:36:33Z

Zoltan:

This might depend on optimizations that are missing in the mono JIT/interpreter to get the same performance.
Ryan:
Filip seemed to think the codegen ended up being the same, but I'd like to be sure of that before we merge this.

So I think on the JIT this is ok - there's an extra null check compared to the intrinsic, but as Filip mentioned it would get eliminated if the extension method is inlined.

On the other hand, in the interpreter the null check isn't done in the transform, it's done in the interpretation of MINT_LDFLDA, so it seems like there's an extra cost here.

So maybe this is a bad idea after all. One thing we can do is move the method to be an extension method in RuntimeHelpers.Mono.cs, but leave it as an intrinsic. We wouldn't get as much sharing with CoreCLR, but at least we'd do something about mono/mono#15029

BrzVlad · 2020-11-02T17:38:44Z

@lambdageek so the interpreter only generated a MINT_LDFLDA instruction for this ?

vargaz · 2020-11-02T19:29:36Z

Imho, moving these small pieces of low level code to shared is problematic in general, they do depend on implementation details in the runtime.

This reverts commit f45059f.

…cation" This reverts commit 6a8bc4d.

But keep it as an intrinsic

lambdageek · 2020-11-02T20:07:11Z

Ok, I'm going to retool this PR to:

move GetRawData to an extension method in RuntimeHelpers and fixup the intrinsics
replace Object.CloneInternal by MarshalAsAttribute.CloneInternal at its one use site
re-enable System.Reflection.Tests.TypeInfoTests.FindMembers and fix [netcore] Make System.Reflection.Tests.TypeInfoTests.FindMembers Pass mono/mono#15029

there was only one use

reworked PR

CoffeeFlux

Much happier with this approach, thanks!

LGTM, but hopefully Vlad can take a look at the interpreter changes.

BrzVlad · 2020-11-02T20:54:49Z

@lambdageek now that you brought the idea up, you could use MINT_LDFLDA_UNSAFE instead on the interpreter for better perf. I can add it in a separate PR though, if you feel it's unrelated.

lambdageek · 2020-11-02T21:09:33Z

I think I'm ok with the PR as is.

I think Zoltan makes a compelling argument that we shouldn't try to share low-level hacks like the Unsafe.As<RawData> implementation of GetRawData.

Fixes mono/mono#15030 and fixes mono/mono#15031 and fixes mono/mono#15032

author Stephen Toub <[email protected]> 1604601164 -0500 committer Tammy Qiu <[email protected]> 1604960878 -0500 Add stream conformance tests for TranscodingStream (dotnet#44248) * Add stream conformance tests for TranscodingStream * Special-case 0-length input buffers to TranscodingStream.Write{Async} The base implementation of Encoder.Convert doesn't like empty inputs. Regardless, if the input is empty, we can avoid a whole bunch of unnecessary work. JIT: minor inliner refactoring (dotnet#44215) Extract out the budget check logic so it can vary by inlining policy. Use this to exempt the FullPolicy from budget checking. Fix inline xml to dump the proper (full name) hash for inlinees. Update range dumper to dump ranges in hex. Remove unused QCall for WinRTSupported (dotnet#44278) ConcurrentQueueSegment allows spinning threads to sleep. (dotnet#44265) * Allow threads to sleep when ConcurrentQueue has many enqueuers/dequeuers. * Update src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueueSegment.cs Co-authored-by: Stephen Toub <[email protected]> * Apply suggestions from code review Co-authored-by: Stephen Toub <[email protected]> Co-authored-by: AMD DAYTONA EPYC <[email protected]> Co-authored-by: Stephen Toub <[email protected]> File.Exists() is not null when true (dotnet#44310) * File.Exists() is not null when true * Fix compile * Fix compile 2 [master][watchOS] Add simwatch64 support (dotnet#44303) Xcode 12.2 removed 32 bits support for watchOS simulators, this PR helps to fix xamarin/xamarin-macios#9949, we have tested the new binaries and they are working as expected ![unknown](https://user-images.githubusercontent.com/204671/98253709-64413200-1f49-11eb-9774-8c5aa416fc57.png) Co-authored-by: dalexsoto <[email protected]> Implementing support to Debugger::Break. (dotnet#44305) Set fgOptimizedFinally flag correctly (dotnet#44268) - Initialize to 0 at compiler startup - Set flag when finally cloning optimization kicks in Fixes non-deterministic generation of nop opcodes into ARM32 code Forbid `- byref cnst` -> `+ (byref -cnst)` transformation. (dotnet#44266) * Add a repro test. * Forbid the transformation for byrefs. * Update src/coreclr/src/jit/morph.cpp Co-authored-by: Andy Ayers <[email protected]> * Update src/coreclr/src/jit/morph.cpp * Fix the test return value. WriteLine is just to make sure we don't delete the value. * improve the test. avoid a possible overflow and don't waste time on printing. Co-authored-by: Andy Ayers <[email protected]> Pick libmonosgen-2.0.so from cmake install directory instead of .libs (dotnet#44291) This aligns Linux with what we already do for all the other platforms. Update SharedPerformanceCounter assert (dotnet#44333) Remove silly ToString in GetCLRInstanceString (dotnet#44335) Use targetPlatformMoniker for net5.0 and newer tfms (dotnet#43965) * Use targetPlatformMoniker for net5.0 and newer tfms * disabling analyzer, update version to 0.0, and use new format. * update the targetFramework.sdk * removing supportedOS assembly level attribute * fix linker errors and addressing feedback * making _TargetFrameworkWithoutPlatform as private [sgen] Add Ward annotations to sgen_get_total_allocated_bytes (dotnet#43833) Attempt to fix https://jenkins.mono-project.com/job/test-mono-mainline-staticanalysis/ Co-authored-by: lambdageek <[email protected]> [tests] Re-enable tests fixed by dotnet#44081 (dotnet#44212) Fixes mono/mono#15030 and fixes mono/mono#15031 and fixes mono/mono#15032 Add an implicit argument coercion check. (dotnet#43386) * Add `impCheckImplicitArgumentCoercion`. * Fix tests with type mismatch. * Try to fix VM signature. * Allow to pass byref as native int. * another fix. * Fix another IL test. [mono] Change CMakelists.txt "python" -> Python3_EXECUTABLE (dotnet#44340) Debian doesn't install a "python" binary for python3. Tweak StreamConformanceTests for cancellation (dotnet#44342) - Avoid unnecessary timers - Separate tests for precancellation, ReadAsync(byte[], ...) cancellation, and ReadAsync(Memory, ...) cancellation Use Dictionary for underlying cache of ResourceSet (dotnet#44104) Simplify catch-rethrow logic in NetworkStream (dotnet#44246) A follow-up on dotnet#40772 (comment), simplifies and harmonizes the way we wrap exceptions into IOException. Having one catch block working with System.Exception seems to be enough here, no need for specific handling of SocketException. Simple GT_NEG optimization for dotnet#13837 (dotnet#43921) * Simple arithmetic optimization with GT_NEG * Skip GT_NEG optimization when an operand is constant. Revert bitwise rotation pattern * Fixed Value Numbering assert * Cleaned up code and comments for simple GT_NEG optimization * Formatting Co-authored-by: Julie Lee <[email protected]> [master] Update dependencies from mono/linker (dotnet#44322) * Update dependencies from https://github.com/mono/linker build 20201105.1 Microsoft.NET.ILLink.Tasks From Version 6.0.0-alpha.1.20527.2 -> To Version 6.0.0-alpha.1.20555.1 * Update dependencies from https://github.com/mono/linker build 20201105.2 Microsoft.NET.ILLink.Tasks From Version 6.0.0-alpha.1.20527.2 -> To Version 6.0.0-alpha.1.20555.2 * Disable new optimization for libraries mode (it cannot work in this mode) Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Marek Safar <[email protected]> Tighten argument validation in StreamConformanceTests (dotnet#44326) Add threshold on number of files / partition in SPMI collection (dotnet#44180) * Add check for files count * Fix the OS check * decrese file limit to 1500: * misc fix * Do not upload to azure if mch files are zero size Fix ELT profiler tests (dotnet#44285) [master] Update dependencies from dotnet/arcade dotnet/llvm-project dotnet/icu (dotnet#44336) [master] Update dependencies from dotnet/arcade dotnet/llvm-project dotnet/icu - Merge branch 'master' into darc-master-2211df94-2a02-4c3c-abe1-e3534e896267 Fix Send_TimeoutResponseContent_Throws (dotnet#44356) If the client times out too quickly, the server may never have a connection to accept and will hang forever. Match CoreCLR behaviour on thread start failure (dotnet#44124) Co-authored-by: Aleksey Kliger (λgeek) <[email protected]> Add slash in Windows SoD tool build (dotnet#44359) * Add slash in Windows SoD tool build * Update SoD search path to match output dir * Fixup dotnet version * Remove merge commit headers * Disable PRs Co-authored-by: Drew Scoggins <andrew.g.scoggins@gmail> Reflect test path changes in .gitattributes; remove nonexistent files (dotnet#44371) Bootstrapping a test for R2RDump (dotnet#42150) Improve performance of Enum's generic IsDefined / GetName / GetNames (dotnet#44355) Eliminates the boxing in IsDefined/GetName/GetValues, and in GetNames avoids having to go through RuntimeType's GetEnumNames override. clarify http version test (dotnet#44379) Co-authored-by: Geoffrey Kizer <[email protected]> Update dependencies from https://github.com/mono/linker build 20201106.1 (dotnet#44367) Microsoft.NET.ILLink.Tasks From Version 6.0.0-alpha.1.20555.2 -> To Version 6.0.0-alpha.1.20556.1 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Disable RunThreadLocalTest8_Values on Mono (dotnet#44357) * Disable RunThreadLocalTest8_Values on Mono It's failing on SLES * fix typo LongProcessNamesAreSupported: make test work on distros where sleep is a symlink/script (dotnet#44299) * LongProcessNamesAreSupported: make test work on distros where sleep is a symlink/script * PR feedback Co-authored-by: Stephen Toub <[email protected]> * fix compilation Co-authored-by: Stephen Toub <[email protected]> add missing constructor overloads (dotnet#44380) Co-authored-by: Geoffrey Kizer <[email protected]> change using in ConnectCallback_UseUnixDomainSocket_Success (dotnet#44366) Clean up the samples (dotnet#44293) Update dotnet/roslyn issue link Delete stale comment about dotnet/roslyn#30797 Fix/remove TODO-NULLABLEs (dotnet#44300) * Fix/remove TODO-NULLABLEs * remove redundant ! * apply Jozkee's feedback * address feedback Update glossary (dotnet#44274) Co-authored-by: Juan Hoyos <[email protected]> Co-authored-by: Stephen Toub <[email protected]> Co-authored-by: Günther Foidl <[email protected]> Add files need for wasm executable relinking/aot to the wasm runtime pack. (dotnet#43785) Co-authored-by: Alexander Köplinger <[email protected]> Move some more UnmanagedCallersOnly tests to IL now that they're invalid C# (dotnet#43366) Fix C++ build for mono/metadata/threads.c (dotnet#44413) `throw` is a reserved keyword in C++. Disable a failing test. (dotnet#44404) Change async void System.Text.Json test to be async Task (dotnet#44418) Improve crossgen2 comparison jobs (dotnet#44119) - Fix compilation on unix platforms - Wrap use of wildcard in quotes - Print better display name into log - Fix X86 constant comparison handling - Add ability to compile specific overload via single method switches Remove some unnecessary GetTypeInfo usage (dotnet#44414) Fix MarshalTypedArrayByte and re-enable it. Re-enable TestFunctionApply

lambdageek added the area-CoreLib-mono label Oct 30, 2020

[mono] Object.GetRawData -> RuntimeHelpers.GetRawData (this object)

fb59f61

Make it into an extension method like CoreCLR.

lambdageek force-pushed the get-raw-data-like-coreclr branch from fed2b12 to d3aaa43 Compare October 30, 2020 20:31

lambdageek changed the title ~~[mono] Object.GetRawData -> RuntimeHelpers.GetRawData (this object)~~ Move RuntimeHelpers.GetRawData() to src/libraries, use in Mono Oct 30, 2020

Move RuntimeHelpers.GetRawData and RawData class to shared location

6a8bc4d

lambdageek force-pushed the get-raw-data-like-coreclr branch from d3aaa43 to 6a8bc4d Compare October 30, 2020 20:34

[mono] Remove intrinsic support for Object.GetRawData

f45059f

lambdageek marked this pull request as ready for review October 30, 2020 20:37

lambdageek requested review from akoeplinger, BrzVlad, CoffeeFlux, EgorBo, marek-safar, SamMonoRT, steveisok and vargaz as code owners October 30, 2020 20:37

monojenkins mentioned this pull request Oct 30, 2020

[netcore] remove 2 extra members from System.Object mono/mono#20535

Merged

stephentoub reviewed Oct 31, 2020

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs Outdated Show resolved Hide resolved

stephentoub previously approved these changes Oct 31, 2020

View reviewed changes

BrzVlad approved these changes Nov 1, 2020

View reviewed changes

lambdageek added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 2, 2020

Revert "[mono] Remove intrinsic support for Object.GetRawData"

81bc63e

This reverts commit f45059f.

lambdageek added 3 commits November 2, 2020 14:46

Revert "Move RuntimeHelpers.GetRawData and RawData class to shared lo…

b77d6a2

…cation" This reverts commit 6a8bc4d.

Move Object.GetRawData to RuntimeHelpers.GetRawData(this object obj)

347cd9d

But keep it as an intrinsic

Update jit and interp intrinsics for RuntimeHelpers.GetRawData

2c85fc7

lambdageek changed the title ~~Move RuntimeHelpers.GetRawData() to src/libraries, use in Mono~~ [mono] remove 2 extra members from System.Object Nov 2, 2020

lambdageek added 2 commits November 2, 2020 15:12

Replace Object.CloneInternal by MarshalAsAttribute.CloneInternal

9556d0f

there was only one use

TypeInfoTests.FindMembers should pass now

a87bfb0

lambdageek force-pushed the get-raw-data-like-coreclr branch from b5ea354 to a87bfb0 Compare November 2, 2020 20:24

lambdageek requested a review from BrzVlad November 2, 2020 20:25

CoffeeFlux approved these changes Nov 2, 2020

View reviewed changes

lambdageek removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 3, 2020

lambdageek merged commit a219be3 into dotnet:master Nov 3, 2020

lambdageek added a commit to lambdageek/runtime that referenced this pull request Nov 3, 2020

[tests] Re-enable tests fixed by dotnet#44081

fecabce

Fixes mono/mono#15030 and fixes mono/mono#15031 and fixes mono/mono#15032

lambdageek added a commit that referenced this pull request Nov 6, 2020

[tests] Re-enable tests fixed by #44081 (#44212)

d51d380

Fixes mono/mono#15030 and fixes mono/mono#15031 and fixes mono/mono#15032

ghost locked as resolved and limited conversation to collaborators Dec 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[mono] remove 2 extra members from System.Object #44081

[mono] remove 2 extra members from System.Object #44081

lambdageek commented Oct 30, 2020 •

edited

Loading

Dotnet-GitSync-Bot commented Oct 30, 2020

CoffeeFlux commented Oct 30, 2020

CoffeeFlux commented Oct 30, 2020

vargaz commented Oct 30, 2020

stephentoub left a comment

CoffeeFlux commented Oct 31, 2020

lambdageek commented Nov 2, 2020

BrzVlad commented Nov 2, 2020

vargaz commented Nov 2, 2020

lambdageek commented Nov 2, 2020 •

edited

Loading

CoffeeFlux left a comment

BrzVlad commented Nov 2, 2020

lambdageek commented Nov 2, 2020

[mono] remove 2 extra members from System.Object #44081

[mono] remove 2 extra members from System.Object #44081

Conversation

lambdageek commented Oct 30, 2020 • edited Loading

Dotnet-GitSync-Bot commented Oct 30, 2020

CoffeeFlux commented Oct 30, 2020

CoffeeFlux commented Oct 30, 2020

vargaz commented Oct 30, 2020

stephentoub left a comment

Choose a reason for hiding this comment

CoffeeFlux commented Oct 31, 2020

lambdageek commented Nov 2, 2020

BrzVlad commented Nov 2, 2020

vargaz commented Nov 2, 2020

lambdageek commented Nov 2, 2020 • edited Loading

CoffeeFlux left a comment

Choose a reason for hiding this comment

BrzVlad commented Nov 2, 2020

lambdageek commented Nov 2, 2020

lambdageek commented Oct 30, 2020 •

edited

Loading

lambdageek commented Nov 2, 2020 •

edited

Loading