Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify remaining diverged misc types in corelib #10293

Closed
danmoseley opened this issue May 7, 2018 · 7 comments
Closed

Unify remaining diverged misc types in corelib #10293

danmoseley opened this issue May 7, 2018 · 7 comments

Comments

@danmoseley
Copy link
Member

We have reached the point that 60% of CoreCLR corelib code is shareable. We believe we can reasonably take this further, perhaps to 80% however much of the remaining reconciliation will take domain knowledge because the merges are not trivial. See #7394

From a diff of corert\src\system.private.corelib\src and coreclr\src\mscorlib\src these are files whose implementations are yet to be unified. In some cases this will take care. In some happy cases the CoreRT code may just be a frozen old copy of the CoreCLR code from years ago in which case possibly resolution may be just "take coreclr". Once implementations have been reconciled, the shared copy should move under "shared" and the other copies deleted.

Some of these will be specific to CoreCLR/CoreRT and those obviously should stay as they are.

  • .\internal\developerexperience\developerexperience.cs
  • .\internal\diagnostics\exceptionextensions.cs
  • .\internal\diagnostics\stacktracehelper.cs
  • .\internal\intrinsicsupport\comparerhelpers.cs
  • .\internal\intrinsicsupport\equalitycomparerhelpers.cs
  • .\internal\runtime\eetype.runtime.cs
  • .\internal\runtime\threadstatics.cs
  • .\internal\runtime\typeloaderexceptionhelper.cs
  • .\internal\runtime\augments\asynccausalitytracer.cs
  • .\internal\runtime\augments\dynamicdelegateaugments.cs
  • .\internal\runtime\augments\enuminfo.cs
  • .\internal\runtime\augments\environmentaugments.corert.cs
  • .\internal\runtime\augments\environmentaugments.corert.win32.cs
  • .\internal\runtime\augments\environmentaugments.cs
  • .\internal\runtime\augments\environmentaugments.fromregistry.win32.cs
  • .\internal\runtime\augments\environmentaugments.fromregistry.withoutregistry.cs
  • .\internal\runtime\augments\environmentaugments.projectn.cs
  • .\internal\runtime\augments\environmentaugments.unix.cs
  • .\internal\runtime\augments\environmentaugments.windows.cs
  • .\internal\runtime\augments\interopcallbacks.cs
  • .\internal\runtime\augments\reflectionexecutiondomaincallbacks.cs
  • .\internal\runtime\augments\runtimeaugments.cs
  • .\internal\runtime\augments\spinlocktracecallbacks.cs
  • .\internal\runtime\augments\stacktracemetadatacallbacks.cs
  • .\internal\runtime\augments\tasktracecallbacks.cs
  • .\internal\runtime\augments\typeloadercallbacks.cs
  • .\internal\runtime\augments\winrtinterop.cs
  • .\internal\toolchain\nonexecutableattribute.cs
  • .\interop\interop.manual.cs
  • .\interop\interop.winrt.cs
  • .\system__canon.cs
  • .\system\activator.cs
  • .\system\appcontext.cs
  • .\system\appcontext.unix.cs
  • .\system\appcontext.windows.cs
  • .\system\appcontextconfighelper.cs
  • .\system\appcontextdefaultvalues.cs
  • .\system\array.corert.cs
  • .\system\array.cs
  • .\system\attribute.cs
  • .\system\buffer.cs
  • .\system\byreference.cs
  • .\system\currentsystemtimezone.cache.cs
  • .\system\datetime.unix.corert.cs
  • .\system\datetime.windows.corert.cs
  • .\system\defaultbinder.canconvert.cs
  • .\system\delegate.cs
  • .\system\delegate.defaultparameters.cs
  • .\system\eetypeptr.cs
  • .\system\enum.cs
  • .\system\environment.cs
  • .\system\environment.unix.cs
  • .\system\environment.win32.cs
  • .\system\environment.windows.cs
  • .\system\exception.cs
  • .\system\gc.cs
  • .\system\guid.corert.cs
  • .\system\helpers.cs
  • .\system\highperformancecounter.unix.cs
  • .\system\highperformancecounter.windows.cs
  • .\system\insufficientmemoryexception.cs
  • .\system\invokeutils.cs
  • .\system\math.corert.cs
  • .\system\mathf.corert.cs
  • .\system\mdarray.cs
  • .\system\missingfieldexception.cs
  • .\system\missingmemberexception.cs
  • .\system\modulehandle.cs
  • .\system\multicastdelegate.cs
  • .\system\number.corert.cs
  • .\system\number.unix.cs
  • .\system\number.windows.cs
  • .\system\object.cs
  • .\system\outofmemoryexception.cs
  • .\system\primitivesruntimecontracts.cs
  • .\system\runtimeargumenthandle.cs
  • .\system\runtimeexceptionhelpers.cs
  • .\system\runtimefieldhandle.cs
  • .\system\runtimemethodhandle.cs
  • .\system\runtimetypehandle.cs
  • .\system\string.corert.cs
  • .\system\string.intern.cs
  • .\system\throwhelper.cs
  • .\system\timezoneinfo.winrt.cs
  • .\system\type.corert.cs
  • .\system\type.internal.cs
  • .\system\type.unix.cs
  • .\system\type.win32.cs
  • .\system\type.winrt.cs
  • .\system\typedreference.cs
  • .\system\typeloadexception.corert.cs
  • .\system\typeloadexception.cs
  • .\system\typeunificationkey.cs
  • .\system\valuetype.cs
  • .\system\weakreference.cs
  • .\system\weakreferenceoft.cs
  • .\system\runtime\commandline.windows.cs
  • .\system\runtime\exceptionids.cs
  • .\system\runtime\gcsettings.cs
  • .\system\runtime\initializefinalizerthread.cs
  • .\system\runtime\memoryfailpoint.cs
  • .\system\runtime\runtimeexportattribute.cs
  • .\system\runtime\runtimeimportattribute.cs
  • .\system\runtime\runtimeimports.cs
  • .\system\runtime\typeloaderexports.cs
  • .\system\runtime\loader\assemblyloadcontext.cs
  • .\system\runtime\serialization\serializationinfo.cs
  • .\system\text\stringbuilder.corert.cs
  • .\system\text\stringbuildercache.cs
@danmoseley
Copy link
Member Author

@maryamariyan after Microsoft.Win32 there are probably some of these that don't need too much context.

@danmoseley
Copy link
Member Author

For those that are seemingly horrible diffs, you can use git log to check whether they are simply an older version of CoreCLR. If so, very likely they can just be replaced with the CoreCLR version.

@maryamariyan
Copy link
Member

Thanks @danmosemsft

@jkotas
Copy link
Member

jkotas commented May 12, 2018

you can use git log to check whether they are simply an older version of CoreCLR

I doubt that there any in this issue where this would work.

The relatively easy ones in this issue that I see are: *Exception.cs, CurrentSystemTimeZone.Cache.cs, SerializationInfo.cs, Array.cs (the runtime-specific parts were separated from non-runtime parts in CoreRT repo to some degree), Buffer.cs, GC*.cs.

Also, I found it useful to work on the horrible diffs incrementally. Once you identify pattern where things are different, fix that to make the diff less horrible. The files are still different, but less so - you can come to it later if it is not clear what to do about the rest. An example of such change is dotnet/corert#5765 . This change was actually fixing bug - the strategy to fix that bug was to unify one particular aspect of the file.

maryamariyan referenced this issue in maryamariyan/coreclr May 18, 2018
maryamariyan referenced this issue in maryamariyan/coreclr May 18, 2018
maryamariyan referenced this issue in maryamariyan/coreclr May 18, 2018
maryamariyan referenced this issue in maryamariyan/coreclr May 18, 2018
maryamariyan referenced this issue in maryamariyan/coreclr May 21, 2018
- InsufficientMemory,
- OutOfMemory
- ThreadInterrupted

Related to: dotnet/coreclr#17904
maryamariyan referenced this issue in maryamariyan/coreclr May 21, 2018
- InsufficientMemory,
- OutOfMemory
- ThreadInterrupted

Related to: dotnet/coreclr#17904
maryamariyan referenced this issue in maryamariyan/coreclr May 21, 2018
- InsufficientMemory,
- OutOfMemory
- ThreadInterrupted

Related to: dotnet/coreclr#17904
maryamariyan referenced this issue in maryamariyan/coreclr May 21, 2018
    - InsufficientMemory,
    - OutOfMemory
    - ThreadInterrupted

Related to: dotnet/coreclr#17904
@maryamariyan
Copy link
Member

maryamariyan commented May 22, 2018

@jkotas I looked at CurrentSystemTimeZone.Cache.cs and noticed it is a partial class. While looking more closely at its log and history, I noticed that somebody must have already moved the common implementation into a shared file called CurrentSystemTimzeZone.cs and left the remaining in the *.Cache.cs file.

Please let me know if you still recommend modifying it further to be moved to shared.

@jkotas
Copy link
Member

jkotas commented May 22, 2018

The cache implementation had to be split because of we did not have the public Hashtable in CoreLib. Now that the public Hashtable is in CoreLib, you can undo the split.

maryamariyan referenced this issue in dotnet/coreclr May 22, 2018
…#18049)

* Move following exceptions to shared:

    - InsufficientMemory,
    - OutOfMemory
    - ThreadInterrupted

Related to: dotnet/coreclr#17904

Reduced diff in RegistryKey visible between coreclr and corert
dotnet-bot referenced this issue in dotnet/corefx May 22, 2018
… (#18049)

* Move following exceptions to shared:

    - InsufficientMemory,
    - OutOfMemory
    - ThreadInterrupted

Related to: dotnet/coreclr#17904

Reduced diff in RegistryKey visible between coreclr and corert

Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
dotnet-bot referenced this issue in dotnet/corert May 22, 2018
… (#18049)

* Move following exceptions to shared:

    - InsufficientMemory,
    - OutOfMemory
    - ThreadInterrupted

Related to: dotnet/coreclr#17904

Reduced diff in RegistryKey visible between coreclr and corert

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
stephentoub referenced this issue in dotnet/corefx May 22, 2018
… (#18049)

* Move following exceptions to shared:

    - InsufficientMemory,
    - OutOfMemory
    - ThreadInterrupted

Related to: dotnet/coreclr#17904

Reduced diff in RegistryKey visible between coreclr and corert

Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
jkotas referenced this issue in dotnet/corert May 22, 2018
… (#18049)

* Move following exceptions to shared:

    - InsufficientMemory,
    - OutOfMemory
    - ThreadInterrupted

Related to: dotnet/coreclr#17904

Reduced diff in RegistryKey visible between coreclr and corert

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
@danmoseley
Copy link
Member Author

Extracted decimal into its own issue https://github.com/dotnet/coreclr/issues/18249

@jkotas jkotas closed this as completed Aug 29, 2019
@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants