Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Don't assume objects don't escape in pure helpers. #21177

Merged
merged 1 commit into from Nov 24, 2018

Conversation

erozenfeld
Copy link
Member

We can't assume objects don't escape in helpers marked as pure for the following reasons:

  1. The helpers may call user code that may make objects escape, e.g.,

    CALL_MANAGED_METHOD(fCast, BOOL, args);

  2. The helpers usually protect gc pointers with GCPROTECT_BEGIN() so the pointers are reported as normal pointers to the gc.
    Pointers to stack-allocated objects need to be reported as interior so they have to be protected with
    GCPROTECT_BEGININTERIOR().

  3. The helpers may cause these asserts in ValidateInner on stack-allocated objects:

    CHECK_AND_TEAR_DOWN(bSmallObjectHeapPtr || bLargeObjectHeapPtr);

    CHECK_AND_TEAR_DOWN(GetHeader()->Validate(bVerifySyncBlock));

We can't assume objects don't escape in helpers marked as pure for the following reasons:

1. The helpers may call user code that may make objects escape, e.g.,
https://github.com/dotnet/coreclr/blob/c94d8e68222d931d4bb1c4eb9a52b4b056e85f12/src/vm/jithelpers.cpp#L2371

2. The helpers usually protect gc pointers with GCPROTECT_BEGIN() so the pointers are reported as normal pointers to the gc.
Pointers to stack-allocated objects need to be reported as interior so they have to be protected with
GCPROTECT_BEGININTERIOR().

3. The helpers may cause these asserts in ValidateInner on stack-allocated objects:
https://github.com/dotnet/coreclr/blob/c94d8e68222d931d4bb1c4eb9a52b4b056e85f12/src/vm/object.cpp#L723
https://github.com/dotnet/coreclr/blob/c94d8e68222d931d4bb1c4eb9a52b4b056e85f12/src/vm/object.cpp#L730
@erozenfeld
Copy link
Member Author

@AndyAyersMS @echesakov @dotnet/jit-contrib PTAL

@erozenfeld erozenfeld merged commit 65f8867 into dotnet:master Nov 24, 2018
@AndyAyersMS
Copy link
Member

Do you have a sense of how often being conservative here will block stack allocation?

I'm wondering if we might want to revisit this someday.

@CarolEidt
Copy link

@AndyAyersMS - by revisiting this, I presume you mean refining the way we mark the helpers so that we can identify those that are truly "pure" in their handling of stack-allocated objects?

@@ -579,14 +579,12 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack<GenTree*>* parent

if (asCall->gtCallType == CT_HELPER)
{
const CorInfoHelpFunc helperNum = comp->eeGetHelperNum(asCall->gtCallMethHnd);
// TODO-ObjectStackAllocation: Special-case helpers here that

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erozenfeld - is there a tracking issue for this TODO?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CarolEidt It's one of the items in todo list in https://github.com/dotnet/coreclr/issues/20253#issue-366568135 :

  • special case calls to helpers where arguments don't escape (this will require ensuring that the helpers report gc arguments as interior to gc)

@AndyAyersMS
Copy link
Member

Yes... wonder how costly/risky it would be to update helpers to have more flexible expectations.

@erozenfeld
Copy link
Member Author

@AndyAyersMS Yes, I think we'll have to address this. There are two aspects:

  1. Identify helpers where params are guaranteed not to escape.
  2. Either change those helpers to report gc params as interior or, if that approach is not acceptable for perf reasons, create additional versions of the helpers that report params as interior and are only called when gc args may be stack-allocated.

We may decide to do this for a subset of the helpers, I haven't done analysis on cost/benefit tradeoff yet.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants