Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

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

Did you accidentally miss including a file containing GCMemoryInfoData?

I see it in src/coreclr/src/System.Private.CoreLib/src/System/GC.cs, is that not what you are seeing?

Large number of build legs is failing with:

src/libraries/System.Private.CoreLib/src/System/GCMemoryInfo.cs(20,26): error CS0246: The type or namespace name 'GCMemoryInfoData' could not be found (are you missing a using directive or an assembly reference?)

After closer look, these build errors are in Mono build. So you will need to do something to make the Mono build green.

I wonder about this:

size_t GetHighPrecisionTimeStamp()
{
int64_t ts = GCToOSInterface::QueryPerformanceCounter();

return (size_t)(ts / qpf_us);

}

qpf_us being an int64_t, there could be a round-off error on some systems. It's probably more precise and efficient to use a double qpf_interval_us = 1e6 / qpf and do return (size_t)(ts * qpf_interval) instead.

I think the doc comment here needs a few words more:

    /// <summary>
    /// The index of this GC.
    /// </summary>
    public long Index => _data._index;

suggestion:

    /// The index of this GC. Indices start at 1 and increase by 1 for each GC, so the third GC in the execution of the program will have index 3.

But perhaps you can think of something better...

@jkotas right, it was just mono build failures; I have since moved the definition of GCMemoryInfoData from src\coreclr\src\System.Private.CoreLib\src\System\GC.cs to src\libraries\System.Private.CoreLib\src\System\GCMemoryInfo.cs, hopefully that addresses the issue. too bad this kind of failures do not show up when we build the coreclr version.

@PeterSolMS I've added details to the comment of Index and also changed the type of qpf_us to double and the return type of GetHighPrecisionTimeStamp to uint64_t.

@marek-safar, what's the process of making it work for mono for this PR? obviously mono's implementation is not there yet.

@PeterSolMS thanks so much for finding those!!! I'll make the corresponding changes a bit later today.

documenting the new GetGCMemoryInfo API

You should just include the docs under triple-slash comments in the doc. The doc generation tool will use to populate the docs for newly added APIs.

okie; I didn't know that was all there is for API doc... I thought we'd have sections that you don't see as code comments like "examples of using this API" and etc.

I thought we'd have sections that you don't see as code comments like "examples of using this API" and etc.

We (and others) can always submit PRs to the docs repo to augment the content further after they're initially populated. We just use the doc comments to seed the minimum so we at least have basic docs for summary, parameters, etc.

if there's no GC of the kind that happened, then by design you'd get back all 0's including the index. we can mention this in the doc.

That's nasty... Maybe return a nullable value instead? Or bool return and out parameter?

I believe I've addressed all your concerns, @jkotas and @stephentoub, please take a look at the latest commit. the only thing I'm waiting on is the review from mono folks. @BrzVlad I have no experience doing this; just let me know if I did something wrong.

this is good to go; I will wait till tomorrow noon and if I don't hear any objections by then I will merge.

thank you all for your reviews; really appreciate your help!