From 69b3147a30ad79aa46b71d11393d4432ffa4c3a8 Mon Sep 17 00:00:00 2001 From: koubaa Date: Wed, 11 Dec 2024 10:59:58 -0600 Subject: [PATCH 1/4] Fix race condition with python logging Signed-off-by: koubaa --- python/src/main.cpp | 24 ++++++++++++++++++++++++ src/include/kompute/Core.hpp | 7 ++++++- src/include/kompute/logger/Logger.hpp | 20 +++++++++++--------- 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/python/src/main.cpp b/python/src/main.cpp index 3bddce88..5a3a4441 100644 --- a/python/src/main.cpp +++ b/python/src/main.cpp @@ -14,6 +14,30 @@ namespace py = pybind11; // used in Core.hpp py::object kp_trace, kp_debug, kp_info, kp_warning, kp_error; +static void kp_log(int level, const std::string& msg) { + py::gil_scoped_acquire gil; + switch (level) { + case 0: + kp_trace(msg); break; + case 1: + kp_debug(msg); break; + case 2: + kp_info(msg); break; + case 3: + kp_warning(msg); break; + case 4: + kp_error(msg); break; + default: + break; + } +} + +void py_log_trace(const std::string& msg) {kp_log(0, msg);} +void py_log_debug(const std::string& msg) {kp_log(1, msg);} +void py_log_info(const std::string& msg) {kp_log(2, msg);} +void py_log_warning(const std::string& msg) {kp_log(3, msg);} +void py_log_error(const std::string& msg) {kp_log(4, msg);} + std::unique_ptr opAlgoDispatchPyInit(std::shared_ptr& algorithm, const py::array& push_consts) diff --git a/src/include/kompute/Core.hpp b/src/include/kompute/Core.hpp index 2384e47b..d347a827 100644 --- a/src/include/kompute/Core.hpp +++ b/src/include/kompute/Core.hpp @@ -26,5 +26,10 @@ typedef std::vector Constants; #include namespace py = pybind11; // from python/src/main.cpp -extern py::object kp_trace, kp_debug, kp_info, kp_warning, kp_error; + +extern void py_log_trace(const std::string& msg); +extern void py_log_debug(const std::string& msg); +extern void py_log_info(const std::string& msg); +extern void py_log_warning(const std::string& msg); +extern void py_log_error(const std::string& msg); #endif diff --git a/src/include/kompute/logger/Logger.hpp b/src/include/kompute/logger/Logger.hpp index 6e06918a..bfdad41a 100644 --- a/src/include/kompute/logger/Logger.hpp +++ b/src/include/kompute/logger/Logger.hpp @@ -25,10 +25,12 @@ static const char* KOMPUTE_LOG_TAG = "KomputeLog"; #else #if KOMPUTE_BUILD_PYTHON #include -#include -namespace py = pybind11; // from python/src/main.cpp -extern py::object kp_trace, kp_debug, kp_info, kp_warning, kp_error; +extern void py_log_trace(const std::string& msg); +extern void py_log_debug(const std::string& msg); +extern void py_log_info(const std::string& msg); +extern void py_log_warning(const std::string& msg); +extern void py_log_error(const std::string& msg); #else #include #endif // KOMPUTE_BUILD_PYTHON @@ -57,7 +59,7 @@ setupLogger(); ANDROID_LOG_VERBOSE, KOMPUTE_LOG_TAG, fmt::format(__VA_ARGS__).c_str())) #else #if KOMPUTE_BUILD_PYTHON -#define KP_LOG_TRACE(...) kp_trace(fmt::format(__VA_ARGS__)) +#define KP_LOG_TRACE(...) py_log_trace(fmt::format(__VA_ARGS__)) #else #define KP_LOG_TRACE(...) \ fmt::print("[{} {}] [trace] [{}:{}] {}\n", \ @@ -81,7 +83,7 @@ setupLogger(); ANDROID_LOG_DEBUG, KOMPUTE_LOG_TAG, fmt::format(__VA_ARGS__).c_str())) #else #if KOMPUTE_BUILD_PYTHON -#define KP_LOG_DEBUG(...) kp_debug(fmt::format(__VA_ARGS__)) +#define KP_LOG_DEBUG(...) py_log_debug(fmt::format(__VA_ARGS__)) #else #ifdef __FILE_NAME__ // gcc 12 provides only file name without path #define KP_LOG_DEBUG(...) \ @@ -98,7 +100,7 @@ setupLogger(); __TIME__, \ __FILE__, \ __LINE__, \ - fmt::format(__VA_ARGS__)) + fmt::format(__VA_ARGS__)); std::cout << std::flush #endif // __FILE__NAME__ #endif // KOMPUTE_BUILD_PYTHON #endif // VK_USE_PLATFORM_ANDROID_KHR @@ -115,7 +117,7 @@ setupLogger(); ANDROID_LOG_INFO, KOMPUTE_LOG_TAG, fmt::format(__VA_ARGS__).c_str())) #else #if KOMPUTE_BUILD_PYTHON -#define KP_LOG_INFO(...) kp_info(fmt::format(__VA_ARGS__)) +#define KP_LOG_INFO(...) py_log_info(fmt::format(__VA_ARGS__)) #else #define KP_LOG_INFO(...) \ fmt::print("[{} {}] [info] [{}:{}] {}\n", \ @@ -139,7 +141,7 @@ setupLogger(); ANDROID_LOG_WARN, KOMPUTE_LOG_TAG, fmt::format(__VA_ARGS__).c_str())) #else #if KOMPUTE_BUILD_PYTHON -#define KP_LOG_WARN(...) kp_warning(fmt::format(__VA_ARGS__)) +#define KP_LOG_WARN(...) py_log_warning(fmt::format(__VA_ARGS__)) #else #define KP_LOG_WARN(...) \ fmt::print("[{} {}] [warn] [{}:{}] {}\n", \ @@ -163,7 +165,7 @@ setupLogger(); ANDROID_LOG_ERROR, KOMPUTE_LOG_TAG, fmt::format(__VA_ARGS__).c_str())) #else #if KOMPUTE_BUILD_PYTHON -#define KP_LOG_ERROR(...) kp_error(fmt::format(__VA_ARGS__)) +#define KP_LOG_ERROR(...) py_log_error(fmt::format(__VA_ARGS__)) #else #define KP_LOG_ERROR(...) \ fmt::print("[{} {}] [error] [{}:{}] {}\n", \ From a82f9d399065d4d2c73660d4b5ad5055674fc676 Mon Sep 17 00:00:00 2001 From: koubaa Date: Wed, 11 Dec 2024 11:01:26 -0600 Subject: [PATCH 2/4] Implement physical device features Signed-off-by: koubaa --- src/Manager.cpp | 73 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 60 insertions(+), 13 deletions(-) diff --git a/src/Manager.cpp b/src/Manager.cpp index 32577db1..d77ab417 100644 --- a/src/Manager.cpp +++ b/src/Manager.cpp @@ -10,6 +10,34 @@ #include #include +template <> struct fmt::formatter: public fmt::formatter { + + // parse is inherited from formatter. + + template + fmt::format_context::iterator format (vk::ValidationFeatureEnableEXT const& flag, Context& ctx) const { + std::string name {"unknown"}; + switch(flag) { + case vk::ValidationFeatureEnableEXT::eDebugPrintf: + name = "debugPrintF"; + break; + case vk::ValidationFeatureEnableEXT::eGpuAssisted: + name = "gpuAssisted"; + break; + case vk::ValidationFeatureEnableEXT::eGpuAssistedReserveBindingSlot: + name = "gpuAssistedReserveBindingSlot"; + break; + case vk::ValidationFeatureEnableEXT::eBestPractices: + name = "bestPractices"; + break; + case vk::ValidationFeatureEnableEXT::eSynchronizationValidation: + name = "synchronizationValidation"; + break; + } + return fmt::formatter::format(name, ctx); + } +}; + namespace kp { #ifndef KOMPUTE_DISABLE_VK_DEBUG_LAYERS @@ -185,10 +213,11 @@ Manager::createInstance() #endif if (!applicationExtensions.empty()) { - computeInstanceCreateInfo.enabledExtensionCount = - (uint32_t)applicationExtensions.size(); - computeInstanceCreateInfo.ppEnabledExtensionNames = - applicationExtensions.data(); + computeInstanceCreateInfo.setEnabledExtensionCount( + (uint32_t)applicationExtensions.size()); + computeInstanceCreateInfo.setPpEnabledExtensionNames( + applicationExtensions.data()); + KP_LOG_DEBUG("Desired extensions: {}", fmt::join(applicationExtensions, ", ")); } #ifndef KOMPUTE_DISABLE_VK_DEBUG_LAYERS @@ -212,7 +241,21 @@ Manager::createInstance() for (const std::string& layerName : envLayerNames) { desiredLayerNames.push_back(layerName.c_str()); } - KP_LOG_DEBUG("Desired layers: {}", fmt::join(desiredLayerNames, ", ")); + } + KP_LOG_DEBUG("Desired layers: {}", fmt::join(desiredLayerNames, ", ")); + + vk::ValidationFeaturesEXT validationFeatures; + std::vector validationFeatureEnablesExt; + const char* envPrintfVal = std::getenv("KOMPUTE_ENV_PRINTF"); + if (envPrintfVal != nullptr && *envPrintfVal != '\0') { + KP_LOG_DEBUG("Kompute Manager adding debug printf"); + validationFeatureEnablesExt.push_back(vk::ValidationFeatureEnableEXT::eDebugPrintf); + } + + if (!validationFeatureEnablesExt.empty()) { + validationFeatures.setEnabledValidationFeatures(validationFeatureEnablesExt); + computeInstanceCreateInfo.setPNext(&validationFeatures); + KP_LOG_DEBUG("Validation features: {}", fmt::join(validationFeatureEnablesExt, ", ")); } // Identify the valid layer names based on the desiredLayerNames @@ -236,9 +279,9 @@ Manager::createInstance() KP_LOG_DEBUG( "Kompute Manager Initializing instance with valid layers: {}", fmt::join(validLayerNames, ", ")); - computeInstanceCreateInfo.enabledLayerCount = - static_cast(validLayerNames.size()); - computeInstanceCreateInfo.ppEnabledLayerNames = validLayerNames.data(); + computeInstanceCreateInfo.setEnabledLayerCount( + static_cast(validLayerNames.size())); + computeInstanceCreateInfo.setPpEnabledLayerNames(validLayerNames.data()); } else { KP_LOG_WARN("Kompute Manager no valid layer names found from desired " "layer names"); @@ -268,15 +311,14 @@ Manager::createInstance() KP_LOG_DEBUG("Kompute Manager Instance Created"); #ifndef KOMPUTE_DISABLE_VK_DEBUG_LAYERS - KP_LOG_DEBUG("Kompute Manager adding debug callbacks"); if (validLayerNames.size() > 0) { + KP_LOG_DEBUG("Kompute Manager adding debug callbacks"); vk::DebugReportFlagsEXT debugFlags = vk::DebugReportFlagBitsEXT::eError | vk::DebugReportFlagBitsEXT::eWarning; - vk::DebugReportCallbackCreateInfoEXT debugCreateInfo = {}; - debugCreateInfo.pfnCallback = - (PFN_vkDebugReportCallbackEXT)debugMessageCallback; - debugCreateInfo.flags = debugFlags; + auto debugCreateInfo = vk::DebugReportCallbackCreateInfoEXT() + .setFlags(debugFlags) + .setPfnCallback((PFN_vkDebugReportCallbackEXT)debugMessageCallback); this->mDebugDispatcher.init(*this->mInstance, &vkGetInstanceProcAddr); this->mDebugReportCallback = @@ -384,6 +426,7 @@ Manager::createDevice(const std::vector& familyQueueIndices, } else { this->mComputeQueueFamilyIndices = familyQueueIndices; } + KP_LOG_DEBUG("compute queue family indices: {}", fmt::join(this->mComputeQueueFamilyIndices, ", ")); std::unordered_map familyQueueCounts; std::unordered_map> familyQueuePriorities; @@ -425,11 +468,15 @@ Manager::createDevice(const std::vector& familyQueueIndices, validExtensions.push_back(ext.c_str()); } } + if (desiredExtensions.size() != validExtensions.size()) { KP_LOG_ERROR("Kompute Manager not all extensions were added: {}", fmt::join(validExtensions, ", ")); } + KP_LOG_DEBUG("Kompute Manager used extensions {}", + fmt::join(validExtensions, ", ")); + vk::DeviceCreateInfo deviceCreateInfo(vk::DeviceCreateFlags(), deviceQueueCreateInfos.size(), deviceQueueCreateInfos.data(), From e11eb7fdb18f1f559cf5d5cd7d72590341bc2630 Mon Sep 17 00:00:00 2001 From: koubaa Date: Wed, 11 Dec 2024 11:09:42 -0600 Subject: [PATCH 3/4] remove unnecessary include of pybind11 in core Signed-off-by: koubaa --- src/include/kompute/Core.hpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/include/kompute/Core.hpp b/src/include/kompute/Core.hpp index d347a827..c444d714 100644 --- a/src/include/kompute/Core.hpp +++ b/src/include/kompute/Core.hpp @@ -23,8 +23,6 @@ typedef std::vector Constants; #endif // KOMPUTE_VK_API_VERSION #if defined(KOMPUTE_BUILD_PYTHON) -#include -namespace py = pybind11; // from python/src/main.cpp extern void py_log_trace(const std::string& msg); From a554555be0506728efadec8c48aadccb16596926 Mon Sep 17 00:00:00 2001 From: koubaa Date: Wed, 11 Dec 2024 11:12:36 -0600 Subject: [PATCH 4/4] remove logger change Signed-off-by: koubaa --- src/include/kompute/logger/Logger.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/kompute/logger/Logger.hpp b/src/include/kompute/logger/Logger.hpp index bfdad41a..fd3832f3 100644 --- a/src/include/kompute/logger/Logger.hpp +++ b/src/include/kompute/logger/Logger.hpp @@ -100,7 +100,7 @@ setupLogger(); __TIME__, \ __FILE__, \ __LINE__, \ - fmt::format(__VA_ARGS__)); std::cout << std::flush + fmt::format(__VA_ARGS__)); #endif // __FILE__NAME__ #endif // KOMPUTE_BUILD_PYTHON #endif // VK_USE_PLATFORM_ANDROID_KHR