You can also add it to QueryPerformanceCounter and SystemNative_GetTimestamp.

We're confident adding these because we've looked at the implementations in Windows and know the target functions meet our constraints?

@jkotas I would prefer to scope this only to those methods that I personally vetted in this review. There should be a larger initiative to investigate benefit to those functions and ensure there is value.

@stephentoub The QueryPerformanceCounter() is a tricky function because it is heavily optimized on some systems and very fast. However, on some older hardware it could invalidate some of our recommended guidance. I believe on Windows 7 the chance of hitting some of those issues is also possible due to how it was implemented on that system. I would need to look into SystemNative_GetTimestamp.

Which part of the guidance do you believe is invalid for QueryPerformanceCounter on Win7?

@jkotas It is more than a few instructions and can call into an external device for the timestamp. Admittedly it isn't blocking, so perhaps my statement is a stronger than it should be. There is no SLA on how long it can occur nor its precise implementation so on older systems it can be much more complex than we currently observe. When I was on the VS profiler team we were informed this was a possibility on older systems.

You can also update the comment on the attribute "/// The eventual public attribute should replace this one: https://github.com/dotnet/corefx/issues/40740".

That is done in #27368

Admittedly it isn't blocking

Right. I think it should be pretty safe to add this on QueryPerformanceCounter .

on older systems.

There are many things that work in less ideal way on Win7. I am not particularly concerned about the GC pause time being 0.1ms longer on Win7 once in a while.

@jkotas I applied to both QueryPerformanceCounter() and SystemNative_GetTimestamp(). These don't get consumed in SPCL right now so I didn't validate much but I guess I can iterate on issues quickly if we hit any in CoreFX. I was going to add to QueryPerformanceFrequency() but decided not to because that should be a single call and not called with the same frequency.

@stephentoub The implementation of SystemNative_GetTimestamp() appears to satisfy all our requirements so I am inclined to apply it there as well. I hit another JIT issue so needed to validate some things and went through the QPC check locally. I don't see any obvious issues with applying them.

@briansull Please review the JIT changes we spoke about. Thanks.

The implementation of SystemNative_GetTimestamp() appears to satisfy all our requirements so I am inclined to apply it there as well. I hit another JIT issue so needed to validate some things and went through the QPC check locally. I don't see any obvious issues with applying them.

Thanks. My question was more general about everything in this PR, not just the two functions Jan mentioned. I just wanted to confirm that we're confident in these cases. It sounds like we are.