From 30b9f5dde14aefb6c5761bebe0cb2884e10fe913 Mon Sep 17 00:00:00 2001 From: Adrian Lizarraga Date: Tue, 17 Jan 2023 15:55:56 -0800 Subject: [PATCH] Clean up TensorRT deprecations, warnings, unbounded string copy (#14148) ### Description - Updates deprecated use of `nvinfer1::___::destroy()` by using a `std::unique_ptr<>` instead of our own smart pointer that calls `destroy`. See [TensorRT deprecation list](https://docs.nvidia.com/deeplearning/tensorrt/api/c_api/deprecated.html#:~:text=Deprecated%20prior%20to%20TensorRT%208.0%20and%20will%20be,noexcept%20Use%20addMatrixMultiply%20instead.%20Deprecated%20in%20TensorRT%208.4.) and search for `destroy`. - Fixes warnings regarding uninitialized member variables. - Fixes bugs in TensorRT model ID generation: - Potential segfault when model path only has a root component. - Unbounded string copy for non-Windows builds. ### Motivation and Context Clean up --- .../provider_bridge_provider.cc | 5 ++ .../shared_library/provider_interfaces.h | 4 ++ .../tensorrt/tensorrt_execution_provider.cc | 54 +++++++++---------- .../tensorrt/tensorrt_execution_provider.h | 44 +++++++-------- .../tensorrt_execution_provider_utils.h | 30 ++++------- .../core/session/provider_bridge_ort.cc | 4 ++ 6 files changed, 71 insertions(+), 70 deletions(-) diff --git a/onnxruntime/core/providers/shared_library/provider_bridge_provider.cc b/onnxruntime/core/providers/shared_library/provider_bridge_provider.cc index 9cfa7cf12b..b772fe95d6 100644 --- a/onnxruntime/core/providers/shared_library/provider_bridge_provider.cc +++ b/onnxruntime/core/providers/shared_library/provider_bridge_provider.cc @@ -702,4 +702,9 @@ void MurmurHash3::x86_128(const void* key, int len, uint32_t seed, void* out) { return g_host->MurmurHash3__x86_128(key, len, seed, out); } +#ifdef _WIN32 +std::string ToUTF8String(const std::wstring& s) { + return g_host->ToUTF8String(s); +} +#endif } // namespace onnxruntime diff --git a/onnxruntime/core/providers/shared_library/provider_interfaces.h b/onnxruntime/core/providers/shared_library/provider_interfaces.h index 2880d6b70e..1fbe938c2a 100644 --- a/onnxruntime/core/providers/shared_library/provider_interfaces.h +++ b/onnxruntime/core/providers/shared_library/provider_interfaces.h @@ -885,6 +885,10 @@ struct ProviderHost { virtual void MurmurHash3__x86_128(const void* key, int len, uint32_t seed, void* out) = 0; +#ifdef _WIN32 + virtual std::string ToUTF8String(const std::wstring& s) = 0; +#endif + virtual ProviderHostCPU& GetProviderHostCPU() = 0; }; diff --git a/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.cc b/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.cc index 5c3eb51a03..ca815fd788 100644 --- a/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.cc +++ b/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.cc @@ -278,7 +278,7 @@ std::unique_lock TensorrtExecutionProvider::GetApiLock() const { #ifdef ORT_TENSORRT_PLACEHOLDER_BUILDER // instantiate global unused builder object which keeps the TRT kernel library in memory // so that subsequent builders avoid the expensive load / unload process. -auto const placeholder = tensorrt_ptr::unique_pointer(nvinfer1::createInferBuilder(GetTensorrtLogger())); +auto const placeholder = std::unique_ptr(nvinfer1::createInferBuilder(GetTensorrtLogger())); #endif TensorrtExecutionProvider::TensorrtExecutionProvider(const TensorrtExecutionProviderInfo& info) @@ -446,7 +446,7 @@ TensorrtExecutionProvider::TensorrtExecutionProvider(const TensorrtExecutionProv } { auto lock = GetApiLock(); - runtime_ = tensorrt_ptr::unique_pointer(nvinfer1::createInferRuntime(GetTensorrtLogger())); + runtime_ = std::unique_ptr(nvinfer1::createInferRuntime(GetTensorrtLogger())); } } @@ -921,9 +921,9 @@ SubGraphCollection_t TensorrtExecutionProvider::GetSupportedList(SubGraphCollect // Get supported node list recursively SubGraphCollection_t parser_nodes_list; TensorrtLogger& trt_logger = GetTensorrtLogger(); - auto trt_builder = tensorrt_ptr::unique_pointer(nvinfer1::createInferBuilder(trt_logger)); + auto trt_builder = std::unique_ptr(nvinfer1::createInferBuilder(trt_logger)); const auto explicitBatch = 1U << static_cast(nvinfer1::NetworkDefinitionCreationFlag::kEXPLICIT_BATCH); - auto trt_network = tensorrt_ptr::unique_pointer(trt_builder->createNetworkV2(explicitBatch)); + auto trt_network = std::unique_ptr(trt_builder->createNetworkV2(explicitBatch)); auto trt_parser = tensorrt_ptr::unique_pointer(nvonnxparser::createParser(*trt_network, trt_logger)); trt_parser->supportsModel(string_buf.data(), string_buf.size(), parser_nodes_list, model_path_); @@ -1253,10 +1253,10 @@ common::Status TensorrtExecutionProvider::Compile(const std::vector(nvinfer1::createInferBuilder(trt_logger)); + auto trt_builder = std::unique_ptr(nvinfer1::createInferBuilder(trt_logger)); const auto explicitBatch = 1U << static_cast(nvinfer1::NetworkDefinitionCreationFlag::kEXPLICIT_BATCH); - auto trt_network = tensorrt_ptr::unique_pointer(trt_builder->createNetworkV2(explicitBatch)); - auto trt_config = tensorrt_ptr::unique_pointer(trt_builder->createBuilderConfig()); + auto trt_network = std::unique_ptr(trt_builder->createNetworkV2(explicitBatch)); + auto trt_config = std::unique_ptr(trt_builder->createBuilderConfig()); auto trt_parser = tensorrt_ptr::unique_pointer(nvonnxparser::createParser(*trt_network, trt_logger)); trt_parser->parse(string_buf.data(), string_buf.size(), model_path_); trt_config->setMaxWorkspaceSize(max_workspace_size_); @@ -1368,8 +1368,8 @@ common::Status TensorrtExecutionProvider::Compile(const std::vector trt_engine; - tensorrt_ptr::unique_pointer trt_context; + std::unique_ptr trt_engine; + std::unique_ptr trt_context; if (!has_dynamic_shape) { const std::string cache_path = GetCachePath(cache_path_, trt_node_name_with_precision); const std::string engine_cache_path = cache_path + ".engine"; @@ -1384,7 +1384,7 @@ common::Status TensorrtExecutionProvider::Compile(const std::vector engine_buf{new char[engine_size]}; engine_file.read((char*)engine_buf.get(), engine_size); - trt_engine = tensorrt_ptr::unique_pointer(runtime_->deserializeCudaEngine(engine_buf.get(), engine_size, nullptr)); + trt_engine = std::unique_ptr(runtime_->deserializeCudaEngine(engine_buf.get(), engine_size, nullptr)); LOGS_DEFAULT(VERBOSE) << "[TensorRT EP] DeSerialized " + engine_cache_path; if (trt_engine == nullptr) { return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL, @@ -1403,7 +1403,7 @@ common::Status TensorrtExecutionProvider::Compile(const std::vector(runtime_->deserializeCudaEngine(engine_buf.get(), engine_size, nullptr)); + trt_engine = std::unique_ptr(runtime_->deserializeCudaEngine(engine_buf.get(), engine_size, nullptr)); LOGS_DEFAULT(VERBOSE) << "[TensorRT EP] DeSerialized " + engine_cache_path; if (trt_engine == nullptr) { return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL, @@ -1420,13 +1420,13 @@ common::Status TensorrtExecutionProvider::Compile(const std::vector(trt_builder->buildEngineWithConfig(*trt_network, *trt_config)); + trt_engine = std::unique_ptr(trt_builder->buildEngineWithConfig(*trt_network, *trt_config)); if (trt_engine == nullptr) { return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL, "TensorRT EP could not build engine for fused node: " + fused_node.Name()); } if (engine_cache_enable_) { - nvinfer1::IHostMemory* serializedModel = trt_engine->serialize(); + std::unique_ptr serializedModel(trt_engine->serialize()); size_t engine_size = serializedModel->size(); if (engine_decryption_enable_) { // Encrypt engine @@ -1438,7 +1438,6 @@ common::Status TensorrtExecutionProvider::Compile(const std::vector(serializedModel->data()), engine_size); } - serializedModel->destroy(); LOGS_DEFAULT(VERBOSE) << "[TensorRT EP] Serialized " + engine_cache_path; } } @@ -1451,9 +1450,9 @@ common::Status TensorrtExecutionProvider::Compile(const std::vector(allocator_, max_ctx_mem_size_); } - trt_context = tensorrt_ptr::unique_pointer(trt_engine->createExecutionContextWithoutDeviceMemory()); + trt_context = std::unique_ptr(trt_engine->createExecutionContextWithoutDeviceMemory()); } else { - trt_context = tensorrt_ptr::unique_pointer(trt_engine->createExecutionContext()); + trt_context = std::unique_ptr(trt_engine->createExecutionContext()); } if (trt_context == nullptr) { return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL, @@ -1561,7 +1560,7 @@ common::Status TensorrtExecutionProvider::Compile(const std::vector engine_buf{new char[engine_size]}; engine_file.read((char*)engine_buf.get(), engine_size); - *(trt_state->engine) = tensorrt_ptr::unique_pointer( + *(trt_state->engine) = std::unique_ptr( trt_state->runtime->deserializeCudaEngine(engine_buf.get(), engine_size, nullptr)); if (trt_state->engine == nullptr) { return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL, "TensorRT EP Failed to Build Engine."); @@ -1569,10 +1568,10 @@ common::Status TensorrtExecutionProvider::Compile(const std::vectorengine->get(); if (trt_state->context_memory_sharing_enable) { - *(trt_state->context) = tensorrt_ptr::unique_pointer( + *(trt_state->context) = std::unique_ptr( trt_state->engine->get()->createExecutionContextWithoutDeviceMemory()); } else { - *(trt_state->context) = tensorrt_ptr::unique_pointer( + *(trt_state->context) = std::unique_ptr( trt_state->engine->get()->createExecutionContext()); } if (trt_state->context == nullptr) { @@ -1596,7 +1595,7 @@ common::Status TensorrtExecutionProvider::Compile(const std::vectorcontext->reset(); trt_state->engine->reset(); - *(trt_state->engine) = tensorrt_ptr::unique_pointer(trt_state->runtime->deserializeCudaEngine(engine_buf.get(), engine_size, nullptr)); + *(trt_state->engine) = std::unique_ptr(trt_state->runtime->deserializeCudaEngine(engine_buf.get(), engine_size, nullptr)); if (trt_state->engine == nullptr) { return ORT_MAKE_STATUS(ONNXRUNTIME, EP_FAIL, "TensorRT EP could not deserialize engine from encrypted cache: " + engine_cache_path); @@ -1604,10 +1603,10 @@ common::Status TensorrtExecutionProvider::Compile(const std::vectorengine->get(); if (trt_state->context_memory_sharing_enable) { - *(trt_state->context) = tensorrt_ptr::unique_pointer( + *(trt_state->context) = std::unique_ptr( trt_state->engine->get()->createExecutionContextWithoutDeviceMemory()); } else { - *(trt_state->context) = tensorrt_ptr::unique_pointer( + *(trt_state->context) = std::unique_ptr( trt_state->engine->get()->createExecutionContext()); } if (trt_state->context == nullptr) { @@ -1751,7 +1750,7 @@ common::Status TensorrtExecutionProvider::Compile(const std::vectorcontext->reset(); trt_state->engine->reset(); - auto trt_config = tensorrt_ptr::unique_pointer(trt_builder->createBuilderConfig()); + auto trt_config = std::unique_ptr(trt_builder->createBuilderConfig()); trt_config->setMaxWorkspaceSize(*(trt_state->max_workspace_size_ptr)); trt_config->addOptimizationProfile(*trt_profile); @@ -1783,7 +1782,7 @@ common::Status TensorrtExecutionProvider::Compile(const std::vectorengine) = tensorrt_ptr::unique_pointer( + *(trt_state->engine) = std::unique_ptr( trt_builder->buildEngineWithConfig(*trt_state->network->get(), *trt_config)); } if (trt_state->engine == nullptr) { @@ -1796,7 +1795,7 @@ common::Status TensorrtExecutionProvider::Compile(const std::vectorserialize(); + std::unique_ptr serializedModel(trt_engine->serialize()); size_t engine_size = serializedModel->size(); if (trt_state->engine_decryption_enable) { // Encrypt engine @@ -1808,15 +1807,14 @@ common::Status TensorrtExecutionProvider::Compile(const std::vector(serializedModel->data()), engine_size); } - serializedModel->destroy(); } // Build context if (trt_state->context_memory_sharing_enable) { - *(trt_state->context) = tensorrt_ptr::unique_pointer( + *(trt_state->context) = std::unique_ptr( trt_state->engine->get()->createExecutionContextWithoutDeviceMemory()); } else { - *(trt_state->context) = tensorrt_ptr::unique_pointer( + *(trt_state->context) = std::unique_ptr( trt_state->engine->get()->createExecutionContext()); } if (trt_state->context == nullptr) { diff --git a/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.h b/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.h index 2cb9826b89..4558b75fee 100644 --- a/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.h +++ b/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.h @@ -87,33 +87,33 @@ struct TensorrtFuncState { DestroyFunc test_release_func = nullptr; AllocatorHandle allocator = nullptr; tensorrt_ptr::unique_pointer* parser = nullptr; - tensorrt_ptr::unique_pointer* engine = nullptr; - tensorrt_ptr::unique_pointer* context = nullptr; - tensorrt_ptr::unique_pointer* builder = nullptr; - tensorrt_ptr::unique_pointer* network = nullptr; + std::unique_ptr* engine = nullptr; + std::unique_ptr* context = nullptr; + std::unique_ptr* builder = nullptr; + std::unique_ptr* network = nullptr; std::vector> input_info; std::vector> output_info; std::unordered_map>> input_shape_ranges; OrtMutex* tensorrt_mu_ptr = nullptr; - bool fp16_enable; - bool int8_enable; - bool int8_calibration_cache_available; - bool dla_enable; - int dla_core; + bool fp16_enable = false; + bool int8_enable = false; + bool int8_calibration_cache_available = false; + bool dla_enable = false; + int dla_core = 0; size_t* max_workspace_size_ptr = nullptr; std::string trt_node_name_with_precision; - bool engine_cache_enable; + bool engine_cache_enable = false; std::string engine_cache_path; nvinfer1::IRuntime* runtime = nullptr; nvinfer1::IOptimizationProfile* trt_profile = nullptr; AllocatorPtr scratch_allocator; - bool context_memory_sharing_enable; + bool context_memory_sharing_enable = false; size_t* max_context_mem_size_ptr = nullptr; IAllocatorUniquePtr* context_memory = nullptr; std::unordered_map dynamic_range_map; - bool engine_decryption_enable; - int (*engine_decryption)(const char*, char*, size_t*); - int (*engine_encryption)(const char*, char*, size_t); + bool engine_decryption_enable = false; + int (*engine_decryption)(const char*, char*, size_t*) = nullptr; + int (*engine_encryption)(const char*, char*, size_t) = nullptr; }; // Logical device representation. @@ -164,7 +164,7 @@ class TensorrtExecutionProvider : public IExecutionProvider { bool dump_subgraphs_ = false; bool engine_cache_enable_ = false; std::string cache_path_, engine_decryption_lib_path_; - tensorrt_ptr::unique_pointer runtime_ = nullptr; + std::unique_ptr runtime_ = nullptr; OrtMutex tensorrt_mu_; int device_id_; AllocatorPtr allocator_; @@ -172,17 +172,17 @@ class TensorrtExecutionProvider : public IExecutionProvider { bool layer_norm_fp32_fallback_ = false; size_t max_ctx_mem_size_ = 0; IAllocatorUniquePtr context_memory_ = nullptr; - mutable char model_path_[4096]; // Reserved for max path length + mutable char model_path_[4096] = {}; // Reserved for max path length bool engine_decryption_enable_ = false; - int (*engine_decryption_)(const char*, char*, size_t*); - int (*engine_encryption_)(const char*, char*, size_t); + int (*engine_decryption_)(const char*, char*, size_t*) = nullptr; + int (*engine_encryption_)(const char*, char*, size_t) = nullptr; std::unordered_set control_flow_op_set_ = {"If", "Loop", "Scan"}; std::unordered_map> parsers_; - std::unordered_map> engines_; - std::unordered_map> contexts_; - std::unordered_map> builders_; - std::unordered_map> networks_; + std::unordered_map> engines_; + std::unordered_map> contexts_; + std::unordered_map> builders_; + std::unordered_map> networks_; std::unordered_map>> input_info_; std::unordered_map>> output_info_; std::unordered_map>>> input_shape_ranges_; diff --git a/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider_utils.h b/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider_utils.h index d45ff4d05f..ad1fdf0022 100644 --- a/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider_utils.h +++ b/onnxruntime/core/providers/tensorrt/tensorrt_execution_provider_utils.h @@ -10,6 +10,7 @@ #include "ort_trt_int8_cal_table.fbs.h" #include #include "core/providers/cuda/cuda_pch.h" +#include "core/common/path_string.h" #include "core/framework/murmurhash3.h" namespace fs = std::experimental::filesystem; @@ -215,24 +216,18 @@ HashValue TRTGenerateId(const GraphViewer& graph_viewer) { MurmurHash3::x86_128(str.data(), gsl::narrow_cast(str.size()), hash[0], &hash); }; - // Use model name instead of path to avoid cache regeneration if path changes - const auto& model_path = main_graph.ModelPath(); - if (!model_path.IsEmpty()) { - // Get model name - PathString path_string = model_path.GetComponents().back(); - char arr[256]; -#ifdef _WIN32 - wcstombs_s(nullptr, arr, sizeof(arr), path_string.c_str(), sizeof(arr)); -#else - strcpy(arr, path_string.c_str()); -#endif - std::string model_name(arr); + // Use the model's file name instead of the entire path to avoid cache regeneration if path changes + const auto& model_path_components = main_graph.ModelPath().GetComponents(); + + if (!model_path_components.empty()) { + std::string model_name = PathToUTF8String(model_path_components.back()); + LOGS_DEFAULT(INFO) << "[TensorRT EP] Model name is " << model_name; // Ensure enough characters are hashed in case model names are too short - int32_t model_name_length = gsl::narrow_cast(model_name.size()); - constexpr int32_t hash_string_length = 500; + const size_t model_name_length = model_name.size(); + constexpr size_t hash_string_length = 500; std::string repeat_model_name = model_name; - for (int i = model_name_length; i > 0 && i < hash_string_length; i += model_name_length) { + for (size_t i = model_name_length; i > 0 && i < hash_string_length; i += model_name_length) { repeat_model_name += model_name; } hash_str(repeat_model_name); @@ -240,11 +235,6 @@ HashValue TRTGenerateId(const GraphViewer& graph_viewer) { LOGS_DEFAULT(INFO) << "[TensorRT EP] Model path is empty"; } - // fingerprint the main graph by hashing graph inputs - for (const auto* node_arg : main_graph.GetInputsIncludingInitializers()) { - hash_str(node_arg->Name()); - } - // fingerprint current graph by hashing graph inputs for (const auto* node_arg : graph_viewer.GetInputsIncludingInitializers()) { hash_str(node_arg->Name()); diff --git a/onnxruntime/core/session/provider_bridge_ort.cc b/onnxruntime/core/session/provider_bridge_ort.cc index 1d9e8e7bad..18079d3ee2 100644 --- a/onnxruntime/core/session/provider_bridge_ort.cc +++ b/onnxruntime/core/session/provider_bridge_ort.cc @@ -1033,6 +1033,10 @@ struct ProviderHostImpl : ProviderHost { MurmurHash3::x86_128(key, len, seed, out); } +#ifdef _WIN32 + std::string ToUTF8String(const std::wstring& s) override { return onnxruntime::ToUTF8String(s); } +#endif + ProviderHostCPU& GetProviderHostCPU() override { return onnxruntime::GetProviderHostCPU(); } } provider_host_; #if defined(_MSC_VER) && !defined(__clang__)