Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,18 @@ class DynamicLibNotFoundError(RuntimeError):
pass


@dataclass
class FoundVia:
name: str
version: Optional[str] = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently not used: I'd lean towards not adding that in this PR, but only when we have an actual use case, which might give us ideas for a different name (or names).



@dataclass
class LoadedDL:
abs_path: Optional[str]
was_already_loaded_from_elsewhere: bool
_handle_uint: int # Platform-agnostic unsigned pointer value
foundvia: Optional[FoundVia] = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

found_via highly preferred (reads better, and seems to be most common when mapping from camel case to snake case).



def load_dependencies(libname: str, load_func: Callable[[str], LoadedDL]) -> None:
Expand Down
14 changes: 8 additions & 6 deletions cuda_pathfinder/cuda/pathfinder/_dynamic_libs/load_dl_linux.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import os
from typing import Optional, cast

from cuda.pathfinder._dynamic_libs.load_dl_common import LoadedDL
from cuda.pathfinder._dynamic_libs.load_dl_common import FoundVia, LoadedDL
from cuda.pathfinder._dynamic_libs.supported_nvidia_libs import (
LIBNAMES_REQUIRING_RTLD_DEEPBIND,
SUPPORTED_LINUX_SONAMES,
Expand Down Expand Up @@ -131,14 +131,16 @@ def get_candidate_sonames(libname: str) -> list[str]:
return candidate_sonames


def check_if_already_loaded_from_elsewhere(libname: str, _have_abs_path: bool) -> Optional[LoadedDL]:
def check_if_already_loaded_from_elsewhere(
libname: str, _have_abs_path: bool, found_via: Optional[FoundVia] = None
) -> Optional[LoadedDL]:
for soname in get_candidate_sonames(libname):
try:
handle = ctypes.CDLL(soname, mode=os.RTLD_NOLOAD)
except OSError:
continue
else:
return LoadedDL(abs_path_for_dynamic_library(libname, handle), True, handle._handle)
return LoadedDL(abs_path_for_dynamic_library(libname, handle), True, handle._handle, found_via)
return None


Expand Down Expand Up @@ -170,7 +172,7 @@ def load_with_system_search(libname: str) -> Optional[LoadedDL]:
abs_path = abs_path_for_dynamic_library(libname, handle)
if abs_path is None:
raise RuntimeError(f"No expected symbol for {libname=!r}")
return LoadedDL(abs_path, False, handle._handle)
return LoadedDL(abs_path, False, handle._handle, FoundVia("system"))
return None


Expand All @@ -193,7 +195,7 @@ def _work_around_known_bugs(libname: str, found_path: str) -> None:
ctypes.CDLL(dep_path, CDLL_MODE)


def load_with_abs_path(libname: str, found_path: str) -> LoadedDL:
def load_with_abs_path(libname: str, found_path: str, found_via: Optional[FoundVia] = None) -> LoadedDL:
"""Load a dynamic library from the given path.

Args:
Expand All @@ -211,4 +213,4 @@ def load_with_abs_path(libname: str, found_path: str) -> LoadedDL:
handle = _load_lib(libname, found_path)
except OSError as e:
raise RuntimeError(f"Failed to dlopen {found_path}: {e}") from e
return LoadedDL(found_path, False, handle._handle)
return LoadedDL(found_path, False, handle._handle, found_via)
14 changes: 8 additions & 6 deletions cuda_pathfinder/cuda/pathfinder/_dynamic_libs/load_dl_windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import struct
from typing import Optional

from cuda.pathfinder._dynamic_libs.load_dl_common import LoadedDL
from cuda.pathfinder._dynamic_libs.load_dl_common import FoundVia, LoadedDL
from cuda.pathfinder._dynamic_libs.supported_nvidia_libs import (
LIBNAMES_REQUIRING_OS_ADD_DLL_DIRECTORY,
SUPPORTED_WINDOWS_DLLS,
Expand Down Expand Up @@ -100,7 +100,9 @@ def abs_path_for_dynamic_library(libname: str, handle: ctypes.wintypes.HMODULE)
return buffer.value


def check_if_already_loaded_from_elsewhere(libname: str, have_abs_path: bool) -> Optional[LoadedDL]:
def check_if_already_loaded_from_elsewhere(
libname: str, have_abs_path: bool, found_via: Optional[FoundVia] = None
) -> Optional[LoadedDL]:
for dll_name in SUPPORTED_WINDOWS_DLLS.get(libname, ()):
handle = kernel32.GetModuleHandleW(dll_name)
if handle:
Expand All @@ -110,7 +112,7 @@ def check_if_already_loaded_from_elsewhere(libname: str, have_abs_path: bool) ->
# load_with_abs_path(). To make the side-effect more deterministic,
# activate it even if the library was already loaded from elsewhere.
add_dll_directory(abs_path)
return LoadedDL(abs_path, True, ctypes_handle_to_unsigned_int(handle))
return LoadedDL(abs_path, True, ctypes_handle_to_unsigned_int(handle), found_via)
return None


Expand All @@ -128,12 +130,12 @@ def load_with_system_search(libname: str) -> Optional[LoadedDL]:
handle = kernel32.LoadLibraryExW(dll_name, None, 0)
if handle:
abs_path = abs_path_for_dynamic_library(libname, handle)
return LoadedDL(abs_path, False, ctypes_handle_to_unsigned_int(handle))
return LoadedDL(abs_path, False, ctypes_handle_to_unsigned_int(handle), FoundVia("system"))

return None


def load_with_abs_path(libname: str, found_path: str) -> LoadedDL:
def load_with_abs_path(libname: str, found_path: str, found_via: Optional[FoundVia] = None) -> LoadedDL:
"""Load a dynamic library from the given path.

Args:
Expand All @@ -156,4 +158,4 @@ def load_with_abs_path(libname: str, found_path: str) -> LoadedDL:
error_code = ctypes.GetLastError() # type: ignore[attr-defined]
raise RuntimeError(f"Failed to load DLL at {found_path}: Windows error {error_code}")

return LoadedDL(found_path, False, ctypes_handle_to_unsigned_int(handle))
return LoadedDL(found_path, False, ctypes_handle_to_unsigned_int(handle), found_via)
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import sys

from cuda.pathfinder._dynamic_libs.find_nvidia_dynamic_lib import _FindNvidiaDynamicLib
from cuda.pathfinder._dynamic_libs.load_dl_common import LoadedDL, load_dependencies
from cuda.pathfinder._dynamic_libs.load_dl_common import FoundVia, LoadedDL, load_dependencies
from cuda.pathfinder._utils.platform_aware import IS_WINDOWS

if IS_WINDOWS:
Expand All @@ -26,13 +26,20 @@
def _load_lib_no_cache(libname: str) -> LoadedDL:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be nice to move this below the class _LoadNvidiaDynamicLib code (it's more in line with how I organized the rest of the pathfinder code).

Also, this could be a one-liner:

    return _LoadNvidiaDynamicLib(libname).load_lib()

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suspect it'll be simpler (less code) to wrap this function in a helper that adds found_via into LoadedDL.

def _load_lib_no_cache_impl(libname: str) -> LoadedDL, str
    existing code, but we also keep track of `found_via` and return (loaded, found_via)
def _load_lib_no_cache(libname: str) -> LoadedDL:
    loaded, found_via = _load_lib_no_cache_impl(libname)
    loaded.found_via = FoundVia(found_via)
    return loaded

Alternatively, we could pass found_via down along with abs_path, so that the LoadedDL instances can be constructed with it, rather than retrofitting it in later. That's a bit more surgical, not sure how it'll shake out, but it would seem cleanest.

finder = _FindNvidiaDynamicLib(libname)
abs_path = finder.try_site_packages()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor but would be nice: keep the found_via assignment closer the the source of the abs_path:

    abs_path = finder.try_site_packages()
    if abs_path is not None:
        found_via = "site-packages"
    else:
        abs_path = finder.try_with_conda_prefix()
        if abs_path is not None:
            found_via = "conda"


if abs_path is None:
abs_path = finder.try_with_conda_prefix()
if abs_path is not None:
found_via = FoundVia("conda")
else:
found_via = FoundVia("site-packages")

# If the library was already loaded by someone else, reproduce any OS-specific
# side-effects we would have applied on a direct absolute-path load (e.g.,
# AddDllDirectory on Windows for libs that require it).
loaded = check_if_already_loaded_from_elsewhere(libname, abs_path is not None)
loaded = check_if_already_loaded_from_elsewhere(
libname, abs_path is not None, found_via if abs_path is not None else None
)

# Load dependencies regardless of who loaded the primary lib first.
# Doing this *after* the side-effect ensures dependencies resolve consistently
Expand All @@ -49,8 +56,10 @@ def _load_lib_no_cache(libname: str) -> LoadedDL:
abs_path = finder.try_with_cuda_home()
if abs_path is None:
finder.raise_not_found_error()
else:
found_via = FoundVia("CUDA_HOME")

return load_with_abs_path(libname, abs_path)
return load_with_abs_path(libname, abs_path, found_via)


@functools.cache
Expand Down