Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a ObjectPoolIterator::Type class to allow begin/end iteration on active objects in memory pools. #32126

Merged
merged 32 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
917b36d
Remove PoolCommon: bad name, non-member method, very very limited usage
andy31415 Feb 14, 2024
2015058
Restyle
andy31415 Feb 14, 2024
ce4a175
Have a working iterator (and unit test) for static bitmap iterator
andy31415 Feb 14, 2024
5933dc8
Start unit testing for nested loops
andy31415 Feb 14, 2024
dd0f3e5
Fix tests and implementation
andy31415 Feb 14, 2024
fb27121
Added another test
andy31415 Feb 14, 2024
2c8a665
Extra unit test that we iterate correctly
andy31415 Feb 14, 2024
083dae7
Added comment about returning null on failure
andy31415 Feb 14, 2024
b16a172
Fix compile on fixed size pools
andy31415 Feb 14, 2024
c5885a8
Restyle
andy31415 Feb 14, 2024
d8525d6
Merge branch 'pool_updates' into pool_iterator
andy31415 Feb 14, 2024
fb28a44
Support iteration depth since it seems we need to delete objects duri…
andy31415 Feb 14, 2024
4143e7a
Update to also have a post-iteration clean when using iterators
andy31415 Feb 14, 2024
e6cd0f0
Make sure iterators on pools have a type that can be passed around
andy31415 Feb 14, 2024
317de87
Switch to not use a cast
andy31415 Feb 14, 2024
ddacb05
Fix types ... use auto because types are messy, unsure why
andy31415 Feb 14, 2024
c290fea
Rename for smaller diff
andy31415 Feb 14, 2024
34d4707
Rename for smaller diff
andy31415 Feb 14, 2024
1180ff9
Merge branch 'pool_updates' into pool_iterator
andy31415 Feb 14, 2024
fdbdf59
Correct the comment
andy31415 Feb 14, 2024
45c84bb
More comments cleanup
andy31415 Feb 14, 2024
86b15dc
Remove useless comment
andy31415 Feb 14, 2024
d9d6938
make things compile for NRF
andy31415 Feb 14, 2024
ad23745
Restyle
andy31415 Feb 14, 2024
884f7fb
Merge branch 'master' into pool_iterator
andy31415 Feb 15, 2024
d871d01
Merge branch 'master' into pool_iterator
andreilitvin Feb 22, 2024
7de5ad0
Update the comment
andreilitvin Feb 22, 2024
3a00a35
Merge branch 'master' into pool_iterator
andy31415 Feb 26, 2024
2400a76
Update allocated to active in naming
andy31415 Feb 26, 2024
75aff1b
Merge branch 'master' into pool_iterator
andy31415 Feb 26, 2024
ea10472
Add comment regarding iteration depth
andy31415 Feb 26, 2024
1b477aa
Undo submodule update
andy31415 Feb 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 60 additions & 14 deletions src/lib/support/Pool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,46 @@ Loop StaticAllocatorBitmap::ForEachActiveObjectInner(void * context, Lambda lamb
return Loop::Finish;
}

size_t StaticAllocatorBitmap::FirstAllocatedIndex()
{
size_t idx = 0;
for (size_t word = 0; word * kBitChunkSize < Capacity(); ++word)
{
auto & usage = mUsage[word];
auto value = usage.load(std::memory_order_relaxed);
for (size_t offset = 0; offset < kBitChunkSize && offset + word * kBitChunkSize < Capacity(); ++offset)
{
if ((value & (kBit1 << offset)) != 0)
{
return idx;
}
idx++;
}
}
VerifyOrDie(idx == mCapacity);
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
return mCapacity;
}

size_t StaticAllocatorBitmap::NextActiveIndexAfter(size_t start)
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
{
size_t idx = 0;
for (size_t word = 0; word * kBitChunkSize < Capacity(); ++word)
{
auto & usage = mUsage[word];
auto value = usage.load(std::memory_order_relaxed);
for (size_t offset = 0; offset < kBitChunkSize && offset + word * kBitChunkSize < Capacity(); ++offset)
{
if (((value & (kBit1 << offset)) != 0) && (start < idx))
{
return idx;
}
idx++;
}
}
VerifyOrDie(idx == mCapacity);
return mCapacity;
}

#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

HeapObjectListNode * HeapObjectList::FindNode(void * object) const
Expand Down Expand Up @@ -159,24 +199,30 @@ Loop HeapObjectList::ForEachNode(void * context, Lambda lambda)
p = p->mNext;
}
--mIterationDepth;
if (mIterationDepth == 0 && mHaveDeferredNodeRemovals)
CleanupDeferredReleases();
return result;
}

void HeapObjectList::CleanupDeferredReleases()
{
if (mIterationDepth != 0 || !mHaveDeferredNodeRemovals)
{
// Remove nodes for released objects.
p = mNext;
while (p != this)
return;
}
// Remove nodes for released objects.
HeapObjectListNode * p = mNext;
while (p != this)
{
HeapObjectListNode * next = p->mNext;
if (p->mObject == nullptr)
{
HeapObjectListNode * next = p->mNext;
if (p->mObject == nullptr)
{
p->Remove();
Platform::Delete(p);
}
p = next;
p->Remove();
Platform::Delete(p);
}

mHaveDeferredNodeRemovals = false;
p = next;
}
return result;

mHaveDeferredNodeRemovals = false;
}

#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
Expand Down
165 changes: 162 additions & 3 deletions src/lib/support/Pool.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@

namespace chip {

template <class T>
class BitmapActiveObjectIterator;

namespace internal {

class Statistics
Expand Down Expand Up @@ -91,6 +94,16 @@ class StaticAllocatorBitmap : public internal::StaticAllocatorBase
void * At(size_t index) { return static_cast<uint8_t *>(mElements) + mElementSize * index; }
size_t IndexOf(void * element);

/// Returns the first index that is allocated.
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
///
/// If nothing is allocated, this will return mCapacity
size_t FirstAllocatedIndex();

/// Returns the next active index after `start`.
///
/// If nothing else allocated, returns mCapacity
size_t NextActiveIndexAfter(size_t start);

using Lambda = Loop (*)(void * context, void * object);
Loop ForEachActiveObjectInner(void * context, Lambda lambda);
Loop ForEachActiveObjectInner(void * context, Loop lambda(void * context, const void * object)) const
Expand All @@ -102,6 +115,10 @@ class StaticAllocatorBitmap : public internal::StaticAllocatorBase
void * mElements;
const size_t mElementSize;
std::atomic<tBitChunkType> * mUsage;

/// allow accessing direct At() calls
template <class T>
friend class ::chip::BitmapActiveObjectIterator;
};

template <typename T, typename Function>
Expand Down Expand Up @@ -132,9 +149,9 @@ struct HeapObjectListNode
mPrev->mNext = mNext;
}

void * mObject;
HeapObjectListNode * mNext;
HeapObjectListNode * mPrev;
void * mObject = nullptr;
HeapObjectListNode * mNext = nullptr;
HeapObjectListNode * mPrev = nullptr;
};

struct HeapObjectList : HeapObjectListNode
Expand All @@ -158,6 +175,9 @@ struct HeapObjectList : HeapObjectListNode
return const_cast<HeapObjectList *>(this)->ForEachNode(context, reinterpret_cast<Lambda>(lambda));
}

/// Cleans up any deferred releases IFF iteration depth is 0
void CleanupDeferredReleases();

size_t mIterationDepth = 0;
bool mHaveDeferredNodeRemovals = false;
};
Expand All @@ -166,6 +186,46 @@ struct HeapObjectList : HeapObjectListNode

} // namespace internal

/// Provides iteration over active objects in a Bitmap pool.
///
/// Creating and releasing items within a pool does not invalidate
/// an iterator, however there are no guarantees which objects the
/// iterator will return (i.e. newly created objects while iterating
/// may be visible or not to the iterator depending where they are
/// allocated).
///
/// You are not prevented from releasing the object the iterator
/// currently points at. In that case, iterator should be advanced.
template <class T>
class BitmapActiveObjectIterator
tcarmelveilleux marked this conversation as resolved.
Show resolved Hide resolved
{
public:
using value_type = T;
using pointer = T *;
using reference = T &;

explicit BitmapActiveObjectIterator(internal::StaticAllocatorBitmap * pool, size_t idx) : mPool(pool), mIndex(idx) {}
BitmapActiveObjectIterator() {}
andy31415 marked this conversation as resolved.
Show resolved Hide resolved

bool operator==(const BitmapActiveObjectIterator & other) const
{
return (AtEnd() && other.AtEnd()) || ((mPool == other.mPool) && (mIndex == other.mIndex));
}
bool operator!=(const BitmapActiveObjectIterator & other) const { return !(*this == other); }
BitmapActiveObjectIterator & operator++()
{
mIndex = mPool->NextActiveIndexAfter(mIndex);
return *this;
}
T * operator*() const { return static_cast<T *>(mPool->At(mIndex)); }

private:
bool AtEnd() const { return (mPool == nullptr) || (mIndex >= mPool->Capacity()); }

internal::StaticAllocatorBitmap * mPool = nullptr; // pool that this belongs to
size_t mIndex = std::numeric_limits<size_t>::max();
};

/**
* @class ObjectPool
*
Expand Down Expand Up @@ -205,6 +265,9 @@ class BitMapObjectPool : public internal::StaticAllocatorBitmap
BitMapObjectPool() : StaticAllocatorBitmap(mData.mMemory, mUsage, N, sizeof(T)) {}
~BitMapObjectPool() { VerifyOrDie(Allocated() == 0); }

BitmapActiveObjectIterator<T> begin() { return BitmapActiveObjectIterator<T>(this, FirstAllocatedIndex()); }
BitmapActiveObjectIterator<T> end() { return BitmapActiveObjectIterator<T>(this, N); }

template <typename... Args>
T * CreateObject(Args &&... args)
{
Expand Down Expand Up @@ -331,6 +394,86 @@ class HeapObjectPool : public internal::Statistics, public HeapObjectPoolExitHan
#endif // __SANITIZE_ADDRESS__
}

/// Provides iteration over active objects in the pool.
///
/// NOTE: There is extra logic to allow objects release WHILE the iterator is
/// active while still allowing to advance the iterator.
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
/// This is done by flagging an iteration depth whenever an active
/// iterator exists. This also means that while a pool iterator exists, releasing
/// of tracking memory objects may be deferred until the last active iterator is
/// released.
class ActiveObjectIterator
{
public:
using value_type = T;
using pointer = T *;
using reference = T &;

ActiveObjectIterator() {}
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
ActiveObjectIterator(const ActiveObjectIterator & other) : mCurrent(other.mCurrent), mEnd(other.mEnd)
{
if (mEnd != nullptr)
{
mEnd->mIterationDepth++;
}
}

ActiveObjectIterator & operator=(const ActiveObjectIterator & other)
{
if (mEnd != nullptr)
{
mEnd->mIterationDepth--;
tcarmelveilleux marked this conversation as resolved.
Show resolved Hide resolved
mEnd->CleanupDeferredReleases();
}
mCurrent = other.mCurrent;
mEnd = other.mEnd;
mEnd->mIterationDepth++;
}

~ActiveObjectIterator()
{
if (mEnd != nullptr)
{
mEnd->mIterationDepth--;
mEnd->CleanupDeferredReleases();
}
}

bool operator==(const ActiveObjectIterator & other) const
{
// extra current/end compare is to have all "end iterators"
// compare as equal (in particular default active object iterator is the end
// of an iterator)
return (mCurrent == other.mCurrent) || ((mCurrent == mEnd) && (other.mCurrent == other.mEnd));
}
bool operator!=(const ActiveObjectIterator & other) const { return !(*this == other); }
ActiveObjectIterator & operator++()
{
do
{
mCurrent = mCurrent->mNext;
} while ((mCurrent != mEnd) && (mCurrent->mObject == nullptr));
return *this;
}
T * operator*() const { return static_cast<T *>(mCurrent->mObject); }

protected:
friend class HeapObjectPool<T>;

explicit ActiveObjectIterator(internal::HeapObjectListNode * current, internal::HeapObjectList * end) :
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
mCurrent(current), mEnd(end)
{
mEnd->mIterationDepth++;
}

private:
internal::HeapObjectListNode * mCurrent = nullptr;
internal::HeapObjectList * mEnd = nullptr;
};

ActiveObjectIterator begin() { return ActiveObjectIterator(mObjects.mNext, &mObjects); }
ActiveObjectIterator end() { return ActiveObjectIterator(&mObjects, &mObjects); }

template <typename... Args>
T * CreateObject(Args &&... args)
{
Expand Down Expand Up @@ -456,6 +599,15 @@ enum class ObjectPoolMem
#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
};

template <typename T, ObjectPoolMem P = ObjectPoolMem::kDefault>
struct ObjectPoolIterator;

template <typename T>
struct ObjectPoolIterator<T, ObjectPoolMem::kInline>
{
using Type = BitmapActiveObjectIterator<T>;
};

template <typename T, size_t N, ObjectPoolMem P = ObjectPoolMem::kDefault>
class ObjectPool;

Expand All @@ -465,6 +617,13 @@ class ObjectPool<T, N, ObjectPoolMem::kInline> : public BitMapObjectPool<T, N>
};

#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

template <typename T>
struct ObjectPoolIterator<T, ObjectPoolMem::kHeap>
{
using Type = typename HeapObjectPool<T>::ActiveObjectIterator;
};

template <typename T, size_t N>
class ObjectPool<T, N, ObjectPoolMem::kHeap> : public HeapObjectPool<T>
{
Expand Down
Loading
Loading