From 8bfffaaef784d8b87ddcb70bdb2376d43c1634e0 Mon Sep 17 00:00:00 2001 From: vansangpfiev Date: Tue, 10 Sep 2024 09:44:01 +0700 Subject: [PATCH 1/8] fix: add dll search path --- engine/controllers/server.cc | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/engine/controllers/server.cc b/engine/controllers/server.cc index 42a7595be..43ef21d43 100644 --- a/engine/controllers/server.cc +++ b/engine/controllers/server.cc @@ -21,11 +21,9 @@ constexpr static auto kTensorrtLlmEngine = "cortex.tensorrt-llm"; } // namespace server::server() { - - // Some default values for now below - // log_disable(); // Disable the log to file feature, reduce bloat for - // target - // system () +#if defined(WIN32) + SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_DEFAULT_DIRS); +#endif }; server::~server() {} @@ -293,7 +291,12 @@ void server::LoadModel(const HttpRequestPtr& req, (getenv("ENGINE_PATH") ? getenv("ENGINE_PATH") : file_manager_utils::GetCortexDataPath().string()) + get_engine_path(engine_type); - std::cout << abs_path << std::endl; +#if defined(WIN32) + auto ws = std::wstring(abs_path.begin(), abs_path.end()); + if (AddDllDirectory(ws.c_str()) == 0) { + CTL_WRN("Could not add dll directory: " << abs_path); + } +#endif engines_[engine_type].dl = std::make_unique(abs_path, "engine"); From bf0375f7eb0d2a55518308ab097d9abbf1648a13 Mon Sep 17 00:00:00 2001 From: vansangpfiev Date: Tue, 10 Sep 2024 15:01:13 +0700 Subject: [PATCH 2/8] fix: add dll search path, change cuda extraction path --- engine/commands/engine_init_cmd.cc | 24 +++++++++++--- engine/controllers/server.cc | 51 ++++++++++++++++++++++++++---- engine/controllers/server.h | 7 ++-- 3 files changed, 69 insertions(+), 13 deletions(-) diff --git a/engine/commands/engine_init_cmd.cc b/engine/commands/engine_init_cmd.cc index 7b0153f69..2984e5ea3 100644 --- a/engine/commands/engine_init_cmd.cc +++ b/engine/commands/engine_init_cmd.cc @@ -10,6 +10,7 @@ #include "utils/cuda_toolkit_utils.h" #include "utils/engine_matcher_utils.h" #if defined(_WIN32) || defined(__linux__) +#include "utils/cortex_utils.h" #include "utils/file_manager_utils.h" #endif @@ -125,7 +126,7 @@ bool EngineInitCmd::Exec() const { archive_utils::ExtractArchive(downloadedEnginePath.string(), extract_path.string()); -#if defined(_WIN32) || defined(__linux__) +#if defined(__linux__) // FIXME: hacky try to copy the file. Remove this when we are able to set the library path auto engine_path = extract_path / engineName_; LOG_INFO << "Source path: " << engine_path.string(); @@ -224,15 +225,30 @@ bool EngineInitCmd::Exec() const { download_service.AddDownloadTask( downloadCudaToolkitTask, - [](const std::string& absolute_path, bool unused) { + [this](const std::string& absolute_path, bool unused) { LOG_DEBUG << "Downloaded cuda path: " << absolute_path; // try to unzip the downloaded file std::filesystem::path downloaded_path{absolute_path}; - +#if defined(__linux__) archive_utils::ExtractArchive( absolute_path, downloaded_path.parent_path().parent_path().string()); - +#else + // TODO(any) This is a temporary fix. The issue will be fixed when we has CIs + // to pack CUDA dependecies into engine release + auto get_engine_path = [](std::string_view e) { + if (e == "cortex.llamacpp") { + return cortex_utils::kLlamaLibPath; + } else if (e == "cortex.tensorrt-llm") { + return cortex_utils::kTensorrtLlmPath; + } + return cortex_utils::kLlamaLibPath; + }; + std::string engine_path = + file_manager_utils::GetCortexDataPath().string() + + get_engine_path(engineName_); + archive_utils::ExtractArchive(absolute_path, engine_path); +#endif try { std::filesystem::remove(absolute_path); } catch (std::exception& e) { diff --git a/engine/controllers/server.cc b/engine/controllers/server.cc index 43ef21d43..1565ca224 100644 --- a/engine/controllers/server.cc +++ b/engine/controllers/server.cc @@ -7,8 +7,8 @@ #include "trantor/utils/Logger.h" #include "utils/cortex_utils.h" #include "utils/cpuid/cpu_info.h" -#include "utils/logging_utils.h" #include "utils/file_manager_utils.h" +#include "utils/logging_utils.h" using namespace inferences; using json = nlohmann::json; @@ -288,13 +288,47 @@ void server::LoadModel(const HttpRequestPtr& req, } std::string abs_path = - (getenv("ENGINE_PATH") ? getenv("ENGINE_PATH") - : file_manager_utils::GetCortexDataPath().string()) + + (getenv("ENGINE_PATH") + ? getenv("ENGINE_PATH") + : file_manager_utils::GetCortexDataPath().string()) + get_engine_path(engine_type); -#if defined(WIN32) - auto ws = std::wstring(abs_path.begin(), abs_path.end()); - if (AddDllDirectory(ws.c_str()) == 0) { - CTL_WRN("Could not add dll directory: " << abs_path); +#if defined(_WIN32) + // TODO(?) If we only allow to load an engine at a time, the logic is simpler. + // We would like to support running multiple engines at the same time. Therefore, + // the adding/removing dll directory logic is quite complicated: + // 1. If llamacpp is loaded and new requested engine is tensorrt-llm: + // Unload the llamacpp dll directory then load the tensorrt-llm + // 2. If tensorrt-llm is loaded and new requested engine is llamacpp: + // Do nothing, llamacpp can re-use tensorrt-llm dependencies (need to be tested careful) + // 3. Add dll directory if met other conditions + if (IsEngineLoaded(kLlamaEngine) && engine_type == kTensorrtLlmEngine) { + // Remove llamacpp dll directory + if (!RemoveDllDirectory(engines_[kLlamaEngine].cookie)) { + LOG_INFO << "Could not remove dll directory: " << kLlamaEngine; + } else { + LOG_INFO << "Removed dll directory: " << kLlamaEngine; + } + + auto ws = std::wstring(abs_path.begin(), abs_path.end()); + if (auto cookie = AddDllDirectory(ws.c_str()); cookie != 0) { + LOG_INFO << "Added dll directory: " << abs_path; + engines_[engine_type].cookie = cookie; + } else { + LOG_INFO << "Could not add dll directory: " << abs_path; + } + + } else if (IsEngineLoaded(kTensorrtLlmEngine) && + engine_type == kLlamaEngine) { + // Do nothing + } else { + auto ws = std::wstring(abs_path.begin(), abs_path.end()); + if (auto cookie = AddDllDirectory(ws.c_str()); cookie != 0) { + std::cout << "sangnv" << std::endl; + LOG_INFO << "Added dll directory: " << abs_path; + engines_[engine_type].cookie = cookie; + } else { + LOG_INFO << "Could not add dll directory: " << abs_path; + } } #endif engines_[engine_type].dl = @@ -352,6 +386,9 @@ void server::UnloadEngine( EngineI* e = std::get(engines_[engine_type].engine); delete e; +#if defined(_WIN32) + RemoveDllDirectory(engines_[engine_type].cookie); +#endif engines_.erase(engine_type); LOG_INFO << "Unloaded engine " + engine_type; Json::Value res; diff --git a/engine/controllers/server.h b/engine/controllers/server.h index 6d811192d..58d112435 100644 --- a/engine/controllers/server.h +++ b/engine/controllers/server.h @@ -17,11 +17,11 @@ #include #include "common/base.h" +#include "config/gguf_parser.h" +#include "config/yaml_config.h" #include "cortex-common/EngineI.h" #include "cortex-common/cortexpythoni.h" #include "trantor/utils/SerialTaskQueue.h" -#include "config/yaml_config.h" -#include "config/gguf_parser.h" #include "utils/dylib.h" #include "utils/json.hpp" #ifndef SERVER_VERBOSE @@ -153,6 +153,9 @@ class server : public drogon::HttpController, struct EngineInfo { std::unique_ptr dl; EngineV engine; +#if defined(_WIN32) + DLL_DIRECTORY_COOKIE cookie; +#endif }; std::unordered_map engines_; std::string cur_engine_type_; From d696c16915305c33dd3bfc3396aed57ae5e79082 Mon Sep 17 00:00:00 2001 From: vansangpfiev Date: Wed, 11 Sep 2024 08:19:45 +0700 Subject: [PATCH 3/8] fix: keep libs in engine directory --- engine/commands/engine_init_cmd.cc | 34 +----------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/engine/commands/engine_init_cmd.cc b/engine/commands/engine_init_cmd.cc index 2984e5ea3..ab995aa83 100644 --- a/engine/commands/engine_init_cmd.cc +++ b/engine/commands/engine_init_cmd.cc @@ -126,32 +126,6 @@ bool EngineInitCmd::Exec() const { archive_utils::ExtractArchive(downloadedEnginePath.string(), extract_path.string()); -#if defined(__linux__) - // FIXME: hacky try to copy the file. Remove this when we are able to set the library path - auto engine_path = extract_path / engineName_; - LOG_INFO << "Source path: " << engine_path.string(); - auto executable_path = - file_manager_utils::GetExecutableFolderContainerPath(); - for (const auto& entry : - std::filesystem::recursive_directory_iterator(engine_path)) { - if (entry.is_regular_file() && - entry.path().extension() != ".gz") { - std::filesystem::path relative_path = - std::filesystem::relative(entry.path(), engine_path); - std::filesystem::path destFile = - executable_path / relative_path; - - std::filesystem::create_directories(destFile.parent_path()); - std::filesystem::copy_file( - entry.path(), destFile, - std::filesystem::copy_options::overwrite_existing); - - std::cout << "Copied: " << entry.path().filename().string() - << " to " << destFile.string() << std::endl; - } - } - std::cout << "DLL copying completed successfully." << std::endl; -#endif // remove the downloaded file // TODO(any) Could not delete file on Windows because it is currently hold by httplib(?) @@ -229,12 +203,7 @@ bool EngineInitCmd::Exec() const { LOG_DEBUG << "Downloaded cuda path: " << absolute_path; // try to unzip the downloaded file std::filesystem::path downloaded_path{absolute_path}; -#if defined(__linux__) - archive_utils::ExtractArchive( - absolute_path, - downloaded_path.parent_path().parent_path().string()); -#else - // TODO(any) This is a temporary fix. The issue will be fixed when we has CIs + // TODO(any) This is a temporary fix. The issue will be fixed when we has CIs // to pack CUDA dependecies into engine release auto get_engine_path = [](std::string_view e) { if (e == "cortex.llamacpp") { @@ -248,7 +217,6 @@ bool EngineInitCmd::Exec() const { file_manager_utils::GetCortexDataPath().string() + get_engine_path(engineName_); archive_utils::ExtractArchive(absolute_path, engine_path); -#endif try { std::filesystem::remove(absolute_path); } catch (std::exception& e) { From 7501ddc4f1c28401f1cb06c2c746b24be92d54c7 Mon Sep 17 00:00:00 2001 From: vansangpfiev Date: Wed, 11 Sep 2024 08:23:19 +0700 Subject: [PATCH 4/8] fix: CI --- .github/workflows/cortex-cpp-quality-gate.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cortex-cpp-quality-gate.yml b/.github/workflows/cortex-cpp-quality-gate.yml index 49bc0637b..b10bbff2f 100644 --- a/.github/workflows/cortex-cpp-quality-gate.yml +++ b/.github/workflows/cortex-cpp-quality-gate.yml @@ -90,7 +90,7 @@ jobs: make package - name: Upload Artifact - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: cortex-cpp-${{ matrix.os }}-${{ matrix.name }} path: ./engine/cortex-cpp From c24961852b693c7fce5461eae0c510bb89e8c093 Mon Sep 17 00:00:00 2001 From: vansangpfiev Date: Wed, 11 Sep 2024 08:34:48 +0700 Subject: [PATCH 5/8] fix: clean code --- engine/controllers/server.cc | 37 ++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/engine/controllers/server.cc b/engine/controllers/server.cc index 1565ca224..d26d96eec 100644 --- a/engine/controllers/server.cc +++ b/engine/controllers/server.cc @@ -301,34 +301,31 @@ void server::LoadModel(const HttpRequestPtr& req, // 2. If tensorrt-llm is loaded and new requested engine is llamacpp: // Do nothing, llamacpp can re-use tensorrt-llm dependencies (need to be tested careful) // 3. Add dll directory if met other conditions + + auto add_dll = [this](const std::string& e_type, const std::string& p) { + auto ws = std::wstring(p.begin(), p.end()); + if (auto cookie = AddDllDirectory(ws.c_str()); cookie != 0) { + LOG_INFO << "Added dll directory: " << p; + engines_[e_type].cookie = cookie; + } else { + LOG_WARN << "Could not add dll directory: " << p; + } + }; + if (IsEngineLoaded(kLlamaEngine) && engine_type == kTensorrtLlmEngine) { // Remove llamacpp dll directory if (!RemoveDllDirectory(engines_[kLlamaEngine].cookie)) { LOG_INFO << "Could not remove dll directory: " << kLlamaEngine; } else { - LOG_INFO << "Removed dll directory: " << kLlamaEngine; - } - - auto ws = std::wstring(abs_path.begin(), abs_path.end()); - if (auto cookie = AddDllDirectory(ws.c_str()); cookie != 0) { - LOG_INFO << "Added dll directory: " << abs_path; - engines_[engine_type].cookie = cookie; - } else { - LOG_INFO << "Could not add dll directory: " << abs_path; + LOG_WARN << "Removed dll directory: " << kLlamaEngine; } + add_dll(engine_type, abs_path); } else if (IsEngineLoaded(kTensorrtLlmEngine) && engine_type == kLlamaEngine) { // Do nothing } else { - auto ws = std::wstring(abs_path.begin(), abs_path.end()); - if (auto cookie = AddDllDirectory(ws.c_str()); cookie != 0) { - std::cout << "sangnv" << std::endl; - LOG_INFO << "Added dll directory: " << abs_path; - engines_[engine_type].cookie = cookie; - } else { - LOG_INFO << "Could not add dll directory: " << abs_path; - } + add_dll(engine_type, abs_path); } #endif engines_[engine_type].dl = @@ -387,7 +384,11 @@ void server::UnloadEngine( EngineI* e = std::get(engines_[engine_type].engine); delete e; #if defined(_WIN32) - RemoveDllDirectory(engines_[engine_type].cookie); + if (!RemoveDllDirectory(engines_[engine_type].cookie)) { + LOG_WARN << "Could not remove dll directory: " << engine_type; + } else { + LOG_INFO << "Removed dll directory: " << engine_type; + } #endif engines_.erase(engine_type); LOG_INFO << "Unloaded engine " + engine_type; From 315a5b11c7d02df9fb3659b0a1078cb64ea7cd99 Mon Sep 17 00:00:00 2001 From: vansangpfiev Date: Wed, 11 Sep 2024 08:36:15 +0700 Subject: [PATCH 6/8] fix: more --- engine/controllers/server.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/controllers/server.cc b/engine/controllers/server.cc index d26d96eec..ee9951968 100644 --- a/engine/controllers/server.cc +++ b/engine/controllers/server.cc @@ -21,7 +21,7 @@ constexpr static auto kTensorrtLlmEngine = "cortex.tensorrt-llm"; } // namespace server::server() { -#if defined(WIN32) +#if defined(_WIN32) SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_DEFAULT_DIRS); #endif }; From 1a6d2149105b400be6a5b33ba566526ed4abe380 Mon Sep 17 00:00:00 2001 From: vansangpfiev Date: Wed, 11 Sep 2024 09:19:37 +0700 Subject: [PATCH 7/8] fix: remove if def --- engine/commands/engine_init_cmd.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/engine/commands/engine_init_cmd.cc b/engine/commands/engine_init_cmd.cc index ab995aa83..ac8dde239 100644 --- a/engine/commands/engine_init_cmd.cc +++ b/engine/commands/engine_init_cmd.cc @@ -7,12 +7,10 @@ #include "utils/archive_utils.h" #include "utils/system_info_utils.h" // clang-format on +#include "utils/cortex_utils.h" #include "utils/cuda_toolkit_utils.h" #include "utils/engine_matcher_utils.h" -#if defined(_WIN32) || defined(__linux__) -#include "utils/cortex_utils.h" #include "utils/file_manager_utils.h" -#endif namespace commands { From 00fbe871766a851c7aa1c96be32860419b661a74 Mon Sep 17 00:00:00 2001 From: vansangpfiev Date: Thu, 12 Sep 2024 08:45:32 +0700 Subject: [PATCH 8/8] fix: comment --- engine/commands/engine_init_cmd.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/engine/commands/engine_init_cmd.cc b/engine/commands/engine_init_cmd.cc index ac8dde239..743b39cdc 100644 --- a/engine/commands/engine_init_cmd.cc +++ b/engine/commands/engine_init_cmd.cc @@ -206,10 +206,9 @@ bool EngineInitCmd::Exec() const { auto get_engine_path = [](std::string_view e) { if (e == "cortex.llamacpp") { return cortex_utils::kLlamaLibPath; - } else if (e == "cortex.tensorrt-llm") { + } else { return cortex_utils::kTensorrtLlmPath; } - return cortex_utils::kLlamaLibPath; }; std::string engine_path = file_manager_utils::GetCortexDataPath().string() +