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: Refactor code generation to use Roslyn instead of CodeDOM #40

Closed sergeybykov closed 8 years ago

overeemm commented 9 years ago

I'm curious about your thoughts on this. Is this about the orleans.codegen.cs files? Or about some other part?

gabikliot commented 9 years ago

Yes, its about orleans.codegen.cs . We auto generate this file and we use CodeDom to do that.

sergeybykov commented 9 years ago

Yes, it's about orleans.codegen.cs. As you can see in the code, we use a mix of CodeDOM and code snippets to generate several categories of types: factories, grain references, invokers, state objects, serializers. There are multiple issues here:

  1. CodeDOM is deprecated and does not support several features, notably async/await.
  2. The reason we do via orleans.codegen.cs and do some weird compile time gymnastics is to make IntelliSense work. However, it is only needed for factories, and not other generated artefacts. There is also the option of moving away from generating factories altogether and having only GrainFactory.GetGrain as the way of getting grain references.
  3. It has been suggested that some of the code generation can be done at run time. Pros: less work at compile time and integration into the build process. Cons: more run time magic, potentially preventable failures as startup, hard to debug, slower startup.
  4. Support for VB and F# has issues.
jkonecki commented 9 years ago

+1 for a single grain factory.

overeemm commented 9 years ago

Regarding the intellisense, how do you see that working out with Roslyn? We still need an extra build step to make that work. I will do some investigating tomorrow, to get myself familiar with the code. Let's see if I can contribute...

sergeybykov commented 9 years ago

If we eliminate generation of per-interface factories and only support GrainFactory.GetGrain, then presumably we won't need IntelliSense beyond the normal. We'd still need a build step for generating other artefacts, unless we go all the way with 3.

ReubenBond commented 9 years ago

CodeDOM vs Roslyn is a false dichotomy: Roslyn does include some rather verbose syntax generation methods, but its also a fully managed compiler. Even just using the latter aspect with the current code gen being compiled at runtime instead of build time has its advantages. For example, we can remove the MSBuild tasks and that makes I easier to have implementation, interface, & silo in the same project if people want that.

I am implying that we move away from XxxGrainFactory and use generic factory methods instead - which I think no one is arguing against.

For anyone wanting to play with Roslyn CodeGen, Roslyn Quoter is very handy http://roslynquoter.azurewebsites.net/

MovGP0 commented 9 years ago

A single factory with a generic method like GrainFactory.GetGrain<T> is basically the Resource Locator Antipattern. However, multiple factories are hard to manage - especially when they are static and can't be injected.

A better way would be a generic factory that can be injected like this:

public class MyGrain
{
    private IGrainFactory<IPlayerGrain> PlayerGrainFactory { get; set; }

    public MyGrain(IGrainFactory<IPlayerGrain> playerGrainFactory) 
    { 
        PlayerGrainFactory = playerGrainFactory;
    } 

    public Task Foo(Guid playerId)
    {
        var playerGrain = await PlayerGrainFactory.GetGrain(playerId);
        // ...
    }
}

This way we save multiple problems:

See also: Issue #39 - Support for dependency injection

Dana-Ferguson commented 9 years ago

I'm not sure about the progress on this, but I was able to recompile the ClientGenerator with Microsoft.CodeDom.Providers.DotNetCompilerPlatform, in order get Roslyn support.

I had to remove a System.Runtime.dll from the options.ReferencedAssemblies in ClientGenerator.cs, and add a "using Microsoft.CodeDom.Providers.DotNetCompilerPlatform;" and remove Microsoft.CSharp and VisualBasic usings.

ReubenBond commented 9 years ago

Hi @Kittiepryde, my progress on this is represented by PR #528 (maybe you can review it :smile:).

This issue wasn't fully fleshed-out when it was opened, but the primary driver for a move to Roslyn is to allow us to generate code on-the-fly rather than requiring compile-time code generation. This obviates need for projects to contain MSBuild code and will hopefully help reduce the setup cost for new users as well as setup issues. Under #528, users are no longer required to separate their solution into multiple projects (they probably still should), and everything can be squished into one misguided file as in https://gist.github.com/ReubenBond/f7d7dd546e83a8a8856e. Finally, allowing for runtime code generation frees the project from the need to maintain code generators for each of the supported .NET languages (principally F# and VB).

Most of that can be achieved using CodeDOM, but CodeDOM doesn't support all of the desired language features - for example, it doesn't support switch statements, which are featured prominently in the Orleans codegen (eg, in method invokers). I figured it was about time we rewrote code generation using a library which had full language support. Hopefully it is easier to change/add features using the new code generator.

sergeybykov commented 9 years ago

CodeDOM has been deprecated 5-6 years ago. That's why it doesn't support some of the new language feature, e.g. async/await, and most likely never will.

sergeybykov commented 8 years ago

Resolved by https://github.com/dotnet/orleans/pull/528.