Consolidate QueryPerformanceCounter/GetTickCount usages to minipal/time.h

[huoyaoyuan]

Remove the duplicated implementations and use minipal/time.h as single source of truth.

Not touching a bit ifdef'd-out instances, which are supposed for local debug usages only.

[huoyaoyuan]

I wonder whether the different timer implementations can be replaced by std::chrono::high_resolution_clock.

[huoyaoyuan]

Build error looks unrelated:

/__w/1/s/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_ml_dsa.c:10:85: error: unused parameter 'hasSeed' [-Werror,-Wunused-parameter]
     10 | int32_t CryptoNative_MLDsaGetPalId(const EVP_PKEY* pKey, int32_t* mldsaId, int32_t* hasSeed, int32_t* hasSecretKey)

Should be introduced by #115569

[jkotas]

LGTM. Thank you!

[lateralusX]

LGTM, thanks!

[jkotas]

Build breaks on wasm:

  D:\a\_work\1\s\src\native\minipal\time.c(15): error C2220: the following warning is treated as an error
  D:\a\_work\1\s\src\native\minipal\time.c(15): warning C4189: 'ret': local variable is initialized but not referenced
  D:\a\_work\1\s\src\native\minipal\time.c(23): warning C4189: 'ret': local variable is initialized but not referenced
``

[jkotas]

The large number of non-wasm timeouts looks a bit suspect as well. We will see whether it repros once the build breaks are fixed.

[huoyaoyuan]

I'm somehow confused with wasm build config. Should TARGET_WINDOWS be used instead of HOST_WINDOWS? Seems that HOST is also defined for underlying OS for wasm. And, is there a difference for browser and wasi?

[jkotas]

These build breaks occur in the Windows-hosted Browser-targeting cross-compiler. Mono compiles with higher warning levels - it is why the build break shows up for browser only.

[jkotas]

I left a suggestion that should make the compiler happy

[AaronRobinsonMSFT]

This breaks the PAL tests build.

@huoyaoyuan How was this tested locally?

  1 error generated.
  make[3]: *** [pal/tests/palsuite/CMakeFiles/paltests.dir/threading/WaitForMultipleObjectsEx/test2/test2.cpp.o] Error 1
  make[3]: *** [pal/tests/palsuite/CMakeFiles/paltests.dir/threading/SleepEx/test1/test1.cpp.o] Error 1
  .../src/runtime/src/coreclr/pal/tests/palsuite/threading/WaitForSingleObject/WFSOExThreadTest/WFSOExThreadTest.cpp:167:10: error: use of undeclared identifier 'QueryPerformanceFrequency'
    167 |     if (!QueryPerformanceFrequency(&performanceFrequency))
        |          ^
  1 error generated.
  make[3]: *** [pal/tests/palsuite/CMakeFiles/paltests.dir/threading/Sleep/test2/sleep.cpp.o] Error 1
  make[3]: *** [pal/tests/palsuite/CMakeFiles/paltests.dir/threading/WaitForSingleObject/WFSOExThreadTest/WFSOExThreadTest.cpp.o] Error 1
  .../src/runtime/src/coreclr/pal/tests/palsuite/threading/WaitForSingleObject/WFSOExMutexTest/WFSOExMutexTest.cpp:188:10: error: use of undeclared identifier 'QueryPerformanceFrequency'
    188 |     if (!QueryPerformanceFrequency(&performanceFrequency))
        |          ^
  1 error generated.
  make[3]: *** [pal/tests/palsuite/CMakeFiles/paltests.dir/threading/WaitForSingleObject/WFSOExMutexTest/WFSOExMutexTest.cpp.o] Error 1
  make[2]: *** [pal/tests/palsuite/CMakeFiles/paltests.dir/all] Error 2
  make[1]: *** [CMakeFiles/paltests_install.dir/rule] Error 2
  make: *** [paltests_install] Error 2
  ~/src/runtime/src/coreclr
  Failed to build "CoreCLR component".
.../src/runtime/src/coreclr/runtime.proj(119,5): error MSB3073: The command ".../src/runtime/src/coreclr/"build-runtime.sh" -arm64 -checked -os osx -outputrid osx-arm64 -cmakeargs "-DCLR_DOTNET_RID=osx-arm64" -cmakeargs "-DCLR_DOTNET_HOST_PATH=.../src/runtime/.dotnet/dotnet" -component paltests" exited with code 2.

Build FAILED.

.../src/runtime/src/coreclr/runtime.proj(119,5): error MSB3073: The command ".../src/runtime/src/coreclr/"build-runtime.sh" -arm64 -checked -os osx -outputrid osx-arm64 -cmakeargs "-DCLR_DOTNET_RID=osx-arm64" -cmakeargs "-DCLR_DOTNET_HOST_PATH=.../src/runtime/.dotnet/dotnet" -component paltests" exited with code 2.
    0 Warning(s)
    1 Error(s)

[AaronRobinsonMSFT]

This is a very sloppy change :(

[huoyaoyuan]

It should be minipal_lowres_ticks indeed because it only uses ms precision.

[huoyaoyuan]

I tested PAL test build on local WSL before publishing the PR. Maybe some later commit gets it broken?

[AaronRobinsonMSFT]

I tested PAL test build on local WSL before publishing the PR. Maybe some later commit gets it broken?

Doubtful. The APIs were never removed. How did you build and run the tests?

I used ./build.sh -subset Clr.PalTests

[huoyaoyuan]

How did you build and run the tests?

I use CMake target view in Visual Studio and builds the paltests target. It does break for me for latest commit. I don't remember the exact commit I tested this.

[jkotas]

I thought that we have trigger to run PAL tests on PAL changes, but it is apparently not the case. I am sorry...