Skip to content

[RuntimeAsync] Possible way to optimize GetParallelMethodDesc for async (if needed) #3086

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[RuntimeAsync] Possible way to optimize GetParallelMethodDesc for async (if needed) #3086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions src/coreclr/vm/methodtable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7810,23 +7810,25 @@ MethodDesc* MethodTable::GetParallelMethodDesc(MethodDesc* pDefMD, AsyncVariantL
}
else
{
// Slow path for finding the Async variant (or not-Async variant, if we start from Async one)
// This could be optimized with some trickery around slot numbers, but doing so is ... confusing, so I'm not implementing this yet
mdMethodDef tkMethod = pDefMD->GetMemberDef();
Module* mod = pDefMD->GetModule();
bool isAsyncVariantMethod = pDefMD->IsAsyncVariantMethod();
WORD slot = pDefMD->GetSlot();

MethodTable::IntroducedMethodIterator it(this);
for (; it.IsValid(); it.Next())
// Async variants are laid out one after another
// - fist the task-returning entry and then async2 variant.
// Thus what we look for is at +1 or -1 from the definition we are given.
//
// TODO: if we search within the same MT and the current chunk has space,
// we may just increment/decrement pDefMD by the size of current desc.
// (it would be even faster than through slot tables)
if (pDefMD->IsTaskReturningMethod())
{
MethodDesc* pMD = it.GetMethodDesc();
if (pMD->GetMemberDef() == tkMethod
&& pMD->GetModule() == mod
&& pMD->IsAsyncVariantMethod() != isAsyncVariantMethod)
{
return pMD;
}
return GetMethodDescForSlot_NoThrow(slot + 1);
}
else if (pDefMD->IsAsyncVariantMethod())
{
return GetMethodDescForSlot_NoThrow(slot - 1);
}

// the definition is not a variant from a pair. (TODO: this is likely unreachable)
return NULL;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/vm/methodtablebuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5631,7 +5631,8 @@ MethodTableBuilder::PlaceNonVirtualMethods()
#endif // _DEBUG

if (!fCanHaveNonVtableSlots ||
it->GetMethodType() == mcInstantiated)
it->GetMethodType() == mcInstantiated ||
it->GetAsyncMethodKind() != AsyncMethodKind::NotAsync)
Copy link
Member Author

Choose a reason for hiding this comment

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

This would force async/task-returning methods to have real slots. This is the "size" tradeoff.

{
// We use slot during remoting and to map methods between generic instantiations
// (see MethodTable::GetParallelMethodDesc). The current implementation
Expand Down