Dac changes for pinned object heap #37853
Conversation
|
Tagging subscribers to this area: @tommcdon |
|
Corresponding SOS changes in dotnet/diagnostics#1228 |
|
it seems like the idea of this is to get 1 more than what NUMBERGENERATIONS. I had thought we'd do this by just getting the actual number of generations (you can add a method to GC's dac interface) instead of still hardcoding it. is there a problem doing that? that would allow us to accommodate more generations if need to instead of every time GC adds a new gen dac will have to be updated yet again. |
No problem at all. It's just hard coded everywhere in SOS so I was in that mindset. I just pushed changes to get the total number of generations from the GC. It looks like there is an interface version for the gc->dac stuff, but it is unused since the dac is built along the runtime. I did not bump the version, but can if it's necessary for any reason. |
leculver
left a comment
leculver
left a comment
There was a problem hiding this comment.
Nice work! I have a couple of minor things I would suggest changing before this gets checked in. Thanks!
|
that's great; thanks! I noticed there are a ton of white space changes in gc.cpp. would you mind making a separate PR that just includes those changes alone? I too noticed recently there seems to be some sort of enforcement of the non trailing white space (I dunno why this is enforced). |
| return S_OK; | ||
| } | ||
|
|
||
| HRESULT ClrDataAccess::GetGenerationTable(unsigned int cGenerations, struct DacpGenerationData *pGenerationData, unsigned int *pNeeded) |
There was a problem hiding this comment.
unsigned int *pNeeded [](start = 113, length = 21)
is the only purpose of this arg is for verification on the sos side?
There was a problem hiding this comment.
It's so you can request the number of generations and dynamically allocated an array of the right size
unsigned int count;
if (SUCCEEDED(pSos->GetGenerationTable(0, NULL, &count)
{
DacpGenerationData *pGenerationData = new DacpGenerationData[count];
pSos->GetGenerationTable(count, pGenerationData, NULL);
}
There was a problem hiding this comment.
I see. I didn't know it was going to be called this way. makes sense. thanks!
| @@ -1 +0,0 @@ | |||
| README No newline at end of file | |||
There was a problem hiding this comment.
Anything was supposed to change here?
There was a problem hiding this comment.
No, I'm not sure what was even changing that file. I wasn't opening it purposefully but every time I went to add a commit it was in the list
Maybe add a comment here for the future that this number is now off intentionally for compat reasons. Perhaps a similar comment around Refers to: src/coreclr/src/gc/gcinterface.dac.h:21 in da453e0. [](commit_id = da453e0, deletion_comment = False) |
Co-authored-by: Noah Falk <[email protected]>
ghost
locked as resolved and limited conversation to collaborators
5 participants
Add apis to the dac to get generation specific data for SOS.