From 8c5c8e9bda3d678a2068d233e387c7ccad5ebc21 Mon Sep 17 00:00:00 2001 From: fis Date: Wed, 1 Apr 2020 07:07:47 +0800 Subject: [PATCH 1/4] Reduce span check overhead. --- include/xgboost/span.h | 40 +++++++++++-------- tests/cpp/common/test_span.cc | 50 ++++++++++++------------ tests/cpp/common/test_transform_range.cc | 4 +- 3 files changed, 50 insertions(+), 44 deletions(-) diff --git a/include/xgboost/span.h b/include/xgboost/span.h index 0764849eb596..b28995323610 100644 --- a/include/xgboost/span.h +++ b/include/xgboost/span.h @@ -29,11 +29,13 @@ #ifndef XGBOOST_SPAN_H_ #define XGBOOST_SPAN_H_ -#include // CHECK +#include #include // size_t -#include // numeric_limits +#include // numeric_limits +#include #include +#include /*! * The version number 1910 is picked up from GSL. @@ -69,27 +71,31 @@ namespace xgboost { namespace common { // Usual logging facility is not available inside device code. -// TODO(trivialfis): Make dmlc check more generic. // assert is not supported in mac as of CUDA 10.0 -#define KERNEL_CHECK(cond) \ - do { \ - if (!(cond)) { \ - printf("\nKernel error:\n" \ - "In: %s: %d\n" \ - "\t%s\n\tExpecting: %s\n" \ - "\tBlock: [%d, %d, %d], Thread: [%d, %d, %d]\n\n", \ - __FILE__, __LINE__, __PRETTY_FUNCTION__, #cond, \ - blockIdx.x, blockIdx.y, blockIdx.z, \ - threadIdx.x, threadIdx.y, threadIdx.z); \ - asm("trap;"); \ - } \ +#define KERNEL_CHECK(cond) \ + do { \ + if (!(cond)) { \ + printf("\nKernel error:\n" \ + "In: %s: %d\n" \ + "\t%s\n\tExpecting: %s\n" \ + "\tBlock: [%d, %d, %d], Thread: [%d, %d, %d]\n\n", \ + __FILE__, __LINE__, __PRETTY_FUNCTION__, #cond, blockIdx.x, \ + blockIdx.y, blockIdx.z, threadIdx.x, threadIdx.y, threadIdx.z); \ + asm("trap;"); \ + } \ } while (0); #ifdef __CUDA_ARCH__ #define SPAN_CHECK KERNEL_CHECK #else -#define SPAN_CHECK CHECK // check from dmlc -#endif // __CUDA_ARCH__ +#define SPAN_CHECK(cond) \ + do { \ + if (XGBOOST_EXPECT(!(cond), false)) { \ + fprintf(stderr, "[xgboost] Condition %s failed.\n", #cond); \ + std::terminate(); \ + }; \ + } while (0); +#endif // __CUDA_ARCH__ namespace detail { /*! diff --git a/tests/cpp/common/test_span.cc b/tests/cpp/common/test_span.cc index c27075b93bef..550d826b8257 100644 --- a/tests/cpp/common/test_span.cc +++ b/tests/cpp/common/test_span.cc @@ -99,7 +99,7 @@ TEST(Span, FromPtrLen) { { auto lazy = [=]() {Span tmp (arr, 5);}; - EXPECT_ANY_THROW(lazy()); + EXPECT_DEATH(lazy(), "\\[xgboost\\] Condition .* failed.\n"); } // dynamic extent @@ -286,11 +286,11 @@ TEST(Span, ElementAccess) { ++j; } - EXPECT_ANY_THROW(s[16]); - EXPECT_ANY_THROW(s[-1]); + EXPECT_DEATH(s[16], "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s[-1], "\\[xgboost\\] Condition .* failed.\n"); - EXPECT_ANY_THROW(s(16)); - EXPECT_ANY_THROW(s(-1)); + EXPECT_DEATH(s(16), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s(-1), "\\[xgboost\\] Condition .* failed.\n"); } TEST(Span, Obversers) { @@ -315,13 +315,13 @@ TEST(Span, FrontBack) { { Span s; - EXPECT_ANY_THROW(s.front()); - EXPECT_ANY_THROW(s.back()); + EXPECT_DEATH(s.front(), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.back(), "\\[xgboost\\] Condition .* failed.\n"); } { Span s; - EXPECT_ANY_THROW(s.front()); - EXPECT_ANY_THROW(s.back()); + EXPECT_DEATH(s.front(), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.back(), "\\[xgboost\\] Condition .* failed.\n"); } } @@ -341,9 +341,9 @@ TEST(Span, FirstLast) { ASSERT_EQ(first[i], arr[i]); } auto constexpr kOne = static_cast::index_type>(-1); - EXPECT_ANY_THROW(s.first()); - EXPECT_ANY_THROW(s.first<17>()); - EXPECT_ANY_THROW(s.first<32>()); + EXPECT_DEATH(s.first(), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.first<17>(), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.first<32>(), "\\[xgboost\\] Condition .* failed.\n"); } { @@ -360,9 +360,9 @@ TEST(Span, FirstLast) { ASSERT_EQ(last[i], arr[i+12]); } auto constexpr kOne = static_cast::index_type>(-1); - EXPECT_ANY_THROW(s.last()); - EXPECT_ANY_THROW(s.last<17>()); - EXPECT_ANY_THROW(s.last<32>()); + EXPECT_DEATH(s.last(), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.last<17>(), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.last<32>(), "\\[xgboost\\] Condition .* failed.\n"); } // dynamic extent @@ -379,9 +379,9 @@ TEST(Span, FirstLast) { ASSERT_EQ(first[i], s[i]); } - EXPECT_ANY_THROW(s.first(-1)); - EXPECT_ANY_THROW(s.first(17)); - EXPECT_ANY_THROW(s.first(32)); + EXPECT_DEATH(s.first(-1), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.first(17), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.first(32), "\\[xgboost\\] Condition .* failed.\n"); delete [] arr; } @@ -399,9 +399,9 @@ TEST(Span, FirstLast) { ASSERT_EQ(s[12 + i], last[i]); } - EXPECT_ANY_THROW(s.last(-1)); - EXPECT_ANY_THROW(s.last(17)); - EXPECT_ANY_THROW(s.last(32)); + EXPECT_DEATH(s.last(-1), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.last(17), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s.last(32), "\\[xgboost\\] Condition .* failed.\n"); delete [] arr; } @@ -421,12 +421,12 @@ TEST(Span, Subspan) { ASSERT_EQ(s1.data() + 2, s4.data()); ASSERT_EQ(s4.size(), s1.size() - 2); - EXPECT_ANY_THROW(s1.subspan(-1, 0)); - EXPECT_ANY_THROW(s1.subspan(16, 0)); + EXPECT_DEATH(s1.subspan(-1, 0), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s1.subspan(16, 0), "\\[xgboost\\] Condition .* failed.\n"); auto constexpr kOne = static_cast::index_type>(-1); - EXPECT_ANY_THROW(s1.subspan()); - EXPECT_ANY_THROW(s1.subspan<16>()); + EXPECT_DEATH(s1.subspan(), "\\[xgboost\\] Condition .* failed.\n"); + EXPECT_DEATH(s1.subspan<16>(), "\\[xgboost\\] Condition .* failed.\n"); } TEST(Span, Compare) { diff --git a/tests/cpp/common/test_transform_range.cc b/tests/cpp/common/test_transform_range.cc index 81bc73962818..68319dfd3ff0 100644 --- a/tests/cpp/common/test_transform_range.cc +++ b/tests/cpp/common/test_transform_range.cc @@ -63,11 +63,11 @@ TEST(Transform, Exception) { size_t const kSize {16}; std::vector h_in(kSize); const HostDeviceVector in_vec{h_in, -1}; - EXPECT_ANY_THROW({ + EXPECT_DEATH({ Transform<>::Init([](size_t idx, common::Span _in) { _in[idx + 1]; }, Range(0, static_cast(kSize)), -1) .Eval(&in_vec); - }); + }, ""); } #endif From a7d179268359a4b5e07f0cab0dc7aa7cdadcf55d Mon Sep 17 00:00:00 2001 From: fis Date: Wed, 1 Apr 2020 07:26:31 +0800 Subject: [PATCH 2/4] lint. --- include/xgboost/span.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/xgboost/span.h b/include/xgboost/span.h index b28995323610..52e5e67470e8 100644 --- a/include/xgboost/span.h +++ b/include/xgboost/span.h @@ -93,9 +93,9 @@ namespace common { if (XGBOOST_EXPECT(!(cond), false)) { \ fprintf(stderr, "[xgboost] Condition %s failed.\n", #cond); \ std::terminate(); \ - }; \ + } \ } while (0); -#endif // __CUDA_ARCH__ +#endif // __CUDA_ARCH__ namespace detail { /*! From fb5b6199da865562623656157e2a86d890e47b83 Mon Sep 17 00:00:00 2001 From: fis Date: Wed, 1 Apr 2020 16:46:42 +0800 Subject: [PATCH 3/4] Try flushing stderr. --- include/xgboost/span.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/xgboost/span.h b/include/xgboost/span.h index 52e5e67470e8..1a2dd5774709 100644 --- a/include/xgboost/span.h +++ b/include/xgboost/span.h @@ -92,10 +92,11 @@ namespace common { do { \ if (XGBOOST_EXPECT(!(cond), false)) { \ fprintf(stderr, "[xgboost] Condition %s failed.\n", #cond); \ + fflush(stderr); /* It seems stderr on Windows is beffered? */ \ std::terminate(); \ } \ } while (0); -#endif // __CUDA_ARCH__ +#endif // __CUDA_ARCH__ namespace detail { /*! From 1e6696ec592a4bcae43a1f1655fb625452ceed2f Mon Sep 17 00:00:00 2001 From: fis Date: Wed, 1 Apr 2020 16:55:43 +0800 Subject: [PATCH 4/4] lint. --- include/xgboost/span.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/xgboost/span.h b/include/xgboost/span.h index 1a2dd5774709..1750ac2c4b16 100644 --- a/include/xgboost/span.h +++ b/include/xgboost/span.h @@ -96,7 +96,7 @@ namespace common { std::terminate(); \ } \ } while (0); -#endif // __CUDA_ARCH__ +#endif // __CUDA_ARCH__ namespace detail { /*!