This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new a53f2bda5d ARROW-15415: [C++] Fixes for MSVC + vcpkg Debug build
(#13108)
a53f2bda5d is described below
commit a53f2bda5dfa50f2d1dbfd9d7fbc98d8c9725564
Author: Will Jones <[email protected]>
AuthorDate: Mon Jun 13 15:09:36 2022 +0200
ARROW-15415: [C++] Fixes for MSVC + vcpkg Debug build (#13108)
These are the changes needed for me to be able to compile Arrow in MSCV
(from Visual Studio 2022) with the following CMakeUserPresets entry:
```json
{
"name": "user-cpp-debug-mscv",
"inherits": [ "ninja-debug"],
"cacheVariables": {
"ARROW_DEPENDENCY_SOURCE": "VCPKG",
"CMAKE_BUILD_TYPE": "Debug",
"VCPKG_TARGET_TRIPLET": "x64-windows",
"VCPKG_LIBRARY_LINKAGE": "dynamic",
"ARROW_DEPENDENCY_USE_SHARED": "ON",
"ARROW_BUILD_EXAMPLES": "ON"
}
}
```
Authored-by: Will Jones <[email protected]>
Signed-off-by: David Li <[email protected]>
---
cpp/cmake_modules/FindThrift.cmake | 12 ++++-
cpp/src/arrow/compute/exec/exec_plan.cc | 6 ++-
cpp/src/arrow/compute/exec/hash_join.cc | 1 +
cpp/src/arrow/compute/exec/hash_join.h | 2 +-
cpp/src/arrow/compute/exec/hash_join_node.cc | 1 +
cpp/src/arrow/compute/function.cc | 1 +
cpp/src/arrow/compute/light_array_test.cc | 1 +
cpp/src/arrow/util/tracing.cc | 18 +++----
cpp/src/arrow/util/tracing.h | 44 +++-------------
cpp/src/arrow/util/tracing_internal.cc | 30 +++++++++--
cpp/src/arrow/util/tracing_internal.h | 75 +++++++++++++++++-----------
11 files changed, 107 insertions(+), 84 deletions(-)
diff --git a/cpp/cmake_modules/FindThrift.cmake
b/cpp/cmake_modules/FindThrift.cmake
index 5d195844e2..07028971d9 100644
--- a/cpp/cmake_modules/FindThrift.cmake
+++ b/cpp/cmake_modules/FindThrift.cmake
@@ -47,9 +47,17 @@ endfunction(EXTRACT_THRIFT_VERSION)
if(MSVC_TOOLCHAIN AND NOT DEFINED THRIFT_MSVC_LIB_SUFFIX)
if(NOT ARROW_THRIFT_USE_SHARED)
if(ARROW_USE_STATIC_CRT)
- set(THRIFT_MSVC_LIB_SUFFIX "mt")
+ if("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG")
+ set(THRIFT_MSVC_LIB_SUFFIX "mtd")
+ else()
+ set(THRIFT_MSVC_LIB_SUFFIX "mt")
+ endif()
else()
- set(THRIFT_MSVC_LIB_SUFFIX "md")
+ if("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG")
+ set(THRIFT_MSVC_LIB_SUFFIX "mdd")
+ else()
+ set(THRIFT_MSVC_LIB_SUFFIX "md")
+ endif()
endif()
endif()
endif()
diff --git a/cpp/src/arrow/compute/exec/exec_plan.cc
b/cpp/src/arrow/compute/exec/exec_plan.cc
index bbceb3151d..536e515ee8 100644
--- a/cpp/src/arrow/compute/exec/exec_plan.cc
+++ b/cpp/src/arrow/compute/exec/exec_plan.cc
@@ -85,9 +85,11 @@ struct ExecPlanImpl : public ExecPlan {
#ifdef ARROW_WITH_OPENTELEMETRY
if (HasMetadata()) {
auto pairs = metadata().get()->sorted_pairs();
+ opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> span =
+ ::arrow::internal::tracing::UnwrapSpan(span_.details.get());
std::for_each(std::begin(pairs), std::end(pairs),
- [this](std::pair<std::string, std::string> const& pair) {
- span_.Get().span->SetAttribute(pair.first, pair.second);
+ [span](std::pair<std::string, std::string> const& pair) {
+ span->SetAttribute(pair.first, pair.second);
});
}
#endif
diff --git a/cpp/src/arrow/compute/exec/hash_join.cc
b/cpp/src/arrow/compute/exec/hash_join.cc
index 63d9522c44..391c8b2cd4 100644
--- a/cpp/src/arrow/compute/exec/hash_join.cc
+++ b/cpp/src/arrow/compute/exec/hash_join.cc
@@ -29,6 +29,7 @@
#include "arrow/compute/exec/key_hash.h"
#include "arrow/compute/exec/task_util.h"
#include "arrow/compute/kernels/row_encoder.h"
+#include "arrow/util/tracing_internal.h"
namespace arrow {
namespace compute {
diff --git a/cpp/src/arrow/compute/exec/hash_join.h
b/cpp/src/arrow/compute/exec/hash_join.h
index 84685989a1..2b3bef0ead 100644
--- a/cpp/src/arrow/compute/exec/hash_join.h
+++ b/cpp/src/arrow/compute/exec/hash_join.h
@@ -28,7 +28,7 @@
#include "arrow/result.h"
#include "arrow/status.h"
#include "arrow/type.h"
-#include "arrow/util/tracing_internal.h"
+#include "arrow/util/tracing.h"
namespace arrow {
namespace compute {
diff --git a/cpp/src/arrow/compute/exec/hash_join_node.cc
b/cpp/src/arrow/compute/exec/hash_join_node.cc
index 8ea6883b55..60ad75228a 100644
--- a/cpp/src/arrow/compute/exec/hash_join_node.cc
+++ b/cpp/src/arrow/compute/exec/hash_join_node.cc
@@ -28,6 +28,7 @@
#include "arrow/util/future.h"
#include "arrow/util/make_unique.h"
#include "arrow/util/thread_pool.h"
+#include "arrow/util/tracing_internal.h"
namespace arrow {
diff --git a/cpp/src/arrow/compute/function.cc
b/cpp/src/arrow/compute/function.cc
index 2b3d4e6feb..fd80de4dbb 100644
--- a/cpp/src/arrow/compute/function.cc
+++ b/cpp/src/arrow/compute/function.cc
@@ -215,6 +215,7 @@ Result<Datum> Function::Execute(const std::vector<Datum>&
args,
}
util::tracing::Span span;
+
START_COMPUTE_SPAN(span, name(),
{{"function.name", name()},
{"function.options", options ? options->ToString() :
"<NULLPTR>"},
diff --git a/cpp/src/arrow/compute/light_array_test.cc
b/cpp/src/arrow/compute/light_array_test.cc
index 3f6d478035..dcc7841a09 100644
--- a/cpp/src/arrow/compute/light_array_test.cc
+++ b/cpp/src/arrow/compute/light_array_test.cc
@@ -18,6 +18,7 @@
#include "arrow/compute/light_array.h"
#include <gtest/gtest.h>
+#include <numeric>
#include "arrow/compute/exec/test_util.h"
#include "arrow/testing/generator.h"
diff --git a/cpp/src/arrow/util/tracing.cc b/cpp/src/arrow/util/tracing.cc
index b8bddcd505..8bf21f688c 100644
--- a/cpp/src/arrow/util/tracing.cc
+++ b/cpp/src/arrow/util/tracing.cc
@@ -16,30 +16,28 @@
// under the License.
#include "arrow/util/tracing.h"
+
+#include "arrow/util/config.h"
#include "arrow/util/make_unique.h"
#include "arrow/util/tracing_internal.h"
namespace arrow {
+
+using internal::make_unique;
namespace util {
namespace tracing {
#ifdef ARROW_WITH_OPENTELEMETRY
-Span::Impl& Span::Set(const Impl& impl) {
- inner_impl.reset(new Impl(impl));
- return *inner_impl;
-}
+Span::Span() noexcept { details =
make_unique<::arrow::internal::tracing::SpanImpl>(); }
-Span::Impl& Span::Set(Impl&& impl) {
- inner_impl.reset(new Impl(std::move(impl)));
- return *inner_impl;
+#else
+
+Span::Span() noexcept { /* details is left a nullptr */
}
#endif
-// Default destructor when impl type is complete.
-Span::~Span() = default;
-
} // namespace tracing
} // namespace util
} // namespace arrow
diff --git a/cpp/src/arrow/util/tracing.h b/cpp/src/arrow/util/tracing.h
index 15f7fca1ee..c6968219b6 100644
--- a/cpp/src/arrow/util/tracing.h
+++ b/cpp/src/arrow/util/tracing.h
@@ -19,49 +19,21 @@
#include <memory>
-#include "arrow/util/logging.h"
+#include "arrow/util/visibility.h"
namespace arrow {
-
-namespace internal {
-namespace tracing {
-
-// Forward declaration SpanImpl.
-class SpanImpl;
-
-} // namespace tracing
-} // namespace internal
-
namespace util {
namespace tracing {
-class ARROW_EXPORT Span {
+class ARROW_EXPORT SpanDetails {
public:
- using Impl = arrow::internal::tracing::SpanImpl;
-
- Span() = default; // Default constructor. The inner_impl is a nullptr.
- ~Span(); // Destructor. Default destructor defined in tracing.cc where impl
is a
- // complete type.
-
- Impl& Set(const Impl&);
- Impl& Set(Impl&&);
-
- const Impl& Get() const {
- ARROW_CHECK(inner_impl)
- << "Attempted to dereference a null pointer. Use Span::Set before "
- "dereferencing.";
- return *inner_impl;
- }
-
- Impl& Get() {
- ARROW_CHECK(inner_impl)
- << "Attempted to dereference a null pointer. Use Span::Set before "
- "dereferencing.";
- return *inner_impl;
- }
+ virtual ~SpanDetails() {}
+};
- private:
- std::unique_ptr<Impl> inner_impl;
+class ARROW_EXPORT Span {
+ public:
+ Span() noexcept;
+ std::unique_ptr<SpanDetails> details;
};
} // namespace tracing
diff --git a/cpp/src/arrow/util/tracing_internal.cc
b/cpp/src/arrow/util/tracing_internal.cc
index 904a1fd76a..668a2aaba8 100644
--- a/cpp/src/arrow/util/tracing_internal.cc
+++ b/cpp/src/arrow/util/tracing_internal.cc
@@ -202,14 +202,38 @@ opentelemetry::trace::Tracer* GetTracer() {
return tracer.get();
}
-#ifdef ARROW_WITH_OPENTELEMETRY
+opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& UnwrapSpan(
+ ::arrow::util::tracing::SpanDetails* span) {
+ SpanImpl* span_impl = checked_cast<SpanImpl*>(span);
+ ARROW_CHECK(span_impl->ot_span)
+ << "Attempted to dereference a null pointer. Use Span::Set before "
+ "dereferencing.";
+ return span_impl->ot_span;
+}
+
+const opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& UnwrapSpan(
+ const ::arrow::util::tracing::SpanDetails* span) {
+ const SpanImpl* span_impl = checked_cast<const SpanImpl*>(span);
+ ARROW_CHECK(span_impl->ot_span)
+ << "Attempted to dereference a null pointer. Use Span::Set before "
+ "dereferencing.";
+ return span_impl->ot_span;
+}
+
+opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& RewrapSpan(
+ ::arrow::util::tracing::SpanDetails* span,
+ opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> ot_span) {
+ SpanImpl* span_impl = checked_cast<SpanImpl*>(span);
+ span_impl->ot_span = std::move(ot_span);
+ return span_impl->ot_span;
+}
+
opentelemetry::trace::StartSpanOptions SpanOptionsWithParent(
const util::tracing::Span& parent_span) {
opentelemetry::trace::StartSpanOptions options;
- options.parent = parent_span.Get().span->GetContext();
+ options.parent = UnwrapSpan(parent_span.details.get())->GetContext();
return options;
}
-#endif
} // namespace tracing
} // namespace internal
diff --git a/cpp/src/arrow/util/tracing_internal.h
b/cpp/src/arrow/util/tracing_internal.h
index d0d6062e6e..2898fd245f 100644
--- a/cpp/src/arrow/util/tracing_internal.h
+++ b/cpp/src/arrow/util/tracing_internal.h
@@ -106,48 +106,63 @@ AsyncGenerator<T>
PropagateSpanThroughAsyncGenerator(AsyncGenerator<T> wrapped)
return PropagateSpanThroughAsyncGenerator(std::move(wrapped),
std::move(span));
}
-class SpanImpl {
+class SpanImpl : public ::arrow::util::tracing::SpanDetails {
public:
- opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> span;
+ ~SpanImpl() override = default;
+ opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> ot_span;
};
+opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& UnwrapSpan(
+ ::arrow::util::tracing::SpanDetails* span);
+
+const opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& UnwrapSpan(
+ const ::arrow::util::tracing::SpanDetails* span);
+
+opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& RewrapSpan(
+ ::arrow::util::tracing::SpanDetails* span,
+ opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> ot_span);
+
opentelemetry::trace::StartSpanOptions SpanOptionsWithParent(
const util::tracing::Span& parent_span);
-#define START_SPAN(target_span, ...)
\
- auto opentelemetry_scope##__LINE__ =
\
- ::arrow::internal::tracing::GetTracer()->WithActiveSpan(
\
- target_span
\
- .Set(::arrow::util::tracing::Span::Impl{
\
-
::arrow::internal::tracing::GetTracer()->StartSpan(__VA_ARGS__)}) \
- .span)
-
-#define START_SPAN_WITH_PARENT(target_span, parent_span, ...)
\
- auto opentelemetry_scope##__LINE__ =
\
- ::arrow::internal::tracing::GetTracer()->WithActiveSpan(
\
- target_span
\
- .Set(::arrow::util::tracing::Span::Impl{
\
- ::arrow::internal::tracing::GetTracer()->StartSpan(
\
- __VA_ARGS__,
\
-
::arrow::internal::tracing::SpanOptionsWithParent(parent_span))}) \
- .span)
-
-#define START_COMPUTE_SPAN(target_span, ...) \
- START_SPAN(target_span, __VA_ARGS__); \
- target_span.Get().span->SetAttribute( \
- "arrow.memory_pool_bytes",
::arrow::default_memory_pool()->bytes_allocated())
+#define START_SPAN(target_span, ...) \
+ auto opentelemetry_scope##__LINE__ = \
+ ::arrow::internal::tracing::GetTracer()->WithActiveSpan( \
+ ::arrow::internal::tracing::RewrapSpan( \
+ target_span.details.get(), \
+ ::arrow::internal::tracing::GetTracer()->StartSpan(__VA_ARGS__)))
+
+#define START_SPAN_WITH_PARENT(target_span, parent_span, ...) \
+ auto opentelemetry_scope##__LINE__ = \
+ ::arrow::internal::tracing::GetTracer()->WithActiveSpan( \
+ ::arrow::internal::tracing::RewrapSpan( \
+ target_span.details.get(), \
+ \
+ ::arrow::internal::tracing::GetTracer()->StartSpan( \
+ __VA_ARGS__, \
+
::arrow::internal::tracing::SpanOptionsWithParent(parent_span))))
+
+#define START_COMPUTE_SPAN(target_span, ...) \
+ START_SPAN(target_span, __VA_ARGS__); \
+ ::arrow::internal::tracing::UnwrapSpan(target_span.details.get()) \
+ ->SetAttribute("arrow.memory_pool_bytes", \
+ ::arrow::default_memory_pool()->bytes_allocated())
#define START_COMPUTE_SPAN_WITH_PARENT(target_span, parent_span, ...) \
START_SPAN_WITH_PARENT(target_span, parent_span, __VA_ARGS__); \
- target_span.Get().span->SetAttribute( \
- "arrow.memory_pool_bytes",
::arrow::default_memory_pool()->bytes_allocated())
+ ::arrow::internal::tracing::UnwrapSpan(target_span.details.get()) \
+ ->SetAttribute("arrow.memory_pool_bytes", \
+ ::arrow::default_memory_pool()->bytes_allocated())
-#define EVENT(target_span, ...) target_span.Get().span->AddEvent(__VA_ARGS__)
+#define EVENT(target_span, ...) \
+
::arrow::internal::tracing::UnwrapSpan(target_span.details.get())->AddEvent(__VA_ARGS__)
-#define MARK_SPAN(target_span, status) \
- ::arrow::internal::tracing::MarkSpan(status, target_span.Get().span.get())
+#define MARK_SPAN(target_span, status) \
+ ::arrow::internal::tracing::MarkSpan( \
+ status,
::arrow::internal::tracing::UnwrapSpan(target_span.details.get()).get())
-#define END_SPAN(target_span) target_span.Get().span->End()
+#define END_SPAN(target_span) \
+ ::arrow::internal::tracing::UnwrapSpan(target_span.details.get())->End()
#define END_SPAN_ON_FUTURE_COMPLETION(target_span, target_future,
target_capture) \
target_future = target_future.Then(
\
