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

Allow stackalloc for reference types when they are used as locals when not escaping #16154

Closed
sirgru opened this issue Dec 30, 2016 · 12 comments
Closed

Comments

@sirgru
Copy link

sirgru commented Dec 30, 2016

Problem:
In some domains, game development for example, reducing allocations on the heap is a big concern. One obvious way to reduce allocations was proposed here: https://github.com/dotnet/coreclr/issues/1784 . And, while this solution has good merit, there are a couple of drawbacks:

  • It relies on the compiler to perform the analysis. This makes no guarantees about where object will be created based on the analysis.
  • Does not show intent of where we want the allocation.
  • Sometimes in tight loops we want to be sure to prevent allocations. Even if we can be 'pretty sure' that compiler will create the class on the stack, some change can inadvertently be made down the line and this behavior can be silently broken.

Solution:
Introduce use of stackalloc instead of new for reference types to allow creation on the stack, in cases where they are locals and are used in no features that would risk escaping, such as being used as method parameters of being captured. After something has been stackalloc-ed, any attempt to use it in a way that could possibly lead to escaping would prevent compilation. In that case the user could simply use regular new.

Example:

void Update() {
    Klass k = stackalloc Klass();

   _myVar = k; // Does not compile, assigment to non locals is forbidden.
}  // k is removed from stack, quick and simple

However, there are still cases where proving this may be more complicated:

class Klass {
    public Register() {
        Manager.Instance.Register(this);
    }
}

class Fast {
    void Update() {
        Klass k = stackalloc Klass ();  
        k.Register();  // Compile Error: Uses a method that references an external object where it passes itself as a parameter
    } 
}

I can imagine there are cases where the compiler can not deduce if the escaping happened. In that case, an exception can be thrown and no assignement would happen.
So, this proposal can be seen as a more strict, enforced version of the previous one, in order to show intent and attempt to make guarantees. This proposal has no connections with unsafe context.

@HaloFour
Copy link

This has come up before and IIRC it's not possible in C# without support for it from the CLR:

#2104
https://github.com/dotnet/coreclr/issues/1784

@jaredpar
Copy link
Member

I can imagine there are cases where the compiler can not deduce if the escaping happened

This is the core problem with this proposal. In order to be safe the compiler must be able to distinguish between operations that allow escaping and those which do not. This is a significant problem for C# as it exist today.

Even the simple code that you put into the sample is problematic:

Kclass c = stackalloc Kclass()

This itself could be dangerous as constructors can be a source of escaping this:

class Kclass {
  static Kclass _dangerous;
  internal KClass() {
    // Evil but legal
    _dangerous = this;
  }
}

Hence for the compiler to know that even the simple act of allocating is safe it must understand the escaping semantics of the constructor of Kclass. That includes the same knowledge for any base constructors, methods invoked, virtual methods invoked, etc ...

Solving this problem can really only be done with a new annotation, for example noescape. Interprocedural analysis is generally not a good solution for this problem as it's both expensive and not applicable to all scenarios. For instance I can't do inter procedural analysis on a reference assembly (method stubs only, no code).

One of the projects we did in Midori took the annotation approach (it was called scoped). It is possible to add to C# but it significantly complicates the language and is fairly viral. I don't think it would work well as a general feature.

@sirgru
Copy link
Author

sirgru commented Dec 30, 2016

Thank you for the explanation. This seems to be a hard constraint for this and similar proposals I've seen around. Too bad. I agree that annotating classes would not be a great solution, as that would require almost everyone to make the annotations in order to be compatible with this feature.

Would it be possible to throw an exception though? As a rough outline, could the CLR enter a special "assignment checking mode" at the time of construction or assignment from anything that is stackalloc-ed, and while within that mode every assignment is examined until exited from that mode. If within the assignment the right side is the location of this then operation is aborted and exception is thrown? This hypothetical workaround could solve the problems above. In that case, any kind of analysis but the most simple assignment in scope would not make much sense.

class Kclass {
  static Kclass _dangerous;
  internal Kclass() {
    _dangerous = this;  // Stopped by exception, we're still in monitoring mode for that k
  }
}

class Fast {
    void Update() {
       // Enter "assignment monitoring mode"
        Kclass k = stackalloc Kclass ();  
       // Exit "assignment monitoring mode"

        // Enter "assignment monitoring mode"
       _myVar = k;  // compiler can catch this one
       // Exit "assignment monitoring mode"
       
        // Enter "assignment monitoring mode"
        k.Register();     // Actually, not discovered at compile time, but exception.
       // Exit "assignment monitoring mode" 
       
    } 
}

@alrz
Copy link
Contributor

alrz commented Dec 30, 2016

@jnm2
Copy link
Contributor

jnm2 commented Dec 30, 2016

Re: throwing exception, wouldn't runtime tracking nullify any perf gains from doing this in the first place?

@sirgru
Copy link
Author

sirgru commented Dec 30, 2016

Probably, but I had to try :) It could prevent the allocations while slowing down the assignments.

@sirgru
Copy link
Author

sirgru commented Dec 30, 2016

Actually, I have concluded this problem is not solvable in the way I wanted it without being more trouble than it's worth. Sometimes the compiler can make a proper deduction, and in that case the other proposal can come in handy as a benefit (but not expectation). If these allocations are an issue, I guess they must be solved manually, like before:

private Klass _tempK = new Klass();
void Update() {
    _tempK.Init();
   // Use of _tempK ...
}

@ygc369
Copy link

ygc369 commented Jul 11, 2017

@jnm2 @sirgru @jaredpar
I think throwing exception is a good solution.
Although runtime tracking might slow down the assignments a little, but heap allocation is avoided. I think this is a reasonable tradeoff. It might make the program run a bit slower, but it also reduces GC pauses which are more unacceptable. If someone doesn't like this, he can choose to not use stackalloc. Once someone choose to use it, he should know the side-effect and be responsible for it, while gaining the benefits.

@sirgru
Copy link
Author

sirgru commented Jul 12, 2017

@ygc369 this seems to be an eternal topic, I see your issues about the similar thing are still having traction.
Most of the time I work with Unity engine and they use Mono and a non-generational garbage collector, which means these "innocent" allocations cause big trouble (GC hiccups), even more than they hypothetically would under CLR where short-lived objects would be collected as Gen0. As such, CoreCLR improvements would probably not benefit my use case directly any time soon, but from reading Joe Duffy's blog and other resources I don't have doubt these small, "innocent" allocations are evil in many other domains where performance is important.

I currently don't believe the solution above would the be a good fit, but I thought about the subject later and posted a proposal here https://github.com/dotnet/coreclr/issues/8756. The gist is: the objects could potentially live on the heap "inside" a custom allocator, where they are reference counted and un-examined by regular GC. Once they get out of scope, if the reference count is not 1 (the last remaining one), this means the variable has "escaped" and it should either be promoted to a full "heap" variable, now examined by GC or an exception could be thrown. This would effectively lower the GC pressure with minimal performance impact, as in most cases these short-lived objects are not referenced in many places. Also, it would not bring the stack back into the picture, which can be good because I don't think stack objects can be treated polymorphically. It would also have a benefit of improving memory locality in places where it matters, provided the programmer does it right.

@ygc369
Copy link

ygc369 commented Jul 13, 2017

@sirgru
I've seen https://github.com/dotnet/coreclr/issues/8756. I think you can write a simple one yourself.
For example:

class Allocator<T> where T: new()
{
    private T[] arr;
    private int alloc_index
    public Allocator(int max_num)
    {
        this.arr=new T[max_num];
        int i;
        for(i=0;i<max_num;i++)this.arr[i]=new T();
        this.alloc_index=0;
    }
    public T alloc()
    {
        return this.arr[this.alloc_index++];
    }
}

@jnm2
Copy link
Contributor

jnm2 commented Oct 4, 2018

Fyi, work has started on stack-allocated objects. dotnet/coreclr#20251

@CyrusNajmabadi
Copy link
Member

Closing this out. We're doing all language design now at dotnet/csharplang. If you're still interested in this idea let us know and we can migrate this over to a discussion in that repo. Thanks!

@CyrusNajmabadi CyrusNajmabadi closed this as not planned Won't fix, can't repro, duplicate, stale Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants