Dac changes for pinned object heap

[davmason]

Add apis to the dac to get generation specific data for SOS.

[ghost]

Tagging subscribers to this area: @tommcdon
Notify danmosemsft if you want to be subscribed.

[davmason]

Corresponding SOS changes in dotnet/diagnostics#1228

[Maoni0]

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.

[davmason]

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]

Nice work! I have a couple of minor things I would suggest changing before this gets checked in. Thanks!

[leculver]

LGTM

[Maoni0]

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).

[Maoni0]

unsigned int *pNeeded [](start = 113, length = 21)

is the only purpose of this arg is for verification on the sos side?

[davmason]

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);
}

[Maoni0]

I see. I didn't know it was going to be called this way. makes sense. thanks!

[Maoni0]

LGTM; thanks for doing this, @davmason!

[VSadov]

Anything was supposed to change here?

[davmason]

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

[VSadov]

#define NUMBERGENERATIONS 4

Maybe add a comment here for the future that this number is now off intentionally for compat reasons. Perhaps a similar comment around DAC_NUMBERGENERATIONS as well.

---

Refers to: src/coreclr/src/gc/gcinterface.dac.h:21 in da453e0. [](commit_id = da453e0, deletion_comment = False)

[VSadov]

LGTM
Thanks!!

[noahfalk]

LGTM modulo a few comments inline, thanks @davmason!