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

Contribution idea: Refactor code generation to use Roslyn instead of CodeDOM #40

Closed
sergeybykov opened this issue Jan 28, 2015 · 12 comments
Assignees

Comments

@sergeybykov
Copy link
Contributor

No description provided.

@overeemm
Copy link

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

@gabikliot
Copy link
Contributor

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

@sergeybykov
Copy link
Contributor Author

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
Copy link
Contributor

+1 for a single grain factory.

@overeemm
Copy link

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link

MovGP0 commented Feb 14, 2015

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:

  • No Resource Locator Antipattern
  • The DI framework manages which factory needs to be injected
  • Testable with Unit Tests, since the implementation of the factory can be replaced with a mock

See also: Issue #39 - Support for dependency injection

@gabikliot gabikliot changed the title Refactor code generation to use Roslyn instead of CodeDOM Contribution idea: Refactor code generation to use Roslyn instead of CodeDOM May 1, 2015
@Dana-Ferguson
Copy link
Contributor

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
Copy link
Member

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

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
Copy link
Contributor Author

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
Copy link
Contributor Author

Resolved by #528.

sebastianburckhardt pushed a commit to sebastianburckhardt/orleans that referenced this issue Apr 24, 2017
PR Revamped TransactionConfiguration, DI enable TX class instantiation. dotnet#37 broke almost all of the transaction tests.
This gets them working again.
@ghost ghost locked as resolved and limited conversation to collaborators Sep 30, 2021
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

8 participants