Skip to content

Commit

Permalink
Prefer defaulted default constructor for Bitset (kokkos#6524)
Browse files Browse the repository at this point in the history
* Prefer defaulted default constructor for Bitset

The default argument to the constructor that takes the size of the
bitset was deferring to another constructor that creates an empty view
with a label argument.  This alocates 128 bits for the view header.
This showed when constructing an UnorderedMap with a pointless 128-bit
"header-only" allocation which implies an unnecessary fence.

* Fixup update Bitset allocated unit test

* Add tool-based test checking bitset default constructor does not allocate

* Check other Bitset constructors do allocate

* Drop weak test to see if NVCC ICE is gone
  • Loading branch information
dalg24 committed Oct 25, 2023
1 parent cf5a859 commit 3bcf965
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 5 deletions.
9 changes: 5 additions & 4 deletions containers/src/Kokkos_Bitset.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,10 @@ class Bitset {
using block_view_type = View<unsigned*, Device, MemoryTraits<RandomAccess>>;

public:
/// constructor
Bitset() = default;

/// arg_size := number of bit in set
Bitset(unsigned arg_size = 0u) : Bitset(Kokkos::view_alloc(), arg_size) {}
Bitset(unsigned arg_size) : Bitset(Kokkos::view_alloc(), arg_size) {}

template <class... P>
Bitset(const Impl::ViewCtorProp<P...>& arg_prop, unsigned arg_size)
Expand Down Expand Up @@ -291,8 +292,8 @@ class Bitset {
}

private:
unsigned m_size;
unsigned m_last_block_mask;
unsigned m_size = 0;
unsigned m_last_block_mask = 0;
block_view_type m_blocks;

private:
Expand Down
25 changes: 24 additions & 1 deletion containers/unit_tests/TestBitset.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
#include <Kokkos_Bitset.hpp>
#include <array>

#include <../../core/unit_test/tools/include/ToolTestingUtilities.hpp>

namespace Test {

namespace Impl {
Expand Down Expand Up @@ -155,7 +157,7 @@ void test_bitset() {

{
unsigned ts = 100u;
bitset_type b1;
bitset_type b1(Kokkos::view_alloc("MyBitset"), 0);
ASSERT_TRUE(b1.is_allocated());

b1 = bitset_type(ts);
Expand All @@ -165,6 +167,9 @@ void test_bitset() {
ASSERT_TRUE(b1.is_allocated());
ASSERT_TRUE(b2.is_allocated());
ASSERT_TRUE(b3.is_allocated());

bitset_type b4;
ASSERT_FALSE(b4.is_allocated());
}

std::array<unsigned, 7> test_sizes = {
Expand Down Expand Up @@ -237,6 +242,24 @@ void test_bitset() {
}

TEST(TEST_CATEGORY, bitset) { test_bitset<TEST_EXECSPACE>(); }

TEST(TEST_CATEGORY, bitset_default_constructor_no_alloc) {
using namespace Kokkos::Test::Tools;
listen_tool_events(Config::DisableAll(), Config::EnableAllocs());

auto success = validate_absence(
[&]() {
Kokkos::Bitset bs;
EXPECT_FALSE(bs.is_allocated());
},
[&](AllocateDataEvent) {
return MatchDiagnostic{true, {"Found alloc event"}};
});
ASSERT_TRUE(success);

listen_tool_events(Config::DisableAll());
}

} // namespace Test

#endif // KOKKOS_TEST_BITSET_HPP

0 comments on commit 3bcf965

Please sign in to comment.