Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

windows: provide more accurate result for available_parallelism if cores > 64 #114856

Closed
wants to merge 2 commits into from
Closed

windows: provide more accurate result for available_parallelism if cores > 64 #114856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ebd39e5
windows: provide more accurate result for available_parallelism if co…
klensy Aug 15, 2023
4dfdc9d
miri: teach about GetActiveProcessorCount
klensy Aug 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion library/std/src/sys/windows/c/windows_sys.lst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2477,11 +2477,11 @@ Windows.Win32.System.Pipes.PIPE_TYPE_BYTE
Windows.Win32.System.Pipes.PIPE_TYPE_MESSAGE
Windows.Win32.System.Pipes.PIPE_WAIT
Windows.Win32.System.SystemInformation.GetSystemDirectoryW
Windows.Win32.System.SystemInformation.GetSystemInfo
Windows.Win32.System.SystemInformation.GetSystemTimeAsFileTime
Windows.Win32.System.SystemInformation.GetWindowsDirectoryW
Windows.Win32.System.SystemInformation.PROCESSOR_ARCHITECTURE
Windows.Win32.System.SystemInformation.SYSTEM_INFO
Windows.Win32.System.SystemServices.ALL_PROCESSOR_GROUPS
Windows.Win32.System.SystemServices.DLL_PROCESS_DETACH
Windows.Win32.System.SystemServices.DLL_THREAD_DETACH
Windows.Win32.System.SystemServices.EXCEPTION_MAXIMUM_PARAMETERS
Expand Down Expand Up @@ -2513,6 +2513,7 @@ Windows.Win32.System.Threading.DEBUG_PROCESS
Windows.Win32.System.Threading.DETACHED_PROCESS
Windows.Win32.System.Threading.ExitProcess
Windows.Win32.System.Threading.EXTENDED_STARTUPINFO_PRESENT
Windows.Win32.System.Threading.GetActiveProcessorCount
Windows.Win32.System.Threading.GetCurrentProcess
Windows.Win32.System.Threading.GetCurrentProcessId
Windows.Win32.System.Threading.GetCurrentThread
Expand Down
9 changes: 5 additions & 4 deletions library/std/src/sys/windows/c/windows_sys.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,10 @@ extern "system" {
pub fn FreeEnvironmentStringsW(penv: PCWSTR) -> BOOL;
}
#[link(name = "kernel32")]
extern "system" {
pub fn GetActiveProcessorCount(groupnumber: u16) -> u32;
}
#[link(name = "kernel32")]
extern "system" {
pub fn GetCommandLineW() -> PCWSTR;
}
Expand Down Expand Up @@ -338,10 +342,6 @@ extern "system" {
pub fn GetSystemDirectoryW(lpbuffer: PWSTR, usize: u32) -> u32;
}
#[link(name = "kernel32")]
extern "system" {
pub fn GetSystemInfo(lpsysteminfo: *mut SYSTEM_INFO) -> ();
}
#[link(name = "kernel32")]
extern "system" {
pub fn GetSystemTimeAsFileTime(lpsystemtimeasfiletime: *mut FILETIME) -> ();
}
Expand Down Expand Up @@ -822,6 +822,7 @@ impl ::core::clone::Clone for ADDRINFOA {
pub const AF_INET: ADDRESS_FAMILY = 2u16;
pub const AF_INET6: ADDRESS_FAMILY = 23u16;
pub const AF_UNSPEC: ADDRESS_FAMILY = 0u16;
pub const ALL_PROCESSOR_GROUPS: u32 = 65535u32;
#[repr(C)]
pub union ARM64_NT_NEON128 {
pub Anonymous: ARM64_NT_NEON128_0,
Expand Down
12 changes: 5 additions & 7 deletions library/std/src/sys/windows/thread.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,13 @@ impl Thread {

pub fn available_parallelism() -> io::Result<NonZeroUsize> {
let res = unsafe {
let mut sysinfo: c::SYSTEM_INFO = crate::mem::zeroed();
c::GetSystemInfo(&mut sysinfo);
sysinfo.dwNumberOfProcessors as usize
// FIXME: windows::Win32::System::SystemServices::ALL_PROCESSOR_GROUPS should be u16, not u32
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

microsoft/win32metadata@8f70180
will be changed to Windows.Win32.System.Threading.Apis.ALL_PROCESSOR_GROUPS

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... I've been thinking about this some more. Could you move the correct definition of ALL_PROCESSOR_GROUPS into c.rs? That way we can keep all the FIXMEs into one file so they're easier to find and fix when the bindings are next updated. Otherwise it might be easier to forget about it.

// FIXME: if you need even more accurate result on 32-bit Windows with more, than 64 cores,
// consider implementing this using GetLogicalProcessorInformationEx
c::GetActiveProcessorCount(c::ALL_PROCESSOR_GROUPS as u16) as usize
};
match res {
0 => Err(io::const_io_error!(
io::ErrorKind::NotFound,
"The number of hardware threads is not known for the target platform",
)),
0 => Err(io::Error::last_os_error()),
cpus => Ok(unsafe { NonZeroUsize::new_unchecked(cpus) }),
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/tools/miri/src/shims/windows/foreign_items.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
)?;
}

// Number of cores, but more accurate vaulue, than GetSystemInfo.dwNumberOfProcessors
// if number of cores > 64. If called with ALL_PROCESSOR_GROUPS as groupnumber,
// returns total number of cores, instead of number in group, see
// https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-getactiveprocessorcount
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds like we should do throw_unsup_format! if the groupnumber is not ALL_PROCESSOR_GROUPS, since we are not correctly handling that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vaulue typo.

"GetActiveProcessorCount" => {
let [groupnumber] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
this.read_scalar(groupnumber)?.to_u16()?;
this.write_scalar(Scalar::from_u32(this.machine.num_cpus), dest)?;
}

// Thread-local storage
"TlsAlloc" => {
// This just creates a key; Windows does not natively support TLS destructors.
Expand Down
Loading