From b30e0e2283ad814ca82b7eeceaf56579234ad962 Mon Sep 17 00:00:00 2001 From: Dmitri Smirnov Date: Sat, 26 Feb 2022 20:07:18 -0800 Subject: [PATCH] Remove inline_containers include from tensor_shape (#10682) Hide Inlined Hash set and maps guts behind template forward declarations. Currently CUDA 10.2 compiler can not compile abseil but provider interfaces use those types in their signatures. InlinedVector seems to be fine. Introduce core/common/inlined_containers_fwd.h header --- .../core/common/inlined_containers.h | 142 ++++++------------ .../core/common/inlined_containers_fwd.h | 121 +++++++++++++++ .../onnxruntime/core/framework/tensor_shape.h | 20 ++- .../core/framework/fallback_cpu_capability.cc | 1 + .../core/framework/fallback_cpu_capability.h | 2 +- .../providers/cpu/controlflow/scan_utils.h | 1 + .../core/providers/cpu/tensor/transpose.h | 2 + .../providers/shared_library/provider_api.h | 2 +- .../shared_library/provider_interfaces.h | 2 +- .../core/session/provider_bridge_ort.cc | 2 + 10 files changed, 193 insertions(+), 102 deletions(-) create mode 100644 include/onnxruntime/core/common/inlined_containers_fwd.h diff --git a/include/onnxruntime/core/common/inlined_containers.h b/include/onnxruntime/core/common/inlined_containers.h index ac94f7eb15..06739b3f1d 100644 --- a/include/onnxruntime/core/common/inlined_containers.h +++ b/include/onnxruntime/core/common/inlined_containers.h @@ -1,21 +1,12 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -//===- llvm/ADT/SmallVector.h - 'Normally small' vectors --------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -// This file contains code and comments derived from llvm/ADT/SmallVector.h -// -// Specifically CalculateInlinedVectorDefaultInlinedElements() template is derived from -// CalculateSmallVectorDefaultInlinedElements() and its comments. - #pragma once #include +#include "core/common/inlined_containers_fwd.h" + #ifdef _MSC_VER #pragma warning(push) // C4127: conditional expression is constant @@ -25,7 +16,6 @@ #pragma warning(disable : 4324) #endif -#include #include #include @@ -37,105 +27,63 @@ #endif namespace onnxruntime { -/// Inspired by LLVM SmallVector with ONNX Runtime adjustments for abseil. -/// -/// Helper class for calculating the default number of inline elements for -/// `InlinedVector`. -/// This produces the following on MSVC x64 -/// int8_t -> 41 -// int16_t -> 21 -// int32_t -> 11 -// int64_t -> 6 -// std::string 40 -> 1 -template -struct CalculateInlinedVectorDefaultInlinedElements { - // Parameter controlling the default number of inlined elements - // for `InlinedVector`. - // - // The default number of inlined elements ensures that - // 1. There is at least one inlined element. - // 2. `sizeof(InlinedVector) <= kPreferredInlinedVectorSizeof` unless - // it contradicts 1. - static constexpr size_t kPreferredInlinedVectorSizeof = 64; - - // static_assert that sizeof(T) is not "too big". - // - // Because the InlinedVector must have at least one inlined element, it is possible - // for an arbitrarily large inlined element to allocate an arbitrarily large - // amount of inline storage. So we want to call attention to these cases and - // make sure that users are making an intentional decision if they request a lot of inline storage. - // - // We want this assertion to trigger in pathological cases, but otherwise - // not be too easy to hit. To accomplish that, the cutoff is actually somewhat - // larger than kPreferredInlinedVectorSizeof (otherwise, - // `InlinedVector>` would be one easy way to trip it, and that - // pattern seems useful in practice). - // - // One wrinkle is that this assertion is in theory non-portable, since - // sizeof(absl::InlinedVector) is in general platform-dependent. However, we don't expect this - // to be much of an issue, because most LLVM development happens on 64-bit - // hosts, and therefore sizeof(T) is expected to *decrease* when compiled for - // 32-bit hosts, dodging the issue. The reverse situation, where development - // happens on a 32-bit host and then fails due to sizeof(T) *increasing* on a - // 64-bit host, is expected to be very rare. - static_assert( - sizeof(absl::InlinedVector) <= kPreferredInlinedVectorSizeof, - "You are trying to use a default number of inlined elements for " - "`InlinedVector` but `sizeof(T)` is really big! Please use an " - "explicit number of inlined elements with `InlinedVector` to make " - "sure you really want that much inline storage."); - - // Discount the size of the header itself when calculating the maximum inline - // bytes. - static constexpr size_t PreferredInlineBytes = - kPreferredInlinedVectorSizeof - (sizeof(absl::InlinedVector) - sizeof(T)); - static constexpr size_t NumElementsThatFit = PreferredInlineBytes / sizeof(T); - static constexpr size_t value = - NumElementsThatFit == 0 ? 1 : NumElementsThatFit; -}; - -// Use InlinedVector for small arrays that can fit on a stack with a default -// value pre-calculated. -// Use TensorShapeVector for shapes. -template ::value, - typename Allocator = std::allocator> -using InlinedVector = absl::InlinedVector; - // InlinedHashSet and InlinedHashMap are preferred // hash based containers. They store their values in the // buckets array that is allocated in one shot. It eliminates // per-node new/delete calls. Always call reserve() on any hash set/map // when the number of items is known in advance. // This does not allocate a dummy 'end' node on default construction. -template , - typename Eq = absl::container_internal::hash_default_eq, - typename Allocator = std::allocator> -using InlinedHashSet = absl::flat_hash_set; +template +class InlinedHashSet : public absl::flat_hash_set, + absl::container_internal::hash_default_eq, + Allocator> { + using Base = absl::flat_hash_set, + absl::container_internal::hash_default_eq, Allocator>; -template , - typename Eq = absl::container_internal::hash_default_eq, - typename Allocator = std::allocator>> -using InlinedHashMap = absl::flat_hash_map; + public: + using Base::Base; +}; +template +class InlinedHashMap : public absl::flat_hash_map, + absl::container_internal::hash_default_eq, + Allocator> { + using Base = absl::flat_hash_map, + absl::container_internal::hash_default_eq, Allocator>; + public: + using Base::Base; +}; // Use this hash set/map where pointer stability is required, otherwise use // InlinedHashSet and InlinedHashMap // This does not allocate a dummy 'end' node on default construction. // Use reserve() when the number of elements is known. -template , - class Eq = absl::container_internal::hash_default_eq, - class Alloc = std::allocator> -using NodeHashSet = absl::node_hash_set; +template +class NodeHashSet : public absl::node_hash_set, + absl::container_internal::hash_default_eq, + Alloc> { + using Base = absl::node_hash_set, + absl::container_internal::hash_default_eq, Alloc>; -template , - class Eq = absl::container_internal::hash_default_eq, - class Alloc = std::allocator>> -using NodeHashMap = absl::node_hash_map; + public: + using Base::Base; +}; + +template +class NodeHashMap : public absl::node_hash_map, + absl::container_internal::hash_default_eq, + Alloc> { + using Base = absl::node_hash_map, + absl::container_internal::hash_default_eq, Alloc>; + + public: + using Base::Base; +}; } // namespace onnxruntime diff --git a/include/onnxruntime/core/common/inlined_containers_fwd.h b/include/onnxruntime/core/common/inlined_containers_fwd.h new file mode 100644 index 0000000000..f7938e47eb --- /dev/null +++ b/include/onnxruntime/core/common/inlined_containers_fwd.h @@ -0,0 +1,121 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +#pragma once + +#include +#include + +#ifdef _MSC_VER +#pragma warning(push) +// C4127: conditional expression is constant +#pragma warning(disable : 4127) +// C4324: structure was padded due to alignment specifier +// Usage of alignas causes some internal padding in places. +#pragma warning(disable : 4324) +#endif + +#include + +#ifdef _MSC_VER +#pragma warning(pop) +#endif + +// Forward declarations for contexts where abseil can not be compiled and +// not really needed but we want to have it in the headers that are included +// e.g. CUDA 10 and .CU files +// InlinedVector seems to be fine with old CUDA + +//===- llvm/ADT/SmallVector.h - 'Normally small' vectors --------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// This file contains code and comments derived from llvm/ADT/SmallVector.h +// +// Specifically CalculateInlinedVectorDefaultInlinedElements() template is derived from +// CalculateSmallVectorDefaultInlinedElements() and its comments. + +namespace onnxruntime { +/// Inspired by LLVM SmallVector with ONNX Runtime adjustments for abseil. +/// +/// Helper class for calculating the default number of inline elements for +/// `InlinedVector`. +/// This produces the following on MSVC x64 +/// int8_t -> 41 +// int16_t -> 21 +// int32_t -> 11 +// int64_t -> 6 +// std::string 40 -> 1 +template +struct CalculateInlinedVectorDefaultInlinedElements { + // Parameter controlling the default number of inlined elements + // for `InlinedVector`. + // + // The default number of inlined elements ensures that + // 1. There is at least one inlined element. + // 2. `sizeof(InlinedVector) <= kPreferredInlinedVectorSizeof` unless + // it contradicts 1. + static constexpr size_t kPreferredInlinedVectorSizeof = 64; + + // static_assert that sizeof(T) is not "too big". + // + // Because the InlinedVector must have at least one inlined element, it is possible + // for an arbitrarily large inlined element to allocate an arbitrarily large + // amount of inline storage. So we want to call attention to these cases and + // make sure that users are making an intentional decision if they request a lot of inline storage. + // + // We want this assertion to trigger in pathological cases, but otherwise + // not be too easy to hit. To accomplish that, the cutoff is actually somewhat + // larger than kPreferredInlinedVectorSizeof (otherwise, + // `InlinedVector>` would be one easy way to trip it, and that + // pattern seems useful in practice). + // + // One wrinkle is that this assertion is in theory non-portable, since + // sizeof(absl::InlinedVector) is in general platform-dependent. However, we don't expect this + // to be much of an issue, because most LLVM development happens on 64-bit + // hosts, and therefore sizeof(T) is expected to *decrease* when compiled for + // 32-bit hosts, dodging the issue. The reverse situation, where development + // happens on a 32-bit host and then fails due to sizeof(T) *increasing* on a + // 64-bit host, is expected to be very rare. + static_assert( + sizeof(absl::InlinedVector) <= kPreferredInlinedVectorSizeof, + "You are trying to use a default number of inlined elements for " + "`InlinedVector` but `sizeof(T)` is really big! Please use an " + "explicit number of inlined elements with `InlinedVector` to make " + "sure you really want that much inline storage."); + + // Discount the size of the header itself when calculating the maximum inline + // bytes. + static constexpr size_t PreferredInlineBytes = + kPreferredInlinedVectorSizeof - (sizeof(absl::InlinedVector) - sizeof(T)); + static constexpr size_t NumElementsThatFit = PreferredInlineBytes / sizeof(T); + static constexpr size_t value = + NumElementsThatFit == 0 ? 1 : NumElementsThatFit; +}; + +// Use InlinedVector for small arrays that can fit on a stack with a default +// value pre-calculated. +// Use TensorShapeVector for shapes. +template ::value, + typename Allocator = std::allocator> +using InlinedVector = absl::InlinedVector; + +template > +class InlinedHashSet; + +template >> +class InlinedHashMap; + +template > +class NodeHashSet; + +template >> +class NodeHashMap; + +} diff --git a/include/onnxruntime/core/framework/tensor_shape.h b/include/onnxruntime/core/framework/tensor_shape.h index 84b13f8bc7..7a9c739138 100644 --- a/include/onnxruntime/core/framework/tensor_shape.h +++ b/include/onnxruntime/core/framework/tensor_shape.h @@ -10,7 +10,23 @@ #include #include "onnxruntime_config.h" -#include "core/common/inlined_containers.h" +// I have to bring it here because including inline_containers.h +// causes CUDA 10.2 compilers to fail +#ifdef _MSC_VER +#pragma warning(push) +// C4127: conditional expression is constant +#pragma warning(disable : 4127) +// C4324: structure was padded due to alignment specifier +// Usage of alignas causes some internal padding in places. +#pragma warning(disable : 4324) +#endif + +#include + +#ifdef _MSC_VER +#pragma warning(pop) +#endif + namespace onnxruntime { #ifdef __GNUC__ @@ -23,7 +39,7 @@ namespace onnxruntime { constexpr size_t kTensorShapeSmallBufferElementsSize = 5; // Use this type to build a shape and then create TensorShape. -using TensorShapeVector = InlinedVector; +using TensorShapeVector = absl::InlinedVector; inline TensorShapeVector ToShapeVector(const gsl::span& span) { TensorShapeVector out; diff --git a/onnxruntime/core/framework/fallback_cpu_capability.cc b/onnxruntime/core/framework/fallback_cpu_capability.cc index 732be2c6c3..f12598cd58 100644 --- a/onnxruntime/core/framework/fallback_cpu_capability.cc +++ b/onnxruntime/core/framework/fallback_cpu_capability.cc @@ -2,6 +2,7 @@ // Licensed under the MIT License. #include "core/framework/fallback_cpu_capability.h" +#include "core/common/inlined_containers.h" #include diff --git a/onnxruntime/core/framework/fallback_cpu_capability.h b/onnxruntime/core/framework/fallback_cpu_capability.h index 9d5cc181d7..fda5fdb188 100644 --- a/onnxruntime/core/framework/fallback_cpu_capability.h +++ b/onnxruntime/core/framework/fallback_cpu_capability.h @@ -3,7 +3,7 @@ #pragma once -#include "core/common/inlined_containers.h" +#include "core/common/inlined_containers_fwd.h" #include "core/framework/kernel_registry.h" #include "core/graph/graph_viewer.h" #include diff --git a/onnxruntime/core/providers/cpu/controlflow/scan_utils.h b/onnxruntime/core/providers/cpu/controlflow/scan_utils.h index baf1b263bf..2be61e2c6a 100644 --- a/onnxruntime/core/providers/cpu/controlflow/scan_utils.h +++ b/onnxruntime/core/providers/cpu/controlflow/scan_utils.h @@ -7,6 +7,7 @@ #include #include "core/common/common.h" +#include "core/common/inlined_containers.h" #include "core/framework/allocator.h" #include "core/framework/feeds_fetches_manager.h" #include "core/framework/ort_value.h" diff --git a/onnxruntime/core/providers/cpu/tensor/transpose.h b/onnxruntime/core/providers/cpu/tensor/transpose.h index 50c56ca0e4..7672e093e4 100644 --- a/onnxruntime/core/providers/cpu/tensor/transpose.h +++ b/onnxruntime/core/providers/cpu/tensor/transpose.h @@ -5,6 +5,8 @@ #ifndef SHARED_PROVIDER #include "core/common/common.h" +#include "core/common/inlined_containers.h" +#include "core/framework/tensor_shape.h" #include "core/framework/op_kernel.h" #endif diff --git a/onnxruntime/core/providers/shared_library/provider_api.h b/onnxruntime/core/providers/shared_library/provider_api.h index 1d7426cb59..b36c0c15c8 100644 --- a/onnxruntime/core/providers/shared_library/provider_api.h +++ b/onnxruntime/core/providers/shared_library/provider_api.h @@ -18,7 +18,7 @@ #include #include "core/common/common.h" #include "core/common/const_pointer_container.h" -#include "core/common/inlined_containers.h" +#include "core/common/inlined_containers_fwd.h" #include "core/common/type_list.h" #include "core/common/logging/severity.h" #include "core/framework/allocator.h" diff --git a/onnxruntime/core/providers/shared_library/provider_interfaces.h b/onnxruntime/core/providers/shared_library/provider_interfaces.h index 6fe810b7c9..4afb294648 100644 --- a/onnxruntime/core/providers/shared_library/provider_interfaces.h +++ b/onnxruntime/core/providers/shared_library/provider_interfaces.h @@ -4,7 +4,7 @@ // Public wrappers around internal ort interfaces (currently) #include "core/providers/shared_library/provider_host_api.h" -#include "core/common/inlined_containers.h" +#include "core/common/inlined_containers_fwd.h" #include "core/providers/shared/common.h" #define PROVIDER_DISALLOW_ALL(TypeName) \ diff --git a/onnxruntime/core/session/provider_bridge_ort.cc b/onnxruntime/core/session/provider_bridge_ort.cc index 684d7be7cf..fe1a6a2c88 100644 --- a/onnxruntime/core/session/provider_bridge_ort.cc +++ b/onnxruntime/core/session/provider_bridge_ort.cc @@ -3,6 +3,8 @@ // This is the Onnxruntime side of the bridge to allow providers to be built as a DLL // It implements onnxruntime::ProviderHost + +#include "core/common/inlined_containers.h" #include "core/framework/allocatormgr.h" #include "core/framework/compute_capability.h" #include "core/framework/data_types.h"