dotnet / orleans

Cloud Native application framework for .NET
https://docs.microsoft.com/dotnet/orleans
MIT License
10.07k stars 2.03k forks source link

Contribution idea: Port Orleans to coreclr #368

Closed gabikliot closed 6 years ago

gabikliot commented 9 years ago

Port the Orleans code to run on coreclr. Some APIs from the full .NET got deprecated in coreclr (aka .NET Core), mainly around files and reflection, but at large the porting effort shouldn't be too big. This will allow to run Orleans efficiently across different platforms.


Tools: Coreclr api search .NET API Usage .NET Portability Analyzer - download and Install .NET Portability Analyzer Usage

Progress so far: CoreCLR milestones ApiPortabilityAnalysis result file for all projects (let's find an appropriate place to place updates of this file, please don't pollute the master branch with these big reports)


There are 2 types of issues: (a) easy, just change an API - we have done all those. (b) deeper profound issues where API was deprecated or changed significantly and we will need to investigate other ways to achieve the same functionality:

Need to investigate and consider profound rewrite - feel to open a GH issue with ideas on how to handle each one of those issues (separate issue for each) : High priority

2nd priority

3rd priority


Already done issues (for progress tracking):

stavro commented 9 years ago

We run the compatibility tool some time ago. There are a couple of unsupported APIs, mainly about files (need to use streams instead) and code gen. We don't think it would be too hard to do the port. Perhaps a good way to start is to re-evaluate what needs to be ported exactly and then do the port in small steps, one API at a time (as opposite to one huge PR). That would make it much easier to validate and accept it, and also other will be able to join in and help.

Can someone share the instructions of how to set up the compatibility tool locally?

gabikliot commented 9 years ago

http://blogs.msdn.com/b/dotnet/archive/2014/08/06/leveraging-existing-code-across-net-platforms.aspx https://visualstudiogallery.msdn.microsoft.com/1177943e-cfb7-4822-a8a6-e56c7905292b

pherbel commented 9 years ago

Hi I think it is very important step and I would like to use Orleans client from the coreclr project. Is there a working in progress branch for this? Maybe I can contribute

gabikliot commented 9 years ago

We do not have a progress branch for this yet. In the meanwhile, we are doing a bunch of preparation work that will make the porting easier, such as reducing code generation (which is one of the biggest hurdles for coreclr port). This is tracked here: #474.

If you can contribute to the coreclr port, that would be wonderful! Perhaps a good first step is to precisely identify all pieces that need to be ported.

sergeybykov commented 9 years ago

No branch yet. We can create one for this.

pherbel commented 9 years ago

As I checked the code with the portability analyzer first step could be change of the usage of the Type class. The analyzer suggests use TypeInfo instead of Type which is fine for the coreclr. I assume that We don't need to change the CodeGeneration part because it will be deprecated and it has lot of Type usage. Will the CodeGeneration part be also deprecated in the Orleans.dll?

What do you think?

sergeybykov commented 9 years ago

I think most of the Orleans\CodeGeneration code will need to stay. We hope to significantly reduce the code in ClientGenerator.

gabikliot commented 9 years ago

We can't tell for sure if ALL CodeGeneration would be deprecated. Certainly parts of it will be. But some parts maybe much harder to get rid off. So I would not rely on it 100% at least not in the short term. Can definitely change usages of Type to TypeInfo. What were the other parts?

pherbel commented 9 years ago

I see. @gabikliot Not need to change everywhere. Type has some member which are not available in coreclr (e.g IsClass) but these are on exist on TypeInfo class. There are some other things which are not available at all like ReflectionOnlyGetType() We need to find workaround. Type stuff one of the biggest part because it is used a lot of places by Orleans. Other tricky part will be the assembly loading and AppDomain things but I hope we can find something similar solution what asp.net does. There are some other small things Environments, Serialization, etc. I think these are easiest part. And finally there are some stuff what we need to remove from the Runtime and find platform independent solution like Performance Counters.

BTW Would be really helpful if somebody who is working on the aspnet 5 and can help us to review and give feedback about the changes.

sergeybykov commented 9 years ago

I would suggest to put Reflection-only stuff aside, as we most likely need a complete solution there. It'd be a bit ironic because we migrated to Reflection-only assembly loading only about a year ago. But that's okay if we need it for CoreCLR compatibility. One possible straightforward solution here is to switch to or add support for explicitly listing grain assemblies.

AppDomain is primarily used for code inspection and generation and workarounds for .NET's assembly loading peculiarities. Then there are a couple of global event handlers for unhandled exceptions and process exit - easy. The rest is used for testing, and we already support starting silos in separate processes instead of app domains there. Can be easily ifdef'ed I think.

Performance counters are already isolated in OrleansPerfCounterManager and statistics, so should be relatively easy to condition or retarget.

We can definitely get in touch with the CoreCLR and ASP.NET5 folks to get their feedback and advise. We'll arrange that.

I created a branch for this work - https://github.com/dotnet/orleans/tree/coreclr-compatibility. Let's get started.

pherbel commented 9 years ago

@sergeybykov Did you have a chance to talk to CoreCLR folks about this. I'm just wondering where we can start and how. Maybe we should put together a check list about the steps. What do you think?

sergeybykov commented 9 years ago

CLR folks pointed to https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/support-dotnet-core-instructions.md. @gabikliot was going to put together a checklist.

gabikliot commented 9 years ago

I checked in the ApiPortabilityAnalysis result file for Orleans.dll here.

I also created a list of issues based on the analysis result in the top commet in this PR https://github.com/dotnet/orleans/issues/368#issue-72289980. I suggest people just grab one issue at a time, fix it, submit a PR and that way, gradually, over multiple PRs we will get closer and closer to the goal. The majority of issues are minor, but there are a couple that we will need to look at more seriously and consider alternatives. However, there are enough small/medium issues so anyone can make progress.

pherbel commented 9 years ago

@gabikliot @sergeybykov Could you please update the coreclr-compatibility.branch?

Here is my repo with some initial commits. I look forward your feedback. Thanks https://github.com/pherbel/orleans/commits/coreclr_portability

grahamehorner commented 9 years ago

@gabikliot would be glad to help out in anyway I can; loving the work the team is doing and would love to be a part of this.

gabikliot commented 9 years ago

Hi @grahamehorner. Your help would be great! We already have a first PR in this direction (#590 by @pherbel), which we are going to test and hopefully accept. There is more things that need to be ported. I would suggest just to pick one issue at a time from this list (https://github.com/dotnet/orleans/blob/master/misc/ApiPortabilityAnalysis-Orleans.dll-June.30.2015.htm) and submit a PR to master branch with that change. We prefer a number of smaller, independent, self contained PRs over one big PR with lots of different unrelated porting issues. It helps us test and make sure the existing code is operational all the time.

pherbel commented 9 years ago

@grahamehorner Great! @gabikliot SerializationAttirbute and the BinarySerialization will not be supported in the coreclr So we need to figure out how we can change it.

I found these related to serialization: Coreclr issue 1347 Coreclr issue 1203

Also there is a useful link to search the coreclr api and package Coreclr api search

jthelin commented 9 years ago

This is probably a heresy question, but here goes anyway ;)

If CoreClr is so far removed from .NET 4.5 requiring all these non backward compatible code changes, then what are the advantages of trying to be compatible with CoreClr rather than instead targeting Mono [on Linux + OS-X] as the "cross-platform" runtime?

All the portability analysis reports to date show that Orleans code base is more compatible with Mono than CoreClr.

For example, if we remove [Serializable] from all Exception classes then this code branch is unusable with "old" test host / suite which uses AppDomain and MarshellByRefObject in several important places. We will need to give quite a lot of thought to how to port the current test environment over to CoreClr.

We will effectively have created a one-way code fork, and two increasingly divergent code bases.

Having to we use lots of #if blocks to keep a single code base seems like a bad code smell to me, but so does two divergent code bases!

Thoughts?

Update: My Bad on the TypeInfo API changes. I updated my post above to reflect that new info. Type.GetTypeInfo is available in .NET 4.5 as an extension method in System.Refection namespace, but we don't import that everywhere, so VS IntellisSense sometimes can't see it. :(

jthelin commented 9 years ago

I updated my comment above after I found that GetTypeInfo is available as an extension method in System.Reflection namespace in .NET 4.5. My bad.

Given that, can we package up the CoreClr compat changes into those API changes that are already supported in 4.5 and which we can apply to the master branch, and have this coreclr branch for changes that are only applicable to CoreClr going forward?

If we end up with only a very small delta between the 4.5 master branch and the coreclr branch, then most of my meta-concerns might go away completely.

jthelin commented 9 years ago

I would like to use Orleans client from the coreclr project.

If we take a step back and re-exampine @pherbel stated requirement, then the initial focus should be on client access from CoreClr.

We don't necessarily need to port the whole of OrleansRuntime.dll to CoreClr immediately to help @pherbel do what he wants to do.

So initially targeting changes to Orleans.dll creates a good Minimal Viable Product deliverable and gets us to First Base with coreclr compat.

gabikliot commented 9 years ago

I think we don't need a separate coreclr branch at all. I recommend doing all porting efforts on the master branch. Fix a porting issue, submit a PR against master, we will test its correctness and perf. That way there will not be, by definition, "We will effectively have created a one-way code fork, and two increasingly divergent code bases." I am for one always-correct and operational code base.

Clearly, after we do all "easy" porting fixes we will remain with unsupported features, which we will need to look at one at a time. My recommendation is to do as much as we can first, without breaking changes, and then re-evaluate.


@pherbel , thanks for the links. Super useful!

ghost commented 9 years ago

@jthelin I think ultimately coreclr is the target we should be aiming for as coreclr this designed specifically for cloud applications; and orleans is a cloud application, this would also allow Orleans to run on nano server, Linux, mac OSes and windows IoT; rather than targeting the mono framework which won't run on nano server or windows IoT given the reduced OS footprint.

Also while I would opt for the creation of a set of windows universal components for the orleans client functionality; so these can run across all OS and device/processor platforms. Phone/Mobile, Table/Xbox, Desktop/Server

ghost commented 9 years ago

@gabikliot I'm just configuring my Orleans dev laptop with Windows10, VS2015 RC & GIT, once done I'll pick up the two items:-

TextReader.Close -> TextReader.Dispose Socket.Close -> Socket.Dispose

and when complete submit a PR

grahamehorner commented 9 years ago

@gabikliot FYI, I've switched accounts new account

gabikliot commented 9 years ago

@grahamehorner , what is GabrielCloudSystems? My name is Gabriel and I am definitely involved with Cloud Systems. :-)

grahamehorner commented 9 years ago

Gabriel Cloud Systems LTD is my company name, We are doing development in the #IoT space & .NET consultancy work in the north east of England for a number of clients (I own the company) :D but I/we also do open source and community forums/projects

Gabriel = Angel – Cloud and the Cloud is all about communications which just so happens that the patron saint is St. Gabriel and is the street name where we have our offices :D

gabikliot commented 9 years ago

Well, now you just need more Gabriels working for your company (or change your name) and you've got it all! ;-)

grahamehorner commented 9 years ago

@gabikliot I'd offer to hire you; but I'm sure you'd be out of my budget ;) (but worth every penny and more I suspect) lolz

pherbel commented 9 years ago

@gabikliot I agree to use master branch. Did you have a chance to check my PR? I will rebase it on the current master branch. In my PR there is a vNext project which is very helpful to check the real compatibility (It could be tricky if you are just using the 4.5.1 build). Maybe the vNext project is not fit to the master branch.

pherbel commented 9 years ago

@grahamehorner Great PRs! :smile:

grahamehorner commented 9 years ago

should Orleans using NewtonSoft,Json to perform BSON serialization given the BinaryFormatter.Serialize is not supported in coreclr; would be a breaking change and/or require a tool to perform a one time conversion of serialized and persisted object.

grahamehorner commented 9 years ago

@pherbel small but sweet ;) glad to help out anyway I can :D

grahamehorner commented 9 years ago

@gabikliot @pherbel personally I'd keep the Orleans.vNext in the master branch as well, the .cs files should be shared across the solutions in matching vNext projects eg. OrleansRuntime.vNext and the targets in the vNext project.json should be net45, dnx451, dnxcore :smile:

pherbel commented 9 years ago

@grahamehorner Yeah. That was the original idea. Actually it doesn't bother the current project structure. And later we can drop the old projects. The Serialization will be a tricky part.

gabikliot commented 9 years ago

@pherbel, I have looked the PR already, but I still need to test it more thoroughly for perf as well, in our internal vso test suite. Just want to make sure all those different reflection APIs did not hurt perf. We were a bit busy last days with 1.0.9 and other internal priorities, so it got pushed a bit. I will be on it in the next days. Yes, please ebase it, it would be great. Re vNext - I don't think there is any problem keeping it. Re serialization - this is a bigger issue. We need to consider #37. So I suggest we delay it for a bit, and hopefully will figure out a solution for pluggable serialization format to solve #37 which will also solve coreclr issue.

pherbel commented 9 years ago

@gabikliot Thanks it makes sense

gabikliot commented 9 years ago

Check this out as well: http://dotnetstatus.azurewebsites.net/usage

jthelin commented 9 years ago

http://dotnetstatus.azurewebsites.net/usage

I still have a really hard time getting my head around the fact that there is more red in the ASP.NET 5 column than in the Mono column! It is a big step function change in .NET APIs :(

Re serialization - this is a bigger issue.

It does seem like [Serializable] is going to be one of the largest disruption areas, so agree with @gabikliot that we should take step back and re-evaluate the big-picture requirements around serialization functionality before we dive in.

One embryonic concern I have is that as Orleans moves towards more runtime codegen approaches, it might make integrating with "pre-generated" serialization mechanisms like Bond or ProtoBuf more difficult, but we will need to look at that space in more detail to be able to form a concrete opinion.

grahamehorner commented 9 years ago

I agree serialization is going to be a issue; however abstract serialization away and allow a dependency injection of the serialization functionality it allows implementations that can be swapped in according to the target platform requirements/compatibility until such time a universal serialization implementation is developed or chosen.

grahamehorner commented 9 years ago

@gabikliot @jthelin re-evaluation of the serialization requirements is a good plan, as this could firm up the interface contract for a dependency injected serialization component and/or serialization component providers.

dVakulen commented 8 years ago

Current status: ManagementObjectCollection and ManagementObject aren't supported in coreclr, and I haven't found replacement for it yet, AsyncLocal can be used instead of CallContext.LogicalSetData and Trace.CorrelationManager but it isn't available in .Net 4.5, as well as System.Diagnostics.Process.SafeHandle.

grahamehorner commented 8 years ago

Dns.GetHostAddresses - not supported in .NET Core. Investigate and propose alternative options to find DNS name. We only use it for logging now, no logic relies on it.

could be resolved by using the libuv http://docs.libuv.org/en/latest/dns.html in a similar way the ASP.NET team do with Kestrel

dVakulen commented 8 years ago

@grahamehorner Thanks for advise, but this one was already resolved by using Dns.GetHostAddressesAsync

galvesribeiro commented 8 years ago

Just to let every1 know, since there was no progress being made on the Type -> TypeInfo since august, I'm working on it now and will submit a PR soon.

Thanks

dVakulen commented 8 years ago

FYI: Currently I'm working on CallContext.LogicalSetData.

Thanks

galvesribeiro commented 8 years ago

Good! I'll be catching the first(easy) list, and them will move to the assembly load stuff... Lets keep in touch on Gitter :)

grahamehorner commented 8 years ago

@dVakulen ok; didn't spot that, thanks for pointing this out

grahamehorner commented 8 years ago

would anyone have any objections to wrapping/replacing the creating an abstraction layer for Orleans PerformanceCounters with an implementation that uses ApplicationInsights

gabikliot commented 8 years ago

Go for it! No objections.

galvesribeiro commented 8 years ago

@grahamehorner that is something we need not just for PerformanceCounters, but for whole instrumentation. AI must be somehow a Plugin... Maybe we can have an interface like ITelemetryProvider and have many providers for many diff APM tools like NewRelic, ApplicationInsights and so on and use DI to inject the configured one... Do you mind create another issue for that so we can discuss it there? Performance Counters here are not actually something that is blocking this port to coreclr since it is already an external project. Thanks!