remove unnecessary lock and code clean up (#1114)

* refactoring the ep codes.

* remove unnecessary lock.

* fix the comment to claim KernelRegistryManager is not thread safe.

* clarify that APIs to add custom op in inferencesession is not thread safe.

include/onnxruntime/core/framework/execution_provider.h

@@ -104,7 +104,9 @@ class IExecutionProvider {
      For Direct3D operator kernels, this may return an IUnknown supporting
      QueryInterface to ID3D12GraphicsCommandList1.
   */
-  virtual const void* GetExecutionHandle() const noexcept = 0;
+  virtual const void* GetExecutionHandle() const noexcept {
+    return nullptr;
+  }
 
   /**
      @return type of the execution provider; should match that set in the node

onnxruntime/core/framework/kernel_registry_manager.cc

@@ -21,7 +21,6 @@ Status KernelRegistryManager::CreateKernel(const onnxruntime::Node& node,
   }
   Status status;
   {
-    std::lock_guard<OrtMutex> lock(lock_);
     for (auto& registry : custom_kernel_registries_) {
       status = registry->TryCreateKernel(node, execution_provider, session_state.GetInitializedTensors(),
                                          session_state.GetMLValueNameIdxMap(), session_state.GetFuncMgr(), op_kernel);
@@ -32,11 +31,8 @@ Status KernelRegistryManager::CreateKernel(const onnxruntime::Node& node,
   }
 
   KernelRegistry* p = nullptr;
-  {
-    std::lock_guard<OrtMutex> lock(lock_);
-    auto iter = provider_type_to_registry_.find(ptype);
-    if (iter != provider_type_to_registry_.end()) p = iter->second.get();
-  }
+  auto iter = provider_type_to_registry_.find(ptype);
+  if (iter != provider_type_to_registry_.end()) p = iter->second.get();
   if (p != nullptr) {
     status = p->TryCreateKernel(node, execution_provider, session_state.GetInitializedTensors(),
                                 session_state.GetMLValueNameIdxMap(), session_state.GetFuncMgr(), op_kernel);
@@ -53,7 +49,6 @@ Status KernelRegistryManager::CreateKernel(const onnxruntime::Node& node,
 }
 
 Status KernelRegistryManager::RegisterKernels(const ExecutionProviders& execution_providers) {
-  std::lock_guard<OrtMutex> lock(lock_);
   for (auto& provider : execution_providers) {
     auto iter = provider_type_to_registry_.find(provider->Type());
     if (iter != provider_type_to_registry_.end()) {
@@ -76,7 +71,6 @@ void KernelRegistryManager::RegisterKernelRegistry(std::shared_ptr<KernelRegistr
   if (nullptr == kernel_registry) {
     return;
   }
-  std::lock_guard<OrtMutex> lock(lock_);
   custom_kernel_registries_.push_front(kernel_registry);
 }
 
@@ -98,7 +92,6 @@ Status KernelRegistryManager::SearchKernelRegistry(const onnxruntime::Node& node
   }
   Status status;
   {
-    std::lock_guard<OrtMutex> lock(lock_);
     for (auto& registry : custom_kernel_registries_) {
       *kernel_create_info = registry->TryFindKernel(node, "");  // the last argument is ignored
       if (*kernel_create_info != nullptr) {
@@ -108,11 +101,8 @@ Status KernelRegistryManager::SearchKernelRegistry(const onnxruntime::Node& node
   }
 
   KernelRegistry* p = nullptr;
-  {
-    std::lock_guard<OrtMutex> lock(lock_);
-    auto iter = provider_type_to_registry_.find(ptype);
-    if (iter != provider_type_to_registry_.end()) p = iter->second.get();
-  }
+  auto iter = provider_type_to_registry_.find(ptype);
+  if (iter != provider_type_to_registry_.end()) p = iter->second.get();
   if (p != nullptr) {
     *kernel_create_info = p->TryFindKernel(node, "");  // the last argument is ignored
     if (*kernel_create_info != nullptr) {

onnxruntime/core/framework/kernel_registry_manager.h

@@ -25,7 +25,7 @@ class SessionState;
 // 2. common execution provider type specific kernel registries.
 // The 1st and 2nd ones are shared across sessions.
 
-// This class is thread safe
+// This class is not thread safe.
 class KernelRegistryManager {
  public:
   KernelRegistryManager() = default;
@@ -83,6 +83,5 @@ class KernelRegistryManager {
   // Each kernel registry may contain kernels from many different providers.
   // in order to search kernels from a specific provider, we have to iterate all its elements
   std::list<std::shared_ptr<KernelRegistry>> custom_kernel_registries_;
-  mutable OrtMutex lock_;
 };
 }  // namespace onnxruntime

onnxruntime/core/providers/cpu/cpu_execution_provider.h

@@ -50,11 +50,6 @@ class CPUExecutionProvider : public IExecutionProvider {
     return Status(common::ONNXRUNTIME, common::FAIL, "Shouldn't reach here. CPUExecutionProvider doesn't support CopyTensor");
   }
 
-  const void* GetExecutionHandle() const noexcept override {
-    // The CPU interface does not return anything interesting.
-    return nullptr;
-  }
-
   std::shared_ptr<KernelRegistry> GetKernelRegistry() const override;
 
 

onnxruntime/core/providers/cuda/cuda_execution_provider.h

@@ -42,11 +42,6 @@ class CUDAExecutionProvider : public IExecutionProvider {
 
   Status CopyTensor(const Tensor& src, Tensor& dst, int exec_queue_id) const override;
 
-  const void* GetExecutionHandle() const noexcept override {
-    // The CUDA interface does not return anything interesting.
-    return nullptr;
-  }
-
   cublasHandle_t PerThreadCublasHandle() {
     // Assure each thread has its TLS context.
     if (!per_thread_context_)

onnxruntime/core/providers/mkldnn/mkldnn_execution_provider.h

@@ -36,10 +36,6 @@ class MKLDNNExecutionProvider : public IExecutionProvider {
 
   Status CopyTensor(const Tensor& src, Tensor& dst) const override;
 
-  const void* GetExecutionHandle() const noexcept override {
-    return nullptr;
-  }
-
   virtual std::shared_ptr<KernelRegistry> GetKernelRegistry() const override;
 
   std::shared_ptr<mkldnn::memory> GetWeightsMemoryBuffer(const std::string& weight_key) {

onnxruntime/core/providers/ngraph/ngraph_execution_provider.h

@@ -33,8 +33,6 @@ class NGRAPHExecutionProvider : public IExecutionProvider {
   Status Compile(const std::vector<onnxruntime::Node*>& fused_nodes,
                  std::vector<NodeComputeInfo>& node_compute_funcs) override;
 
-  const void* GetExecutionHandle() const noexcept override { return nullptr; }
-
   std::shared_ptr<KernelRegistry> GetKernelRegistry() const override;
 
  private:

onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.h

@@ -64,10 +64,6 @@ class TensorrtExecutionProvider : public IExecutionProvider {
 
   Status CopyTensor(const Tensor& src, Tensor& dst) const override;
 
-  const void* GetExecutionHandle() const noexcept override {
-    return nullptr;
-  }
-
   std::shared_ptr<KernelRegistry> GetKernelRegistry() const override;
 
   void SetMaxBatchSize(const int batch_size) {

onnxruntime/core/session/inference_session.h

@@ -158,6 +158,9 @@ class InferenceSession {
     */
   common::Status AddCustomTransformerList(const std::vector<std::string>& transformers_to_enable);
 
+  /**
+    * Add custom ops. This API is not thread safe.
+    */
   common::Status AddCustomOpDomains(const std::vector<OrtCustomOpDomain*>& ops);
 
   /**
@@ -166,6 +169,7 @@ class InferenceSession {
     * The order of invocation indicates the reversed preference order: Register your most
     * preferred registry at the end.
     * Calling this API is optional.
+	* This API is not thread safe.
     * @return OK if success.
     */
   common::Status RegisterCustomRegistry(std::shared_ptr<CustomRegistry> custom_registry);
@@ -198,6 +202,7 @@ class InferenceSession {
     * Initializes a previously loaded model. Initialization includes but is not
     * limited to graph transformations, construction of kernels, etc.
     * This method assumes that a method has been loaded previously.
+	* This API is thread-safe.
     * @return OK if success
     */
   common::Status Initialize();

onnxruntime/test/framework/dummy_provider.h

@@ -35,10 +35,6 @@ class DummyExecutionProvider : public IExecutionProvider {
     return Status::OK();
   }
 
-  const void* GetExecutionHandle() const noexcept override {
-    return nullptr;
-  }
-
   std::shared_ptr<KernelRegistry> GetKernelRegistry() const override;
 };
 

onnxruntime/test/framework/inference_session_test.cc

@@ -119,9 +119,6 @@ class FuseExecutionProvider : public IExecutionProvider {
     return Status::OK();
   }
 
-  const void* GetExecutionHandle() const noexcept override {
-    return nullptr;
-  }
 };
 
 namespace test {

onnxruntime/test/framework/op_kernel_test.cc

@@ -28,10 +28,6 @@ class XPUExecutionProvider : public IExecutionProvider {
     return Status::OK();
   }
 
-  virtual const void* GetExecutionHandle() const noexcept override {
-    // The XPU interface does not return anything interesting.
-    return nullptr;
-  }
 };
 
 }  // namespace test