On Thu, 16 Feb 2023 09:18:58 GMT, Matthias Baesken <mbaes...@openjdk.org> wrote:
>> Enhance os::pd_print_cpu_info on macOS and Windows by information about CPU >> frequency and caches. >> On Windows , this info can be obtained by the Processor Power Information >> API or "powerbase" (CallNtPowerInformation , see >> https://learn.microsoft.com/en-us/windows/win32/api/powerbase/nf-powerbase-callntpowerinformation >> ); this is available since Windows Server 2003/XP. >> On macOS, sysctlbyname can be used. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Do not print idle state infos on Windows, print same MHz values for all > proces only once General approach seems okay. A couple of queries and nits. Thanks. src/hotspot/os/windows/os_windows.cpp line 1900: > 1898: > 1899: size_t sz_check = sizeof(PROCESSOR_POWER_INFORMATION) * > (size_t)proc_count; > 1900: NTSTATUS status = ::CallNtPowerInformation(ProcessorInformation, > NULL, 0, buf, (ULONG) buflen); How is the caller supposed to know what size buffer to pass? Are they just supposed to guess and hope it is big enough?? src/hotspot/os/windows/os_windows.cpp line 1901: > 1899: size_t sz_check = sizeof(PROCESSOR_POWER_INFORMATION) * > (size_t)proc_count; > 1900: NTSTATUS status = ::CallNtPowerInformation(ProcessorInformation, > NULL, 0, buf, (ULONG) buflen); > 1901: int MaxMhz = -1, CurrentMhz = -1, MhzLimit = -1; Nit: variable names should not start with Capital letters src/hotspot/os/windows/os_windows.cpp line 1915: > 1913: CurrentMhz != (int) pppi->CurrentMhz || > 1914: MhzLimit != (int) pppi->MhzLimit) { > 1915: same_vals_for_all_cpus = false; You could break once this is false. src/hotspot/os/windows/os_windows.cpp line 1919: > 1917: } > 1918: // avoid iteration in case buf is too small to hold all proc infos > 1919: if (sz_check > buflen) break; In this case shouldn't the function have returned `STATUS_BUFFER_TOO_SMALL`? src/hotspot/os/windows/os_windows.cpp line 1924: > 1922: > 1923: if (same_vals_for_all_cpus && MaxMhz != -1) { > 1924: st->print_cr("ProcessorInformation for all %d processors :", > proc_count); Nit: Processor Information src/hotspot/os/windows/os_windows.cpp line 1931: > 1929: pppi = (PROCESSOR_POWER_INFORMATION*) buf; > 1930: for (int i = 0; i < proc_count; i++) { > 1931: st->print_cr("ProcessorInformation for processor %d", (int) > pppi->Number); Nit: Processor Information ------------- PR: https://git.openjdk.org/jdk/pull/12403
