From fc3d23d3f93a1f6fecc2efe5b00dd1928f3127dc Mon Sep 17 00:00:00 2001 From: Alejandro Saucedo Date: Mon, 8 Feb 2021 19:15:48 +0000 Subject: [PATCH] Removed OpCreateTensor in favour of manager memory ownership --- src/Manager.cpp | 11 +++ src/OpAlgoBase.cpp | 2 +- src/OpTensorCopy.cpp | 2 +- src/OpTensorCreate.cpp | 76 ------------------- src/OpTensorSyncDevice.cpp | 2 +- src/OpTensorSyncLocal.cpp | 2 +- src/Tensor.cpp | 10 ++- src/include/kompute/Manager.hpp | 45 +++++++++-- src/include/kompute/operations/OpBase.hpp | 8 +- .../kompute/operations/OpTensorCreate.hpp | 74 ------------------ 10 files changed, 64 insertions(+), 168 deletions(-) delete mode 100644 src/OpTensorCreate.cpp delete mode 100644 src/include/kompute/operations/OpTensorCreate.hpp diff --git a/src/Manager.cpp b/src/Manager.cpp index df9d64db6..11d11a26d 100755 --- a/src/Manager.cpp +++ b/src/Manager.cpp @@ -58,6 +58,17 @@ Manager::~Manager() return; } + if (this->mManagedTensors.size()) { + SPDLOG_DEBUG("Kompute Manager explicitly freeing tensors"); + for (const std::shared_ptr& tensor : this->mManagedTensors) { + if (!tensor->isInit()) { + SPDLOG_ERROR("Kompute Manager attempted to free managed tensor but not tensor is not initialised"); + } + tensor->freeMemoryDestroyGPUResources(); + } + this->mManagedTensors.clear(); + } + if (this->mManagedSequences.size()) { SPDLOG_DEBUG("Kompute Manager explicitly running destructor for " "managed sequences"); diff --git a/src/OpAlgoBase.cpp b/src/OpAlgoBase.cpp index c6ecf316d..ad4bbc17b 100644 --- a/src/OpAlgoBase.cpp +++ b/src/OpAlgoBase.cpp @@ -14,7 +14,7 @@ OpAlgoBase::OpAlgoBase(std::shared_ptr physicalDevice, std::shared_ptr commandBuffer, std::vector>& tensors, KomputeWorkgroup komputeWorkgroup) - : OpBase(physicalDevice, device, commandBuffer, tensors, false) + : OpBase(physicalDevice, device, commandBuffer, tensors) { SPDLOG_DEBUG("Kompute OpAlgoBase constructor with params numTensors: {}", tensors.size()); diff --git a/src/OpTensorCopy.cpp b/src/OpTensorCopy.cpp index 3df23aa57..3726c71ec 100644 --- a/src/OpTensorCopy.cpp +++ b/src/OpTensorCopy.cpp @@ -12,7 +12,7 @@ OpTensorCopy::OpTensorCopy(std::shared_ptr physicalDevice, std::shared_ptr device, std::shared_ptr commandBuffer, std::vector> tensors) - : OpBase(physicalDevice, device, commandBuffer, tensors, false) + : OpBase(physicalDevice, device, commandBuffer, tensors) { SPDLOG_DEBUG("Kompute OpTensorCopy constructor with params"); } diff --git a/src/OpTensorCreate.cpp b/src/OpTensorCreate.cpp deleted file mode 100644 index 7918415e9..000000000 --- a/src/OpTensorCreate.cpp +++ /dev/null @@ -1,76 +0,0 @@ - -#include "kompute/Tensor.hpp" - -#include "kompute/operations/OpTensorCreate.hpp" - -namespace kp { - -OpTensorCreate::OpTensorCreate() -{ - SPDLOG_DEBUG("Kompute OpTensorCreate constructor base"); -} - -OpTensorCreate::OpTensorCreate( - std::shared_ptr physicalDevice, - std::shared_ptr device, - std::shared_ptr commandBuffer, - std::vector> tensors) - : OpBase(physicalDevice, device, commandBuffer, tensors, true) -{ - SPDLOG_DEBUG("Kompute OpTensorCreate constructor with params"); -} - -OpTensorCreate::~OpTensorCreate() -{ - SPDLOG_DEBUG("Kompute OpTensorCreate destructor started"); -} - -void -OpTensorCreate::init() -{ - SPDLOG_DEBUG("Kompute OpTensorCreate init called"); - - if (this->mTensors.size() < 1) { - throw std::runtime_error( - "Kompute OpTensorCreate called with less than 1 tensor"); - } - - for (std::shared_ptr tensor : this->mTensors) { - if (tensor->isInit()) { - throw std::runtime_error( - "Kompute OpTensorCreate: Tensor has already been initialized"); - } - if (tensor->tensorType() != Tensor::TensorTypes::eStorage) { - tensor->init(this->mPhysicalDevice, this->mDevice); - - tensor->mapDataIntoHostMemory(); - } - } -} - -void -OpTensorCreate::record() -{ - SPDLOG_DEBUG("Kompute OpTensorCreate record called"); - - for (size_t i = 0; i < this->mTensors.size(); i++) { - if (this->mTensors[i]->tensorType() == Tensor::TensorTypes::eDevice) { - this->mTensors[i]->recordCopyFromStagingToDevice( - this->mCommandBuffer, false); - } - } -} - -void -OpTensorCreate::preEval() -{ - SPDLOG_DEBUG("Kompute OpTensorCreate preEval called"); -} - -void -OpTensorCreate::postEval() -{ - SPDLOG_DEBUG("Kompute OpTensorCreate postEval called"); -} - -} diff --git a/src/OpTensorSyncDevice.cpp b/src/OpTensorSyncDevice.cpp index 340786eb5..92bd7512f 100644 --- a/src/OpTensorSyncDevice.cpp +++ b/src/OpTensorSyncDevice.cpp @@ -15,7 +15,7 @@ OpTensorSyncDevice::OpTensorSyncDevice( std::shared_ptr device, std::shared_ptr commandBuffer, std::vector> tensors) - : OpBase(physicalDevice, device, commandBuffer, tensors, false) + : OpBase(physicalDevice, device, commandBuffer, tensors) { SPDLOG_DEBUG("Kompute OpTensorSyncDevice constructor with params"); } diff --git a/src/OpTensorSyncLocal.cpp b/src/OpTensorSyncLocal.cpp index 09d966e12..c7a4fb638 100644 --- a/src/OpTensorSyncLocal.cpp +++ b/src/OpTensorSyncLocal.cpp @@ -15,7 +15,7 @@ OpTensorSyncLocal::OpTensorSyncLocal( std::shared_ptr device, std::shared_ptr commandBuffer, std::vector> tensors) - : OpBase(physicalDevice, device, commandBuffer, tensors, false) + : OpBase(physicalDevice, device, commandBuffer, tensors) { SPDLOG_DEBUG("Kompute OpTensorSyncLocal constructor with params"); } diff --git a/src/Tensor.cpp b/src/Tensor.cpp index f04165cf9..7400dfff4 100644 --- a/src/Tensor.cpp +++ b/src/Tensor.cpp @@ -229,8 +229,11 @@ Tensor::mapDataFromHostMemory() if (this->mTensorType == TensorTypes::eHost) { hostVisibleMemory = this->mPrimaryMemory; - } else { + } else if (this->mTensorType == TensorTypes::eDevice) { hostVisibleMemory = this->mStagingMemory; + } else { + SPDLOG_WARN("Kompute Tensor mapping data not supported on storage tensor"); + return; } vk::DeviceSize bufferSize = this->memorySize(); @@ -252,8 +255,11 @@ Tensor::mapDataIntoHostMemory() if (this->mTensorType == TensorTypes::eHost) { hostVisibleMemory = this->mPrimaryMemory; - } else { + } else if (this->mTensorType == TensorTypes::eDevice) { hostVisibleMemory = this->mStagingMemory; + } else { + SPDLOG_WARN("Kompute Tensor mapping data not supported on storage tensor"); + return; } vk::DeviceSize bufferSize = this->memorySize(); diff --git a/src/include/kompute/Manager.hpp b/src/include/kompute/Manager.hpp index 8c689ba57..973a00397 100644 --- a/src/include/kompute/Manager.hpp +++ b/src/include/kompute/Manager.hpp @@ -1,13 +1,12 @@ #pragma once #include +#include #include "kompute/Core.hpp" #include "kompute/Sequence.hpp" -#include "kompute/operations/OpTensorCreate.hpp" - #define KP_DEFAULT_SESSION "DEFAULT" namespace kp { @@ -231,8 +230,8 @@ class Manager /** * Function that simplifies the common workflow of tensor creation and * initialization. It will take the constructor parameters for a Tensor - * and will will us it to create a new Tensor and then create it using - * the OpCreateTensor command. + * and will will us it to create a new Tensor and then create it. The + * tensor memory will then be managed and owned by the manager. * * @param data The data to initialize the tensor with * @param tensorType The type of tensor to initialize @@ -242,17 +241,49 @@ class Manager const std::vector& data, Tensor::TensorTypes tensorType = Tensor::TensorTypes::eDevice) { - SPDLOG_DEBUG("Kompute Manager createInitTensor triggered"); + SPDLOG_DEBUG("Kompute Manager buildTensor triggered"); SPDLOG_DEBUG("Kompute Manager creating new tensor shared ptr"); std::shared_ptr tensor = std::make_shared(kp::Tensor(data, tensorType)); - this->evalOpDefault({ tensor }); + tensor->init(this->mPhysicalDevice, this->mDevice); + if (tensor->tensorType() != Tensor::TensorTypes::eStorage) { + tensor->mapDataIntoHostMemory(); + } + this->mManagedTensors.insert(tensor); return tensor; } + /** + * Function that simplifies the common workflow of tensor initialisation. It will take the constructor parameters for a Tensor and will will us it to create a new Tensor. The tensor memory will then be managed and owned by the manager. + * + * @param data The data to initialize the tensor with + * @param tensorType The type of tensor to initialize + * @returns Initialized Tensor with memory Syncd to GPU device + */ + void rebuildTensors(std::vector> tensors) + { + SPDLOG_DEBUG("Kompute Manager rebuildTensors triggered"); + for (std::shared_ptr tensor : tensors) { + + if (tensor->isInit()) { + tensor->freeMemoryDestroyGPUResources(); + } + + tensor->init(this->mPhysicalDevice, this->mDevice); + if (tensor->tensorType() != Tensor::TensorTypes::eStorage) { + tensor->mapDataIntoHostMemory(); + } + + std::set>::iterator it = this->mManagedTensors.find(tensor); + if (it == this->mManagedTensors.end()) { + this->mManagedTensors.insert(tensor); + } + } + } + private: // -------------- OPTIONALLY OWNED RESOURCES std::shared_ptr mInstance = nullptr; @@ -263,6 +294,8 @@ class Manager bool mFreeDevice = false; // -------------- ALWAYS OWNED RESOURCES + std::set> mManagedTensors; + std::unordered_map> mManagedSequences; diff --git a/src/include/kompute/operations/OpBase.hpp b/src/include/kompute/operations/OpBase.hpp index 6e35df994..a423abc20 100644 --- a/src/include/kompute/operations/OpBase.hpp +++ b/src/include/kompute/operations/OpBase.hpp @@ -31,13 +31,11 @@ class OpBase * @param device Vulkan logical device for passing to Algorithm * @param commandBuffer Vulkan Command Buffer to record commands into * @param tensors Tensors that are to be used in this operation - * @param freeTensors Whether operation manages the memory of the Tensors */ OpBase(std::shared_ptr physicalDevice, std::shared_ptr device, std::shared_ptr commandBuffer, - std::vector>& tensors, - bool freeTensors) + std::vector>& tensors) { SPDLOG_DEBUG("Compute OpBase constructor with params"); @@ -45,14 +43,12 @@ class OpBase this->mDevice = device; this->mCommandBuffer = commandBuffer; this->mTensors = tensors; - this->mFreeTensors = freeTensors; } /** * Default destructor for OpBase class. This OpBase destructor class should * always be called to destroy and free owned resources unless it is - * intended to destroy the resources in the parent class. This can be done - * by passing the mFreeTensors=false. + * intended to destroy the resources in the parent class. */ virtual ~OpBase() { diff --git a/src/include/kompute/operations/OpTensorCreate.hpp b/src/include/kompute/operations/OpTensorCreate.hpp deleted file mode 100644 index 4b8c784cc..000000000 --- a/src/include/kompute/operations/OpTensorCreate.hpp +++ /dev/null @@ -1,74 +0,0 @@ -#pragma once - -#include "kompute/Core.hpp" - -#include "kompute/Tensor.hpp" - -#include "kompute/operations/OpBase.hpp" - -namespace kp { - -/** - Operation that creates tensor and manages the memory of the components - created -*/ -class OpTensorCreate : public OpBase -{ - public: - OpTensorCreate(); - - /** - * Default constructor with parameters that provides the bare minimum - * requirements for the operations to be able to create and manage their - * sub-components. - * - * @param physicalDevice Vulkan physical device used to find device queues - * @param device Vulkan logical device for passing to Algorithm - * @param commandBuffer Vulkan Command Buffer to record commands into - * @param tensors Tensors that will be used to create in operation. - * @param freeTensors Whether operation manages the memory of the Tensors - */ - OpTensorCreate(std::shared_ptr physicalDevice, - std::shared_ptr device, - std::shared_ptr commandBuffer, - std::vector> tensors); - - /** - * Default destructor which in this case expects the parent class to free - * the tensors - */ - ~OpTensorCreate() override; - - /** - * In charge of initialising the primary Tensor as well as the staging - * tensor as required. It will only initialise a staging tensor if the - * Primary tensor is of type Device. For staging tensors it performs a - * mapDataIntoHostMemory which would perform immediately as opposed to - * on sequence eval/submission. - */ - void init() override; - - /** - * Record runs the core actions to create the tensors. For device tensors - * it records a copyCommand to move the data from the staging tensor to the - * device tensor. The mapping for staging tensors happens in the init function - * not in the record function. - */ - void record() override; - - /** - * Does not perform any preEval commands. - */ - virtual void preEval() override; - - /** - * Performs a copy back into the main tensor to ensure that the data - * contained is the one that is now being stored in the GPU. - */ - virtual void postEval() override; - - - private: -}; - -} // End namespace kp