From b61173988f5065081d7f695a48ff9aaa422f4f3e Mon Sep 17 00:00:00 2001 From: tvtrimel Date: Wed, 14 Aug 2019 16:08:47 -0700 Subject: [PATCH 1/6] Added check for unnecessary function initializations, and removed lock from unneeded areas of code. --- onnxruntime/core/providers/ngraph/ngraph_custom_op.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc b/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc index cfad088e800da..b551ab1b219b0 100644 --- a/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc +++ b/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc @@ -141,11 +141,10 @@ void NGRAPHCustomOp::Initialize(const OrtCustomOpApi* api, OrtKernelContext* con Status NGRAPHCustomOp::Compute(const OrtCustomOpApi* api, OrtKernelContext* context) const { Ort::CustomOpApi ort{*api}; - //TODO: Minimize locked region - std::lock_guard lock(compute_lock_); - // Initialize nGraph function if it is not already initialized. - Initialize(api, context); + if (ng_curr_exe_ == nullptr) { + Initialize(api, context); + } ORT_ENFORCE(ng_curr_exe_ != nullptr); @@ -186,6 +185,7 @@ Status NGRAPHCustomOp::Compute(const OrtCustomOpApi* api, OrtKernelContext* cont } // Run the graph through nGraph. + std::lock_guard lock(compute_lock_); try { if (!ng_curr_exe_->call(ng_outputs, ng_inputs)) return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, name_ + ": Error while executing nGraph computation"); From 3925eb21863c92b6390044aba10660cc903270d5 Mon Sep 17 00:00:00 2001 From: tvtrimel Date: Thu, 15 Aug 2019 16:34:40 -0700 Subject: [PATCH 2/6] Added LRU cache to EP. --- .../core/providers/ngraph/ngraph_custom_op.cc | 24 ++++++++++++++++++- .../core/providers/ngraph/ngraph_custom_op.h | 2 ++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc b/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc index b551ab1b219b0..80ac6055aa2c0 100644 --- a/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc +++ b/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc @@ -80,7 +80,29 @@ void NGRAPHCustomOp::Initialize(const OrtCustomOpApi* api, OrtKernelContext* con uniq_input_shape.append(reinterpret_cast(tensor_shape.data()), ndim * sizeof(int64_t)); } - auto it = ng_exe_map_.insert({uniq_input_shape, nullptr}); //TODO: Limit the size of map with configurable size. + //auto it = ng_exe_map_.insert({uniq_input_shape, nullptr}); //TODO: Limit the size of map with configurable size. + // not present in cache + if (ng_exe_map_.find(uniq_input_shape) == ng_exe_map_.end()) { + // cache is full + if (keyCache.size() == cacheSize) { + // delete least recently used element + std::string last = keyCache.back(); + + // Pops the last elmeent + keyCache.pop_back(); + + // Erase the last + ng_exe_map_.erase(ng_exe_map_.find(last)); + } + } + + // present in cache + else + keyCache.remove(uniq_input_shape); + + // update reference + keyCache.push_front(uniq_input_shape); + auto it = ng_exe_map_.insert({uniq_input_shape, nullptr}); //ng_exe with current shape already exists if (!it.second) { diff --git a/onnxruntime/core/providers/ngraph/ngraph_custom_op.h b/onnxruntime/core/providers/ngraph/ngraph_custom_op.h index 27659748212de..23b47d282d774 100644 --- a/onnxruntime/core/providers/ngraph/ngraph_custom_op.h +++ b/onnxruntime/core/providers/ngraph/ngraph_custom_op.h @@ -56,6 +56,8 @@ class NGRAPHCustomOp { key = [3,1,2,3,2,4,5] */ mutable std::unordered_map> ng_exe_map_; + mutable std::list keyCache; + const size_t cacheSize = 20; mutable std::mutex compute_lock_; From d45e9feacfce3ac052c2e3f5ecf8440e74308f4a Mon Sep 17 00:00:00 2001 From: tvtrimel Date: Mon, 19 Aug 2019 15:49:39 -0700 Subject: [PATCH 3/6] Bugfixes for nGraph EP Optimization PR --- .../core/providers/ngraph/ngraph_custom_op.cc | 53 ++++++++++++------- .../core/providers/ngraph/ngraph_custom_op.h | 1 - 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc b/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc index 80ac6055aa2c0..33d010317e4d2 100644 --- a/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc +++ b/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc @@ -25,6 +25,8 @@ namespace onnxruntime { namespace ngraph_ep { +#define NGRAPH_EP_LRU_CACHE_DEFAULT_SIZE 200 + static bool check_ngraph_dump_ops() { #ifdef _WIN32 size_t env_name_len = 0; @@ -80,28 +82,35 @@ void NGRAPHCustomOp::Initialize(const OrtCustomOpApi* api, OrtKernelContext* con uniq_input_shape.append(reinterpret_cast(tensor_shape.data()), ndim * sizeof(int64_t)); } - //auto it = ng_exe_map_.insert({uniq_input_shape, nullptr}); //TODO: Limit the size of map with configurable size. - // not present in cache - if (ng_exe_map_.find(uniq_input_shape) == ng_exe_map_.end()) { - // cache is full - if (keyCache.size() == cacheSize) { - // delete least recently used element - std::string last = keyCache.back(); + // Get cache size from environment + std::string tempSize; + if (std::getenv("NGRAPH_EP_LRU_CACHE_SIZE")) { + tempSize = std::getenv("NGRAPH_EP_LRU_CACHE_SIZE"); + } + size_t cacheSize = tempSize.empty() ? NGRAPH_EP_LRU_CACHE_DEFAULT_SIZE : std::stoi(tempSize); + + // Not in cache + if (ng_exe_map_.find(uniq_input_shape) == ng_exe_map_.end()) { + // Check if full + if (keyCache.size() == cacheSize) { + // Delete least recently used element + std::string last = keyCache.back(); - // Pops the last elmeent - keyCache.pop_back(); + // Pop the last elmeent + keyCache.pop_back(); - // Erase the last + // Erase the last element from cache ng_exe_map_.erase(ng_exe_map_.find(last)); } } - // present in cache - else - keyCache.remove(uniq_input_shape); + // Found in cache + else { + keyCache.remove(uniq_input_shape); + } - // update reference - keyCache.push_front(uniq_input_shape); + // update reference + keyCache.push_front(uniq_input_shape); auto it = ng_exe_map_.insert({uniq_input_shape, nullptr}); //ng_exe with current shape already exists @@ -164,9 +173,9 @@ Status NGRAPHCustomOp::Compute(const OrtCustomOpApi* api, OrtKernelContext* cont Ort::CustomOpApi ort{*api}; // Initialize nGraph function if it is not already initialized. - if (ng_curr_exe_ == nullptr) { - Initialize(api, context); - } + compute_lock_.lock(); + Initialize(api, context); + compute_lock_.unlock(); ORT_ENFORCE(ng_curr_exe_ != nullptr); @@ -179,11 +188,15 @@ Status NGRAPHCustomOp::Compute(const OrtCustomOpApi* api, OrtKernelContext* cont for (const auto& ng_param : ng_curr_exe_->get_parameters()) { const OrtValue* input_tensor = ort.KernelContext_GetInput(context, input_index++); void* input_data = const_cast(ort.GetTensorData(input_tensor)); + compute_lock_.lock(); ng_inputs.emplace_back(ng_backend_->create_tensor(ng_param->get_output_element_type(0), ng_param->get_output_shape(0), input_data)); + compute_lock_.unlock(); } } catch (const std::exception& exp) { + compute_lock_.unlock(); return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, name_ + ": Exception while copying input data to nGraph: " + std::string(exp.what())); } catch (...) { + compute_lock_.unlock(); return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, name_ + ": Unknown exception while copying input data to nGraph"); } @@ -198,11 +211,15 @@ Status NGRAPHCustomOp::Compute(const OrtCustomOpApi* api, OrtKernelContext* cont std::vector ort_shape{shape.begin(), shape.end()}; OrtValue* output_tensor = ort.KernelContext_GetOutput(context, output_index++, ort_shape.data(), ort_shape.size()); void* output_data = ort.GetTensorMutableData(output_tensor); + compute_lock_.lock(); ng_outputs.emplace_back(ng_backend_->create_tensor(dtype, shape, output_data)); + compute_lock_.unlock(); } } catch (const std::exception& exp) { + compute_lock_.unlock(); return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, name_ + ": Exception while creating nGraph output Tensor: " + std::string(exp.what())); } catch (...) { + compute_lock_.unlock(); return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, name_ + ": Unknown exception while creating nGraph output Tensor"); } diff --git a/onnxruntime/core/providers/ngraph/ngraph_custom_op.h b/onnxruntime/core/providers/ngraph/ngraph_custom_op.h index 23b47d282d774..ef405c5059774 100644 --- a/onnxruntime/core/providers/ngraph/ngraph_custom_op.h +++ b/onnxruntime/core/providers/ngraph/ngraph_custom_op.h @@ -57,7 +57,6 @@ class NGRAPHCustomOp { */ mutable std::unordered_map> ng_exe_map_; mutable std::list keyCache; - const size_t cacheSize = 20; mutable std::mutex compute_lock_; From a58c8c6dcecfe8b5bb4ed886bf089dddb0422a5c Mon Sep 17 00:00:00 2001 From: tvtrimel Date: Tue, 20 Aug 2019 15:23:17 -0700 Subject: [PATCH 4/6] Changed default cache size to 500 and refactored mutex readability. --- .../core/providers/ngraph/ngraph_custom_op.cc | 26 +++++++++---------- .../core/providers/ngraph/ngraph_custom_op.h | 2 +- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc b/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc index 33d010317e4d2..000bb03e74cad 100644 --- a/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc +++ b/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc @@ -25,7 +25,7 @@ namespace onnxruntime { namespace ngraph_ep { -#define NGRAPH_EP_LRU_CACHE_DEFAULT_SIZE 200 +#define NGRAPH_EP_LRU_CACHE_DEFAULT_SIZE 500 static bool check_ngraph_dump_ops() { #ifdef _WIN32 @@ -84,8 +84,8 @@ void NGRAPHCustomOp::Initialize(const OrtCustomOpApi* api, OrtKernelContext* con // Get cache size from environment std::string tempSize; - if (std::getenv("NGRAPH_EP_LRU_CACHE_SIZE")) { - tempSize = std::getenv("NGRAPH_EP_LRU_CACHE_SIZE"); + if (std::getenv("ONNXRUNTIME_NGRAPH_LRU_CACHE_SIZE")) { + tempSize = std::getenv("ONNXRUNTIME_NGRAPH_LRU_CACHE_SIZE"); } size_t cacheSize = tempSize.empty() ? NGRAPH_EP_LRU_CACHE_DEFAULT_SIZE : std::stoi(tempSize); @@ -173,9 +173,11 @@ Status NGRAPHCustomOp::Compute(const OrtCustomOpApi* api, OrtKernelContext* cont Ort::CustomOpApi ort{*api}; // Initialize nGraph function if it is not already initialized. - compute_lock_.lock(); - Initialize(api, context); - compute_lock_.unlock(); + { + std::mutex init_lock_; + std::lock_guard lock(init_lock_); + Initialize(api, context); + } ORT_ENFORCE(ng_curr_exe_ != nullptr); @@ -188,15 +190,13 @@ Status NGRAPHCustomOp::Compute(const OrtCustomOpApi* api, OrtKernelContext* cont for (const auto& ng_param : ng_curr_exe_->get_parameters()) { const OrtValue* input_tensor = ort.KernelContext_GetInput(context, input_index++); void* input_data = const_cast(ort.GetTensorData(input_tensor)); - compute_lock_.lock(); + std::mutex input_lock_; + std::lock_guard lock(input_lock_); ng_inputs.emplace_back(ng_backend_->create_tensor(ng_param->get_output_element_type(0), ng_param->get_output_shape(0), input_data)); - compute_lock_.unlock(); } } catch (const std::exception& exp) { - compute_lock_.unlock(); return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, name_ + ": Exception while copying input data to nGraph: " + std::string(exp.what())); } catch (...) { - compute_lock_.unlock(); return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, name_ + ": Unknown exception while copying input data to nGraph"); } @@ -211,15 +211,13 @@ Status NGRAPHCustomOp::Compute(const OrtCustomOpApi* api, OrtKernelContext* cont std::vector ort_shape{shape.begin(), shape.end()}; OrtValue* output_tensor = ort.KernelContext_GetOutput(context, output_index++, ort_shape.data(), ort_shape.size()); void* output_data = ort.GetTensorMutableData(output_tensor); - compute_lock_.lock(); + std::mutex output_lock_; + std::lock_guard lock(output_lock_); ng_outputs.emplace_back(ng_backend_->create_tensor(dtype, shape, output_data)); - compute_lock_.unlock(); } } catch (const std::exception& exp) { - compute_lock_.unlock(); return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, name_ + ": Exception while creating nGraph output Tensor: " + std::string(exp.what())); } catch (...) { - compute_lock_.unlock(); return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, name_ + ": Unknown exception while creating nGraph output Tensor"); } diff --git a/onnxruntime/core/providers/ngraph/ngraph_custom_op.h b/onnxruntime/core/providers/ngraph/ngraph_custom_op.h index ef405c5059774..ad9955872d7fb 100644 --- a/onnxruntime/core/providers/ngraph/ngraph_custom_op.h +++ b/onnxruntime/core/providers/ngraph/ngraph_custom_op.h @@ -57,7 +57,7 @@ class NGRAPHCustomOp { */ mutable std::unordered_map> ng_exe_map_; mutable std::list keyCache; - + mutable std::mutex compute_lock_; mutable ONNX_NAMESPACE::ModelProto model_proto_; From ef07cb354f95b52ea87626cd87970b4abc8ad897 Mon Sep 17 00:00:00 2001 From: tvtrimel Date: Tue, 20 Aug 2019 17:53:09 -0700 Subject: [PATCH 5/6] Fixed unsafe environmental variable fetch for Windows. --- onnxruntime/core/providers/ngraph/ngraph_custom_op.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc b/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc index 000bb03e74cad..58b766f69bd26 100644 --- a/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc +++ b/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc @@ -84,9 +84,19 @@ void NGRAPHCustomOp::Initialize(const OrtCustomOpApi* api, OrtKernelContext* con // Get cache size from environment std::string tempSize; + #ifdef _WIN32 + char *buf{nullptr}; + size_t bufSize; + _dupenv_s(&buf, &bufSize, "ONNXRUNTIME_NGRAPH_LRU_CACHE_SIZE"); + if(buf) { + tempSize = buf; + } + free(buf); + #else if (std::getenv("ONNXRUNTIME_NGRAPH_LRU_CACHE_SIZE")) { tempSize = std::getenv("ONNXRUNTIME_NGRAPH_LRU_CACHE_SIZE"); } + #endif size_t cacheSize = tempSize.empty() ? NGRAPH_EP_LRU_CACHE_DEFAULT_SIZE : std::stoi(tempSize); // Not in cache From ad64c09642960c0df95c02a42b580da0e7c10a3f Mon Sep 17 00:00:00 2001 From: tvtrimel Date: Wed, 21 Aug 2019 09:50:35 -0700 Subject: [PATCH 6/6] Cleaned up Windows environment functions and cleaned up mutexes. --- .../core/providers/ngraph/ngraph_custom_op.cc | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc b/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc index 58b766f69bd26..8c301694da96e 100644 --- a/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc +++ b/onnxruntime/core/providers/ngraph/ngraph_custom_op.cc @@ -86,12 +86,11 @@ void NGRAPHCustomOp::Initialize(const OrtCustomOpApi* api, OrtKernelContext* con std::string tempSize; #ifdef _WIN32 char *buf{nullptr}; - size_t bufSize; - _dupenv_s(&buf, &bufSize, "ONNXRUNTIME_NGRAPH_LRU_CACHE_SIZE"); - if(buf) { + size_t bufSize = 0; + if (!_dupenv_s(&buf, &bufSize, "ONNXRUNTIME_NGRAPH_LRU_CACHE_SIZE") && buf) { tempSize = buf; + free(buf); } - free(buf); #else if (std::getenv("ONNXRUNTIME_NGRAPH_LRU_CACHE_SIZE")) { tempSize = std::getenv("ONNXRUNTIME_NGRAPH_LRU_CACHE_SIZE"); @@ -184,8 +183,7 @@ Status NGRAPHCustomOp::Compute(const OrtCustomOpApi* api, OrtKernelContext* cont // Initialize nGraph function if it is not already initialized. { - std::mutex init_lock_; - std::lock_guard lock(init_lock_); + std::lock_guard lock(compute_lock_); Initialize(api, context); } @@ -200,8 +198,7 @@ Status NGRAPHCustomOp::Compute(const OrtCustomOpApi* api, OrtKernelContext* cont for (const auto& ng_param : ng_curr_exe_->get_parameters()) { const OrtValue* input_tensor = ort.KernelContext_GetInput(context, input_index++); void* input_data = const_cast(ort.GetTensorData(input_tensor)); - std::mutex input_lock_; - std::lock_guard lock(input_lock_); + std::lock_guard lock(compute_lock_); ng_inputs.emplace_back(ng_backend_->create_tensor(ng_param->get_output_element_type(0), ng_param->get_output_shape(0), input_data)); } } catch (const std::exception& exp) { @@ -221,8 +218,7 @@ Status NGRAPHCustomOp::Compute(const OrtCustomOpApi* api, OrtKernelContext* cont std::vector ort_shape{shape.begin(), shape.end()}; OrtValue* output_tensor = ort.KernelContext_GetOutput(context, output_index++, ort_shape.data(), ort_shape.size()); void* output_data = ort.GetTensorMutableData(output_tensor); - std::mutex output_lock_; - std::lock_guard lock(output_lock_); + std::lock_guard lock(compute_lock_); ng_outputs.emplace_back(ng_backend_->create_tensor(dtype, shape, output_data)); } } catch (const std::exception& exp) { @@ -232,8 +228,8 @@ Status NGRAPHCustomOp::Compute(const OrtCustomOpApi* api, OrtKernelContext* cont } // Run the graph through nGraph. - std::lock_guard lock(compute_lock_); try { + std::lock_guard lock(compute_lock_); if (!ng_curr_exe_->call(ng_outputs, ng_inputs)) return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL, name_ + ": Error while executing nGraph computation"); } catch (const std::exception& exp) {