-
Notifications
You must be signed in to change notification settings - Fork 106
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.
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
[WIP] Dynamic backend #159
Conversation
…or the solution.
Alright, tests pass! Only the packaging fails (see below) So, right now the build process includes a
Everything is built with Given this layout it's easy to introduce multiple versions for each backend. We just need to choose proper names for the backends As I was mentioning earlier, the only missing bit for this to be merged is updating the |
Backends can now be loaded through command-line arguments, eg
|
Regarding RPATH/RUNPATH:
Similirly, some of the backends .so should be patched similarly. |
Regarding the deployment scheme:
From this, it's possible to see how we run into unclear build/release process, not to mention backend packaging. The other option, RedisAI-centric release model, we place all artifacts under one versioned directory:
This will greatly simplify release and deployment, making mix-and-match backends a little harder, but not impossible. |
Hi Rafi, We can't put all .so in a single directory, because we need to be able to choose between alternative versions of, say, I was thinking something in-between:
So every redisai version has its set of backends, which in turn are versioned across versions and variants. This layout would keep things tidy and expose just the files users are interested about (since they will need to LOADBACKEND the .so file). |
Wouldn't the following be more concise?
I just wanted all shared object of a given backend (of a particular version/variant, including their dependent libraries) to reside on the same directory, to minimize complexity of RUNPATH hacking. I was also wondering if it will be possible to introduce the patchelf post-build steps after we integrate my PR with the ARM work and build system modifications. |
Hi @rafie, thank you. It is fewer directories, but it would be more challenging for users to find the right backend .so file to load. I think the other way would facilitate pointing users to the right file. By the way, with the libraries shipped by vendors (libtensorflow etc) I didn’t find a need to modify their rpath, only the rpath for the backend so (redisai_tensorflow.so). Tests pass in Linux, so I was wondering if the parchelf is a special need for the arm build or it’s general (sorry I’m not an expert in this). Last: would it be possible for you to fix the packaging step, otherwise I won’t be able to merge. Thank you for your help! |
off-topic, rafi != rafie |
Ouch, sorry @rafi! |
So @rafie, this is my final proposal
It's basically your last proposal but we place the library dependencies under Are you ok with this? |
We can modify CMakeLists.txt the following way to replace RPATH with RUNPATH:
|
@rafie Done |
[WIP] Dynamic backend (#159) * Cleanup backend headers * Move backends to libraries * Load backends dynamically * Add error handling * Make RAI_backends non-static * Block backends from being loaded if already loaded previously * Load missing symbols in backend wrappers. Thanks to @MeirShpilraien for the solution. * Add backend loaded logs * Reorganize backend layout and RPATH management * Update CI to reflect latest changes * Rename install directories * Update README and doc * Allow loading backends as command-line arguments to module * Update Dockerfiles * New directory layout * Fix ramp package * Fix path to artifact on CI * Copy recursively * Enable new-dtags for Linux build * Update documentation * Add LOADBACKEND to DEL tests
This PR addresses #36 (and #47)
It introduces the
AI.CONFIG
command with aLOADBACKEND
subcommand (the only one for now but there will likely be more).Backend code in the
src/backends/
directory are compiled to their individual.so
libraries and not loaded when theAI
module is loaded. Instead, the user loads backends on demand by specifying the backend type (TF
,TORCH
orONNX
) and the library for the backend (which could be pointing to any CPU, GPU version thereof, provided they were compiled by linking to the backends).To avoid pollution, we allow only one backend to be loaded once per backend type. That is, you can't have two versions of
TORCH
loaded in the same instance at the same time. This is also in relation to what happens when CUDA kernels are loaded on the GPU. Any more sophisticated behavior will have to be tested. For the same reason, unloading hasn't been implemented yet.Backend loading works as follows:
WIPThe PR is mostly done, but I get a segfault when trying to call RedisModule_Calloc from within the dynamically loaded backend (both on macOS and Linux). I need to create a minimal repro code (e.g. based on the hello world module examples in Redis) so that it's easier to understand what's going on for anyone who'd like to help.UPDATE: solved, thanks @MeirShpilraien!
Tests haven't been updated yet (so they will fail).