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

RuntimeHelpers.PrepareDelegate/PrepareMethod should JIT the given method #15522

Closed
tmds opened this Issue Dec 14, 2017 · 9 comments

Comments

Projects
None yet
4 participants
@tmds
Copy link
Member

tmds commented Dec 14, 2017

There is a comment here:

// This method is a marker placed immediately before a try clause to mark the corresponding catch and finally blocks as
// constrained. There's no code here other than the probe because most of the work is done at JIT time when we spot a call to this routine.
public static void PrepareConstrainedRegions()

But I can't figure out if it is supported or not.

In case it isn't supported, is there another way to ensure certain methods in a call graph are jitted? Or to jit a method without calling it?

@jkotas

This comment has been minimized.

Copy link
Member

jkotas commented Dec 14, 2017

PrepareConstrainedRegions is no-op in .NET Core.

@jkotas jkotas added the question label Dec 14, 2017

@jkotas

This comment has been minimized.

Copy link
Member

jkotas commented Dec 14, 2017

ensure certain methods in a call graph are jitted?

There is not such method today. I think it may be reasonable to fix RuntimeHelpers.PrepareDelegate / PrepareMethod to JIT the method given to it (just the single method - without the complicated limited call-graph walk done by full .NET Framework implementation).

@4creators

This comment has been minimized.

Copy link
Collaborator

4creators commented Dec 14, 2017

I think it may be reasonable to fix RuntimeHelpers.PrepareDelegate / PrepareMethod to JIT the method given to it (just the single method

This could be very useful for hardware intrinsics in R2R scenarios.

@jkotas

This comment has been minimized.

Copy link
Member

jkotas commented Dec 14, 2017

@4creators Could you please elaborate why you think it would be helpful?

@4creators

This comment has been minimized.

Copy link
Collaborator

4creators commented Dec 14, 2017

@jkotas maybe we can move discussion on how to use it in hardware intrinsics to #15427 as it will fit the scope of that issue

@mattwarren

This comment has been minimized.

Copy link
Collaborator

mattwarren commented Dec 15, 2017

I think it may be reasonable to fix RuntimeHelpers.PrepareDelegate / PrepareMethod to JIT the method given to it (just the single method - without the complicated limited call-graph walk done by full .NET Framework implementation).

Yes, this would be nice, based on this comment from @AndyAyersMS it currently has some limitations and the workaround is tricky.

Although I assume from your comment that you're not proposing to also JIT the entire call-graph, just a single method, is that correct?

Would PrepareMethod ever be extended to do this, i.e. ensure the entire call-graph of a method is also JITted, or is that not something you think the run-time should provide?

@jkotas

This comment has been minimized.

Copy link
Member

jkotas commented Dec 15, 2017

the entire call-graph, just a single method, is that correct?

The problem is with finding the entire call-graph is. Static non-virtual calls are easy, but the transitive closure can be very large. In full .NET Framework, this method also consults ReliabilityContract attributes to constraint the call graph to reasonable size.

Virtual calls, generics and every other runtime feature are hard. It would basically have to do same kind of global analysis as full AOT compilation does (slow and complex).

We prefer to describe behavior in terms of what guarantees are provided. Not in terms of how the guarantees are implemented. The guarantees that the PrepareMethod provides in .NET Framework are that the runtime will not introduce exceptions in the part of the callgraph that is annotated as reliable (ie the only exceptions thrown during execution of the code are the ones caused by the user code). These guarantees have been implemented by JITing a bunch of stuff upfront and pre-initializing all sorts of other stuff that can lead to runtime-thrown exceptions. That implementation was not the only way to implement the guarantees.

This feature was originally implemented for SQL CLR. It was super hard to test and use correctly.

it currently has some limitations

This applies to .NET Framework.

My though above was that we can improve .NET Core compatibility for cases where folks take advantage of PrepareMethod side-effects in .NET Framework.

@tmds

This comment has been minimized.

Copy link
Member Author

tmds commented Dec 18, 2017

@jkotas I'm fine with this issue being closed or re-purposed to track support for PrepareMethod.

@jkotas jkotas changed the title Q: Does .NET Core support PrepareConstrainedRegions? RuntimeHelpers.PrepareDelegate/PrepareMethod should JIT the given method Dec 18, 2017

@jkotas jkotas added area-VM up-for-grabs and removed question labels Dec 18, 2017

jkotas added a commit to jkotas/coreclr that referenced this issue Feb 14, 2018

Implement RuntimeHelpers.PrepareMethod/PrepareDelegate for CoreCLR
CoreCLR implementation of this method triggers jiting of the given method only.
It does not walk a subset of callgraph to provide CER guarantees because of CERs
are not supported by CoreCLR.

Fixes dotnet#15522

jkotas added a commit that referenced this issue Feb 15, 2018

Implement RuntimeHelpers.PrepareMethod/PrepareDelegate for CoreCLR (#…
…16382)

* Implement RuntimeHelpers.PrepareMethod/PrepareDelegate for CoreCLR

CoreCLR implementation of this method triggers jiting of the given method only.
It does not walk a subset of callgraph to provide CER guarantees because of CERs
are not supported by CoreCLR.

Fixes #15522
@tmds

This comment has been minimized.

Copy link
Member Author

tmds commented Feb 15, 2018

Thank you for implementing @jkotas!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.