Optimize calling GetTotalWorkerThreadPoolCompletionCount

[billwert]

@ninedan reported that calling GetTotalThreadPoolCompletionCount is costing ~0.62% of all CPU usage in a large server scenario. It looks like we can easily move the following lines inside the check to for the thread adjustment interval, only paying this cost when we might use the answer.

@kouvel what do you think?

runtime/src/coreclr/src/vm/win32threadpool.cpp

Lines 922 to 924 in 2cc926e

	DWORD currentTicks = GetTickCount();
	LONG totalNumCompletions = (LONG)Thread::GetTotalWorkerThreadPoolCompletionCount();
	LONG numCompletions = totalNumCompletions - VolatileLoad(&PriorCompletedWorkRequests);

[kouvel]

Yes these could be moved, but the method called here (GetTotalWorkerThreadPoolCompletionCount) is different from the one you mentioned (GetTotalThreadPoolCompletionCount), which one is showing up in the profile? The latter function is called as part of ThreadPool.CompletedWorkItemCount, if that's the one showing up in the profile then it could be polled less frequently.

[billwert]

Ah, interesting. The .NET Framework code is slightly different and I didn't pay close attention when I was linking the source from this repo. Thanks for calling it out. It does look like it has changed here: dotnet/coreclr/pull/24113.

As this isn't a .NET Core scenario in the customer case I don't have a great barometer for how the new implementation will fare. Looking through the old and new code I suspect the real cost was ::FlushProcessWriteBuffers, which we don't seem to do now. The new code path does still do a fair bit of work which we should avoid if we aren't going to use the answer. I'll send a PR.

[billwert]

I got updated data which shows the actual cost under GetTotalThreadPoolCompletionCount was ThreadStore::GetAllThreadList, which is still in the code path here. (Presumably this is a scenario with very many threads.)

[kouvel]

I see, makes sense, thanks