kunzmi / managedCuda

ManagedCUDA aims an easy integration of NVidia's CUDA in .net applications written in C#, Visual Basic or any other .net language.
Other
440 stars 79 forks source link

NVML observations #29

Closed mgravell closed 7 years ago

mgravell commented 7 years ago

Hi - love the lib. I've recently needed some of the APIs from nvml, and had a few observations:

Am I missing something obvious on packaging for NVML?

I appreciate that NVML is essentially a naked wrapper on the native methods, but it works great at what it does. However, as an additional observation: many many of the "get" methods use ref in the signature, when the semantic is actually out. For example:

uint count;
NvmlNativeMethods.nvmlDeviceGetCount(out count).Verify();

vs

uint count = 0; // dummy
NvmlNativeMethods.nvmlDeviceGetCount(ref count).Verify();

Simply changing the extern declaration makes this work.

(note that .Verify() here is simply an extension method on the result enum)

This would obviously be a breaking change, as would be changing the library name, but: any thoughts?

kunzmi commented 7 years ago

Well I planned more for NVML, but time is missing... For now the library consists only of the raw API and is not even tested. That's why there's no released package on Nuget. For the ref vs. out question: For p/invoke it is better to use ref instead of out, as you have no guarantee that the API actually sets the value. Staying inside C# the compiler can check that and outputs an error if the variable is not set, p/invoke can't do that. Using out and the function doesn't set the variable, e.g. because of an error, leaves the value of the variable undefined. Using ref just leaves the variable untouched and the previous value is still valid.

mgravell commented 7 years ago

For now the library consists only of the raw API and is not even tested. That's why there's no released package on Nuget.

Fair enough

Well I planned more for NVML, but time is missing...

I totally hear you; I maintain multiple libs, and I fully get how time demands are - and how what you need is going to influence the available surface.

For p/invoke it is better to use ref instead of out, as you have no guarantee that the API actually sets the value.

That's actually true inside .NET too, for anyone who happens to be evil enough :) (although if you use C# or VB, the compiler won't let you be evil). But yes, I acknowledge that it is a much more genuine problem for P/Invoke.

Feel free to close; I just felt the feedback might be useful. But: great work.

kunzmi commented 7 years ago

package is now named managedNVML.dll