nvml-wrapper icon indicating copy to clipboard operation
nvml-wrapper copied to clipboard

feature request: add support for `nvmlDeviceGetGraphicsRunningProcesses_v2`

Open KisaragiEffective opened this issue 3 years ago • 2 comments

Hello! I'm one of the users, and found out that this crate does not support nvmlDeviceGetGraphicsRunningProcesses_v2. My personal computer runs on debian 11 (and there's managed package called nvidia-driver), which is v470 and lacks support of nvmlDeviceGetGraphicsRunningProcesses_v3:

$ nm -D /usr/lib/x86_64-linux-gnu/libnvidia-ml.so.1 | grep nvmlDeviceGetGraphicsRunningProcesses
0000000000051370 T nvmlDeviceGetGraphicsRunningProcesses
0000000000051570 T nvmlDeviceGetGraphicsRunningProcesses_v2

So, I purpose add two method: running_graphics_processes_v2 and running_graphics_processes_count_v2. They'll have same return type as running_graphics_processes and running_graphics_processes_count as well.

Here's my current code (you can use this: I wrote this based on your code, so I'll leave this as MIT OR Apache-2.0 license)

Would you mind if I submit PR for this? Thank you.

Code

Code
trait GetRunningGraphicsProcessesV2 {
    fn running_graphics_processes_v2(&self) -> Result<Vec<ProcessInfo>, NvmlError>;

    fn running_graphics_processes_count_v2(&self) -> Result<u32, NvmlError>;
}

type GetProcessV2Sig = unsafe extern "C" fn(
    device: nvmlDevice_t,
    #[allow(non_snake_case)]
    infoCount: *mut c_uint,
    infos: *mut nvmlProcessInfo_t,
) -> nvmlReturn_t;

static GET_PS_V2: Lazy<GetProcessV2Sig> = Lazy::new(|| {
    unsafe {
        let lib = Library::new("libnvidia-ml.so").unwrap();
        let f: GetProcessV2Sig = lib.get(b"nvmlDeviceGetGraphicsRunningProcesses_v2\0").map(|a| *a).unwrap();
        f
    }
});

impl GetRunningGraphicsProcessesV2 for Device<'_> {
    fn running_graphics_processes_v2(&self) -> Result<Vec<ProcessInfo>, NvmlError> {
        let sym = *GET_PS_V2;

        let mut count: c_uint = match self.running_graphics_processes_count_v2()? {
            0 => return Ok(vec![]),
            value => value,
        };
        // Add a bit of headroom in case more processes are launched in
        // between the above call to get the expected count and the time we
        // actually make the call to get data below.
        count += 5;
        let mem = unsafe { mem::zeroed() };
        let mut processes: Vec<nvmlProcessInfo_t> = vec![mem; count as usize];

        let device = unsafe { self.handle() };
        nvml_try(sym(device, &mut count, processes.as_mut_ptr()))?;
        processes.truncate(count as usize);

        Ok(processes.into_iter().map(ProcessInfo::from).collect())
    }

    fn running_graphics_processes_count_v2(&self) -> Result<u32, NvmlError> {
        let sym = *GET_PS_V2;


        // Indicates that we want the count
        let mut count: c_uint = 0;

        let device = unsafe { self.handle() };
        // Passing null doesn't indicate that we want the count. It's just allowed.
        match sym(device, &mut count, null_mut()) {
            nvmlReturn_enum_NVML_ERROR_INSUFFICIENT_SIZE => Ok(count),
            // If success, return 0; otherwise, return error
            other => nvml_try(other).map(|_| 0),
        }
    }
}

KisaragiEffective avatar Aug 17 '22 13:08 KisaragiEffective

Hey! I have seen this and I will give it some thought, but I won't have time to reply for another couple of days 🙂.

Cldfire avatar Aug 19 '22 03:08 Cldfire

Thank you for your reply! Please feel free to tell me it when you're free.

KisaragiEffective avatar Aug 19 '22 08:08 KisaragiEffective

@Cldfire How is it going? May I submit one?

KisaragiEffective avatar Oct 12 '22 10:10 KisaragiEffective

Sorry for taking so long to reply! I got busy with life stuff.

So, I've put some thought into this and I have two main concerns here:

  • In order for me to accept a PR for this we would need to end up with nvmlDeviceGetGraphicsRunningProcesses_v2 being generated and present as a symbol in nvml-wrapper-sys/src/bindings.rs. I don't want to mess around with opening and loading a second library definition to support legacy function calls. Doing this will require making sure bindgen generates bindings for the functions in this ifdef, and making sure that's accomplished without modifying nvml.h in the source tree:

    https://github.com/Cldfire/nvml-wrapper/blob/1e943a42d2362f6aa1ad84d41f6e54af2fa61187/nvml-wrapper-sys/nvml.h#L9106-L9129

  • I don't want to clog editor autocomplete with the names of legacy methods. This means avoiding a situation where the default autocomplete experience when using this library is seeing the following:

    device.running_graphics_<|>
    
        ------------------------------------------
        device.running_graphics_processes()
        device.running_graphics_processes_v1()
        device.running_graphics_processes_v2()
        ------------------------------------------
    

    This excludes a trait-based approach for bringing these into scope because rust-analyzer is smart enough to discover method calls that could be made available by importing a trait and suggest those for autocomplete. The only way to preserve a good default autocomplete experience here would be to have a crate feature such as "legacy-functions" that could be enabled to compile support for older function calls in the wrapper.

Cldfire avatar Nov 26 '22 02:11 Cldfire

@KisaragiEffective I put together a branch meeting both of the constraints I laid out above. It's in a PR here: https://github.com/Cldfire/nvml-wrapper/pull/39

Please give that a look and let me know if it would satisfy your needs :slightly_smiling_face:.

Cldfire avatar Nov 26 '22 04:11 Cldfire