Skip to content

Commit

Permalink
Update MemoryAllocatorDebugger
Browse files Browse the repository at this point in the history
  • Loading branch information
jslee02 committed Nov 16, 2024
1 parent 17ffab2 commit 826198f
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 60 deletions.
18 changes: 8 additions & 10 deletions dart/common/MemoryAllocatorDebugger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@

namespace dart::common {

template <typename T>
class MemoryAllocatorDebugger : public MemoryAllocator
template <typename Derived>
class MemoryAllocatorDebugger : public Derived
{
public:
/// Constructor
Expand All @@ -56,13 +56,13 @@ class MemoryAllocatorDebugger : public MemoryAllocator
[[nodiscard]] static const std::string& getStaticType();

// Documentation inherited
[[nodiscard]] const std::string& getType() const override;
[[nodiscard]] const std::string& getType() const;

// Documentation inherited
[[nodiscard]] void* allocate(size_t bytes) noexcept override;
[[nodiscard]] void* allocate(size_t bytes) noexcept;

// Documentation inherited
void deallocate(void* pointer, size_t bytes) override;
void deallocate(void* pointer, size_t bytes);

/// Returns true if there is no memory allocated by the internal allocator.
[[nodiscard]] bool isEmpty() const;
Expand All @@ -71,17 +71,15 @@ class MemoryAllocatorDebugger : public MemoryAllocator
[[nodiscard]] bool hasAllocated(void* pointer, size_t size) const;

/// Returns the internal allocator
[[nodiscard]] const T& getInternalAllocator() const;
[[nodiscard]] const Derived& derived() const;

/// Returns the internal allocator
[[nodiscard]] T& getInternalAllocator();
[[nodiscard]] Derived& derived();

// Documentation inherited
void print(std::ostream& os = std::cout, int indent = 0) const override;
void print(std::ostream& os = std::cout, int indent = 0) const;

private:
T mInternalAllocator;

size_t mSize = 0;

size_t mPeak = 0;
Expand Down
12 changes: 12 additions & 0 deletions dart/common/MemoryManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ void* MemoryManager::allocate(Type type, size_t bytes)
return nullptr;
}

//==============================================================================
void* MemoryManager::allocate(size_t bytes) noexcept
{
return allocate(Type::Base, bytes);
}

//==============================================================================
void* MemoryManager::allocateUsingFree(size_t bytes)
{
Expand Down Expand Up @@ -120,6 +126,12 @@ void MemoryManager::deallocate(Type type, void* pointer, size_t bytes)
}
}

//==============================================================================
void MemoryManager::deallocate(void* pointer, size_t bytes)
{
deallocate(Type::Base, pointer, bytes);
}

//==============================================================================
void MemoryManager::deallocateUsingFree(void* pointer, size_t bytes)
{
Expand Down
10 changes: 9 additions & 1 deletion dart/common/MemoryManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ namespace dart::common {

/// A composite memory allocator that contains various memory allocators that
/// are optimized for different use cases.
class MemoryManager final
class MemoryManager final : public MemoryAllocator
{
public:
/// Type of the memory allocators
Expand All @@ -66,6 +66,8 @@ class MemoryManager final
/// Destructor
~MemoryManager();

DART_STRING_TYPE(MemoryManager);

/// Returns the base allocator
[[nodiscard]] MemoryAllocator& getBaseAllocator();

Expand All @@ -84,6 +86,9 @@ class MemoryManager final
/// \return On failure, a null pointer
[[nodiscard]] void* allocate(Type type, size_t bytes);

/// Allocates using base allocator
[[nodiscard]] void* allocate(size_t bytes) noexcept final;

/// Allocates \c size bytes of uninitialized storage using FreeListAllocator.
///
/// \param[in] bytes: The byte size to allocate sotrage for.
Expand All @@ -109,6 +114,9 @@ class MemoryManager final
void deallocate(Type type, void* pointer, size_t bytes);
// TODO(JS): Make this constexpr once migrated to C++20

/// Deallocates using base allocator
void deallocate(void* pointer, size_t bytes) final;

void deallocateUsingFree(void* pointer, size_t bytes);

void deallocateUsingPool(void* pointer, size_t bytes);
Expand Down
59 changes: 30 additions & 29 deletions dart/common/detail/MemoryAllocatorDebugger-impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,17 @@
namespace dart::common {

//==============================================================================
template <typename T>
template <typename Derived>
template <typename... Args>
MemoryAllocatorDebugger<T>::MemoryAllocatorDebugger(Args&&... args)
: mInternalAllocator(std::forward<Args>(args)...)
MemoryAllocatorDebugger<Derived>::MemoryAllocatorDebugger(Args&&... args)
: Derived(std::forward<Args>(args)...)
{
// Do nothing
}

//==============================================================================
template <typename T>
MemoryAllocatorDebugger<T>::~MemoryAllocatorDebugger()
template <typename Derived>
MemoryAllocatorDebugger<Derived>::~MemoryAllocatorDebugger()
{
// Lock the mutex
std::lock_guard<std::mutex> lock(mMutex);
Expand All @@ -76,26 +76,26 @@ MemoryAllocatorDebugger<T>::~MemoryAllocatorDebugger()
}

//==============================================================================
template <typename T>
const std::string& MemoryAllocatorDebugger<T>::getStaticType()
template <typename Derived>
const std::string& MemoryAllocatorDebugger<Derived>::getStaticType()
{
static const std::string type
= "MemoryAllocatorDebugger<" + T::getStaticType() + ">";
= "MemoryAllocatorDebugger<" + Derived::getStaticType() + ">";
return type;
}

//==============================================================================
template <typename T>
const std::string& MemoryAllocatorDebugger<T>::getType() const
template <typename Derived>
const std::string& MemoryAllocatorDebugger<Derived>::getType() const
{
return getStaticType();
}

//==============================================================================
template <typename T>
void* MemoryAllocatorDebugger<T>::allocate(size_t bytes) noexcept
template <typename Derived>
void* MemoryAllocatorDebugger<Derived>::allocate(size_t bytes) noexcept
{
void* newPtr = mInternalAllocator.allocate(bytes);
void* newPtr = derived().allocate(bytes);

if (newPtr) {
std::lock_guard<std::mutex> lock(mMutex);
Expand All @@ -108,8 +108,8 @@ void* MemoryAllocatorDebugger<T>::allocate(size_t bytes) noexcept
}

//==============================================================================
template <typename T>
void MemoryAllocatorDebugger<T>::deallocate(void* pointer, size_t bytes)
template <typename Derived>
void MemoryAllocatorDebugger<Derived>::deallocate(void* pointer, size_t bytes)
{
std::lock_guard<std::mutex> lock(mMutex);

Expand All @@ -132,22 +132,23 @@ void MemoryAllocatorDebugger<T>::deallocate(void* pointer, size_t bytes)
return;
}

mInternalAllocator.deallocate(pointer, bytes);
derived().deallocate(pointer, bytes);
mMapPointerToSize.erase(it);
mSize -= bytes;
}

//==============================================================================
template <typename T>
bool MemoryAllocatorDebugger<T>::isEmpty() const
template <typename Derived>
bool MemoryAllocatorDebugger<Derived>::isEmpty() const
{
std::lock_guard<std::mutex> lock(mMutex);
return mMapPointerToSize.empty();
}

//==============================================================================
template <typename T>
bool MemoryAllocatorDebugger<T>::hasAllocated(void* pointer, size_t size) const
template <typename Derived>
bool MemoryAllocatorDebugger<Derived>::hasAllocated(
void* pointer, size_t size) const
{
std::lock_guard<std::mutex> lock(mMutex);

Expand All @@ -165,22 +166,22 @@ bool MemoryAllocatorDebugger<T>::hasAllocated(void* pointer, size_t size) const
}

//==============================================================================
template <typename T>
const T& MemoryAllocatorDebugger<T>::getInternalAllocator() const
template <typename Derived>
const Derived& MemoryAllocatorDebugger<Derived>::derived() const
{
return mInternalAllocator;
return *static_cast<const Derived*>(this);
}

//==============================================================================
template <typename T>
T& MemoryAllocatorDebugger<T>::getInternalAllocator()
template <typename Derived>
Derived& MemoryAllocatorDebugger<Derived>::derived()
{
return mInternalAllocator;
return *static_cast<Derived*>(this);
}

//==============================================================================
template <typename T>
void MemoryAllocatorDebugger<T>::print(std::ostream& os, int indent) const
template <typename Derived>
void MemoryAllocatorDebugger<Derived>::print(std::ostream& os, int indent) const
{
if (indent == 0) {
os << "[" << getType() << "]\n";
Expand All @@ -193,7 +194,7 @@ void MemoryAllocatorDebugger<T>::print(std::ostream& os, int indent) const
os << spaces << "size_in_bytes: " << mSize << "\n";
os << spaces << "peak: " << mPeak << "\n";
os << spaces << "internal_allocator:\n";
mInternalAllocator.print(os, indent + 2);
derived().print(os, indent + 2);
}

} // namespace dart::common
Expand Down
7 changes: 5 additions & 2 deletions pixi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,17 @@ lint-cpp = { cmd = """
cmake \
--build build/$PIXI_ENVIRONMENT_NAME/cpp/$BUILD_TYPE \
--target format
""", depends_on = ["config"] }
""", depends_on = ["config"], env = { BUILD_TYPE = "Release" } }

lint-py = { cmd = """
black . --exclude '\\..*' \
&& isort . --skip-glob '.*'
""", depends_on = ["config"] }

lint = { depends_on = ["lint-cpp", "lint-py"] }
lint = { cmd = "echo linting with $BUILD_TYPE", depends_on = [
"lint-cpp",
"lint-py",
], env = { BUILD_TYPE = "Release" } }

check-lint-cpp = { cmd = """
cmake \
Expand Down
8 changes: 2 additions & 6 deletions tests/unit/common/test_FreeListAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,10 @@ using namespace common;
TEST(FreeListAllocatorTest, Constructors)
{
auto a = FreeListAllocator::Debug();
EXPECT_EQ(
&a.getInternalAllocator().getBaseAllocator(),
&MemoryAllocator::GetDefault());
EXPECT_EQ(&a.derived().getBaseAllocator(), &MemoryAllocator::GetDefault());

auto b = FreeListAllocator::Debug(MemoryAllocator::GetDefault());
EXPECT_EQ(
&b.getInternalAllocator().getBaseAllocator(),
&MemoryAllocator::GetDefault());
EXPECT_EQ(&b.derived().getBaseAllocator(), &MemoryAllocator::GetDefault());

EXPECT_TRUE(a.isEmpty());
EXPECT_TRUE(b.isEmpty());
Expand Down
4 changes: 4 additions & 0 deletions tests/unit/common/test_MemoryManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ TEST(MemoryManagerTest, BaseAllocator)
//==============================================================================
TEST(MemoryManagerTest, Allocate)
{
#ifdef NDEBUG // Release
auto mm = MemoryManager();
#else
auto mm = MemoryAllocatorDebugger<MemoryManager>();
#endif

// Cannot allocate 0 bytes
EXPECT_EQ(mm.allocateUsingFree(0), nullptr);
Expand Down
20 changes: 8 additions & 12 deletions tests/unit/common/test_PoolAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,12 @@ using namespace common;
TEST(PoolAllocatorTest, Constructors)
{
auto a = PoolAllocator::Debug();
EXPECT_EQ(
&a.getInternalAllocator().getBaseAllocator(),
&MemoryAllocator::GetDefault());
EXPECT_EQ(&a.derived().getBaseAllocator(), &MemoryAllocator::GetDefault());

auto b = PoolAllocator::Debug(MemoryAllocator::GetDefault());
EXPECT_EQ(
&b.getInternalAllocator().getBaseAllocator(),
&MemoryAllocator::GetDefault());
EXPECT_EQ(&b.derived().getBaseAllocator(), &MemoryAllocator::GetDefault());

EXPECT_EQ(b.getInternalAllocator().getNumAllocatedMemoryBlocks(), 0);
EXPECT_EQ(b.derived().getNumAllocatedMemoryBlocks(), 0);

EXPECT_TRUE(a.isEmpty());
EXPECT_TRUE(b.isEmpty());
Expand All @@ -73,33 +69,33 @@ TEST(PoolAllocatorTest, Allocate)
EXPECT_TRUE(a.hasAllocated(ptr1, 1));
EXPECT_FALSE(a.hasAllocated(0, 1)); // incorrect address
EXPECT_FALSE(a.hasAllocated(ptr1, 1 * 2)); // incorrect size
EXPECT_EQ(a.getInternalAllocator().getNumAllocatedMemoryBlocks(), 1);
EXPECT_EQ(a.derived().getNumAllocatedMemoryBlocks(), 1);

// Allocate the same size, which doesn't increase the number of memory block
auto ptr2 = a.allocate(1);
EXPECT_NE(ptr2, nullptr);
EXPECT_EQ(a.getInternalAllocator().getNumAllocatedMemoryBlocks(), 1);
EXPECT_EQ(a.derived().getNumAllocatedMemoryBlocks(), 1);

// Allocate different size
auto ptr3 = a.allocate(64);
EXPECT_NE(ptr3, nullptr);
EXPECT_EQ(a.getInternalAllocator().getNumAllocatedMemoryBlocks(), 2);
EXPECT_EQ(a.derived().getNumAllocatedMemoryBlocks(), 2);

// Allocate memory of the max size (= 1024)
auto ptr4 = a.allocate(1024);
EXPECT_NE(ptr4, nullptr);
EXPECT_TRUE(a.hasAllocated(ptr4, 1024));
EXPECT_FALSE(a.hasAllocated(0, 1024));
EXPECT_FALSE(a.hasAllocated(ptr4, 1024 * 2));
EXPECT_EQ(a.getInternalAllocator().getNumAllocatedMemoryBlocks(), 3);
EXPECT_EQ(a.derived().getNumAllocatedMemoryBlocks(), 3);

// Allocate oversized memory (> 1024)
auto ptr5 = a.allocate(2048);
EXPECT_NE(ptr5, nullptr);
EXPECT_TRUE(a.hasAllocated(ptr5, 2048));
EXPECT_FALSE(a.hasAllocated(0, 2048));
EXPECT_FALSE(a.hasAllocated(ptr5, 2048 * 2));
EXPECT_EQ(a.getInternalAllocator().getNumAllocatedMemoryBlocks(), 3);
EXPECT_EQ(a.derived().getNumAllocatedMemoryBlocks(), 3);

EXPECT_FALSE(a.isEmpty());

Expand Down

0 comments on commit 826198f

Please sign in to comment.