On 2022-04-02 16:04 +0100, Simon McVittie wrote: > On Sat, 02 Apr 2022 at 12:55:37 +0100, Wookey wrote: > > On 2022-04-01 00:30 -0400, M. Zhou wrote: > > > They have written > > > their own ffi loader, so I think it is an upstream bug. The upstream > > > should detect and add multiarch directory to the paths. > > > > A correct implemntation really should use the full ldconfig set of search > > paths. > > I think what they should actually be doing on Linux (at least in distro > packages) is taking a step back from all these attempts to reproduce > the system's search path for public shared libraries, and instead doing > this in https://github.com/apache/tvm/blob/main/python/tvm/_ffi/base.py: > > ctypes.CDLL('libtvm.so.0') > > which will (ask glibc to) do the correct path search, in something like > 99% less code.
Aha. That sounds much more the answer to the query in my original mail 'or is there a way to turn on 'just use the system paths' mode?'. This does indeed work to load the library without a lot of manual search-path generation, but it's a bit tricky to use in the existing codebase which wants the loader function to return both a name and a path, and with this magic loading, I don't know the path. _LIB, _LIB_NAME = _load_lib() The second parameter only seems to be used to determine whether libtvm or libtvm_runtime was loaded. I think I can work round that. OK. This patch appears to basically work: -- tvm-0.8.0.orig/python/tvm/_ffi/base.py +++ tvm-0.8.0/python/tvm/_ffi/base.py @@ -48,15 +48,21 @@ else: def _load_lib(): """Load libary by searching possible path.""" - lib_path = libinfo.find_lib_path() # The dll search path need to be added explicitly in # windows after python 3.8 if sys.platform.startswith("win32") and sys.version_info >= (3, 8): for path in libinfo.get_dll_directories(): os.add_dll_directory(path) - lib = ctypes.CDLL(lib_path[0], ctypes.RTLD_GLOBAL) + + use_runtime = os.environ.get("TVM_USE_RUNTIME_LIB", False) + if use_runtime: + lib = ctypes.CDLL('libtvm_runtime.so.0', ctypes.RTLD_GLOBAL) + sys.stderr.write("Loading runtime library %s... exec only\n" % lib._name) + sys.stderr.flush() + else: + lib = ctypes.CDLL('libtvm.so.0', ctypes.RTLD_GLOBAL) lib.TVMGetLastError.restype = ctypes.c_char_p - return lib, os.path.basename(lib_path[0]) + return lib try: @@ -68,10 +74,10 @@ except ImportError: # version number __version__ = libinfo.__version__ # library instance -_LIB, _LIB_NAME = _load_lib() +_LIB = _load_lib() # Whether we are runtime only -_RUNTIME_ONLY = "runtime" in _LIB_NAME +_RUNTIME_ONLY = "runtime" in _LIB._name Yay! _Unless_ you ask to use the runtime version - then it blows up. $ TVM_USE_RUNTIME_LIB=1 tvmc ... File "/usr/lib/python3/dist-packages/tvm/auto_scheduler/cost_model/cost_model.py", line 37, in __init__ self.__init_handle_by_constructor__(_ffi_api.RandomModel) AttributeError: module 'tvm.auto_scheduler._ffi_api' has no attribute 'RandomModel' I've not looked into that yet. Back to the issue of getting the path of the loaded library. Which I no longer obviously _need_, but I would like to know how to do it. There is ctypes.util.find_library(name) which says it returns a path but ctypes.util.find_library('tvm') just returns 'libtvm.so.0' I can't see an attribute in the returned lib object to get the path: lib=ctypes.CDLL('libtvm.so.0') >>> print(lib) <CDLL 'libtvm.so.0', handle 1409040 at 0x7f5f0ef48e20> >> print(lib.__dir__()) ['_name', '_FuncPtr', '_handle', '__module__', '__doc__', '_func_flags_', '_func_restype_', '__init__', '__repr__', '__getattr__', '__getitem__', '__dict__', '__weakref__', '__hash__', '__str__', '__getattribute__', '__setattr__', '__delattr__', '__lt__', '__le__', '__eq__', '__ne__', '__gt__', '__ge__', '__new__', '__reduce_ex__', '__reduce__', '__subclasshook__', '__init_subclass__', '__format__', '__sizeof__', '__dir__', '__class__'] But _something_ knows, because if I ask for an incorrect thing it prints it out as an 'AtributeError': >>> print(lib.wibble) Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/usr/lib/python3.9/ctypes/__init__.py", line 387, in __getattr__ func = self.__getitem__(name) File "/usr/lib/python3.9/ctypes/__init__.py", line 392, in __getitem__ func = self._FuncPtr((name_or_ordinal, self)) AttributeError: /usr/lib/x86_64-linux-gnu/libtvm.so.0: undefined symbol: wibble Again, my weak python-foo is holding me back here. Also, you suggest that for a disto package we just need to do lib = ctypes.CDLL('libtvm.so.0', ctypes.RTLD_GLOBAL) and scrap 99% of the code. However, some of the complicated logic in the special _load_lib code seems like maybe it shouldn't be thrown away. Like allowing to override the path with TVM_LIBRARY_PATH envvar, and setting TVM_USE_RUNTIME_LIB to load libtvm_runtime rather than libtvm (I've currently preserved the latter but not the former). I don't know why this is useful, but I assume upstream has reasons. I guess I should wave my patches upstream and see what they have to say about all this. > Maybe all this complexity is needed on Windows or in a "relocatable" > package, but for a distro package it's completely unnecessary and > sometimes harmful. Agreed. Although of course upstreams often want to support all sorts of 'local install' type circumstances so they have the code anyway, and we end up maintaining a patch to throw it all away and do something sensible instead. > > I also don't think it should use the $PATH paths for finding > > libraries (but maybe upstream have some reason for doing that) > > I suspect the reason is: on Windows, PATH is the equivalent of Linux PATH, > but it also has a dual role as the equivalent of Linux LD_LIBRARY_PATH. That would make sense except the relevant code is already conditional by OS: if sys.platform.startswith("linux") or sys.platform.startswith("freebsd"): dll_path.extend(split_env_var("LD_LIBRARY_PATH", ":")) dll_path.extend(split_env_var("PATH", ":")) elif sys.platform.startswith("darwin"): dll_path.extend(split_env_var("DYLD_LIBRARY_PATH", ":")) dll_path.extend(split_env_var("PATH", ":")) elif sys.platform.startswith("win32"): dll_path.extend(split_env_var("PATH", ";")) So I think it's actually just wrong. I'll take it upstream. Wookey -- Principal hats: Debian, Wookware, ARM http://wookware.org/