-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Implement ICorProfilerInfo14::GetNonGCHeapBounds #85434
Conversation
Tagging subscribers to this area: @tommcdon Issue DetailsContributes to dotnet/diagnostics#4156
|
Object* result = reinterpret_cast<Object*>(nextObj); | ||
INDEBUG(result->Validate()); | ||
return result; | ||
return reinterpret_cast<Object*>(nextObj); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we agreed not to use COOP (Validate()
needs it)
// Add test coverage for IsFrozenObject API, currently, it is expected to return true | ||
// for objects from non-GC heap (it might also return true for frozen segments we don't track) | ||
BOOL isFrozen; | ||
hr = pCorProfilerInfo->IsFrozenObject(obj, &isFrozen); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cshung added test coverage for IsFrozenObject
on your request
/azp run runtime-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
Failures are #85081 and ci timeouts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the one comment, otherwise looks good
Failures are known, #85637 |
|
||
if (pcObjectRanges != nullptr) | ||
{ | ||
*pcObjectRanges = segmentsToInspect; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@EgorBo - I think we want *pcObjectRanges = segmentsCount
I'd expect the current behavior will make a test case like this fail:
ULONG count
GetNonGCHeapBounds(0, &count, nullptr);
ranges = new ranges[count];
GetNonGCHeapBounds(count, nullptr, ranges);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@EgorBo - I think we want
*pcObjectRanges = segmentsCount
I'd expect the current behavior will make a test case like this fail:
ULONG count GetNonGCHeapBounds(0, &count, nullptr); ranges = new ranges[count]; GetNonGCHeapBounds(count, nullptr, ranges);
Ah, makes sense, I thought it was "how many entries were written to ranges". Will fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I've seen it used that way too in other interfaces, but I checked GetGenerationBounds() for the GC case and it appears to have the segmentsCount semantics there so presumably we want to match the behavior.
Contributes to dotnet/diagnostics#4156