Skip to content

[codex] Add container pool hysteresis #11850

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.

Sign up for GitHub

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

Merged
saipraveenb25 merged 6 commits into from
Jul 2, 2026
+308 −6
Merged

[codex] Add container pool hysteresis #11850

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
71be00a
Add hash-set pool hysteresis
saipraveenb25 Jun 30, 2026
4b48e68
Address hash-set pool review comments
saipraveenb25 Jun 30, 2026
07de672
Fix formatting
saipraveenb25 Jul 1, 2026
9707a00
Merge branch 'master' into codex/hash-set-pool-hysteresis
saipraveenb25 Jul 1, 2026
09aba7a
Add dictionary pool hysteresis
saipraveenb25 Jul 1, 2026
c06f066
Merge branch 'master' into codex/hash-set-pool-hysteresis
saipraveenb25 Jul 1, 2026
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
16 changes: 15 additions & 1 deletion source/core/slang-dictionary.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,18 @@ class Dictionary
//

// Removes all values from the map
void clear() { map.clear(); }
void clear()
{
if (!map.empty())
map.clear();
}

// Removes all values and releases backing storage.
void clearAndDeallocate()
{
InnerMap emptyMap(0, map.hash_function(), map.key_eq(), map.get_allocator());
map.swap(emptyMap);
}

// Erases the value at the specified key if it exists
void remove(const TKey& key) { map.erase(key); }
Expand Down Expand Up @@ -178,6 +189,7 @@ class Dictionary
//

std::size_t getCount() const { return map.size(); }
std::size_t getBucketCount() const { return map.bucket_count(); }

//
// Lookup
Expand Down Expand Up @@ -398,7 +410,9 @@ class HashSetBase

public:
auto getCount() const { return dict.getCount(); }
auto getBucketCount() const { return dict.getBucketCount(); }
void clear() { dict.clear(); }
void clearAndDeallocate() { dict.clearAndDeallocate(); }
bool add(const T& obj) { return dict.addIfNotExists(obj, _DummyClass()); }
bool add(T&& obj) { return dict.addIfNotExists(_Move(obj), _DummyClass()); }
void remove(const T& obj) { dict.remove(obj); }
Expand Down
64 changes: 61 additions & 3 deletions source/slang/slang-container-pool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
namespace Slang
{
static const int kContainerPoolSize = 1024;
static const size_t kContainerPoolHashSetMinRetireBucketCount = 4096;
static const size_t kContainerPoolHashSetRetireUnderuseDivisor = 8;
static const int kContainerPoolHashSetRetireUnderuseCount = 2;

template<typename T>
struct ObjectPool
Expand All @@ -29,9 +32,16 @@ struct ObjectPool
return &m_objects[id];
}

void freeObject(T* object)
int getObjectIndex(T* object)
{
auto id = (int)(object - m_objects.getBuffer());
SLANG_RELEASE_ASSERT(id >= 0 && id < m_objects.getCount());
return id;
}

void freeObject(T* object)
{
auto id = getObjectIndex(object);
m_pool.free(id, 1);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

Expand All @@ -44,12 +54,16 @@ struct ContainerPool
ObjectPool<List<void*>> m_listPool;
ObjectPool<Dictionary<void*, void*>> m_dictionaryPool;
ObjectPool<HashSet<void*>> m_hashSetPool;
List<int> m_hashSetUnderuseStreaks;

ContainerPool()
: m_listPool(kContainerPoolSize)
, m_dictionaryPool(kContainerPoolSize)
, m_hashSetPool(kContainerPoolSize)
{
m_hashSetUnderuseStreaks.setCount(kContainerPoolSize);
for (Index i = 0; i < kContainerPoolSize; i++)
m_hashSetUnderuseStreaks[i] = 0;
}

template<typename T>
Expand All @@ -70,6 +84,34 @@ struct ContainerPool
return (HashSet<T*>*)m_hashSetPool.getObject();
}

bool updateHashSetUnderuseStreakAndShouldRetire(
int objectIndex,
size_t liveCount,
size_t bucketCount)
{
// Hash sets normally keep their buckets after `clear()` so the next large use can reuse
// that storage. On long runs, though, a pool slot can become expensive if one large use is
// followed by many tiny uses: every return clears the same large table even though the live
// count stays small. Track that pattern per slot and only release the buckets after it
// repeats, so an occasional small use after a large pass does not cause allocation churn.
bool isUnderused = bucketCount >= kContainerPoolHashSetMinRetireBucketCount &&
liveCount <= bucketCount / kContainerPoolHashSetRetireUnderuseDivisor;
if (!isUnderused)
{
m_hashSetUnderuseStreaks[objectIndex] = 0;
return false;
}

if (m_hashSetUnderuseStreaks[objectIndex] < kContainerPoolHashSetRetireUnderuseCount)
m_hashSetUnderuseStreaks[objectIndex]++;

if (m_hashSetUnderuseStreaks[objectIndex] < kContainerPoolHashSetRetireUnderuseCount)
return false;

m_hashSetUnderuseStreaks[objectIndex] = 0;
return true;
}

template<typename T>
void free(List<T*>* list)
{
Expand All @@ -87,8 +129,24 @@ struct ContainerPool
template<typename T>
void free(HashSet<T*>* set)
{
set->clear();
m_hashSetPool.freeObject((HashSet<void*>*)set);
auto pooledSet = (HashSet<void*>*)set;
auto objectIndex = m_hashSetPool.getObjectIndex(pooledSet);
auto liveCount = set->getCount();
auto bucketCount = set->getBucketCount();
bool shouldRetire =
updateHashSetUnderuseStreakAndShouldRetire(objectIndex, liveCount, bucketCount);

if (shouldRetire)
{
// Swap with an empty set instead of clearing first, because the oversized bucket array
// is exactly the cost we are trying to stop paying on later small uses of this slot.
set->clearAndDeallocate();
}
else
{
set->clear();
}
m_hashSetPool.freeObject(pooledSet);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
};
} // namespace Slang
Expand Down
106 changes: 106 additions & 0 deletions tools/slang-unit-test/unit-test-container-pool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// unit-test-container-pool.cpp

#include "core/slang-list.h"
#include "slang/slang-container-pool.h"
#include "unit-test/slang-unit-test.h"

using namespace Slang;

static void _fillPointerSet(HashSet<int*>* set, List<int>& values, Index count)
{
values.setCount(count);
for (Index i = 0; i < count; i++)
{
values[i] = int(i);
set->add(&values[i]);
}
}

static size_t _growAndReturnLargeHashSet(ContainerPool& pool, List<int>& values)
{
auto set = pool.getHashSet<int>();
_fillPointerSet(set, values, Index(kContainerPoolHashSetMinRetireBucketCount));

auto bucketCount = set->getBucketCount();
SLANG_CHECK(bucketCount >= kContainerPoolHashSetMinRetireBucketCount);
pool.free(set);
return bucketCount;
}

SLANG_UNIT_TEST(containerPoolHashSetClearAndDeallocate)
{
HashSet<int> set;
for (Index i = 0; i < Index(kContainerPoolHashSetMinRetireBucketCount); i++)
set.add(int(i));

auto largeBucketCount = set.getBucketCount();
SLANG_CHECK(largeBucketCount >= kContainerPoolHashSetMinRetireBucketCount);

set.clear();
SLANG_CHECK(set.getCount() == 0);
SLANG_CHECK(set.getBucketCount() == largeBucketCount);

set.add(0);
set.clearAndDeallocate();
SLANG_CHECK(set.getCount() == 0);
SLANG_CHECK(set.getBucketCount() < largeBucketCount);
SLANG_CHECK(set.getBucketCount() < kContainerPoolHashSetMinRetireBucketCount);
}

SLANG_UNIT_TEST(containerPoolHashSetHysteresisRetiresAfterSecondUnderuse)
{
ContainerPool pool;
List<int> values;

auto largeBucketCount = _growAndReturnLargeHashSet(pool, values);

auto set = pool.getHashSet<int>();

@github-actions github-actions Bot Jul 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Gap: No boundary coverage for the two hysteresis thresholds

The retirement rule from slang-container-pool.h is bucketCount >= kContainerPoolMinRetireBucketCount && liveCount <= bucketCount / kContainerPoolRetireUnderuseDivisor, with constants 4096 and 8. All six new tests exercise well-inside-threshold inputs (_fillPointerSet(..., 1) for underuse; bucketCount / divisor + 1 for the reset-triggering "substantial" use). An off-by-one drift on either comparison (> vs >=, < vs <=) would not fail any test — a pool slot with bucketCount == 4096 and liveCount == 512 (the exact bucketCount/8 boundary) is precisely the case the constants were tuned to catch, and the tests do not pin that behavior.

Example: If a future refactor changes bucketCount >= 4096 to bucketCount > 4096, or liveCount <= to liveCount <, sets that live right on the boundary silently stop retiring. The stated 15% Falcor speedup could regress without a test failure.

Suggestion: Add a boundary test that verifies (a) bucketCount == kContainerPoolMinRetireBucketCount with liveCount == bucketCount / kContainerPoolRetireUnderuseDivisor retires on the second underused return, and (b) liveCount == bucketCount / kContainerPoolRetireUnderuseDivisor + 1 (one over) never retires no matter how many returns.

SLANG_CHECK(set->getBucketCount() == largeBucketCount);
_fillPointerSet(set, values, 1);
pool.free(set);

set = pool.getHashSet<int>();
SLANG_CHECK(set->getBucketCount() == largeBucketCount);
_fillPointerSet(set, values, 1);
pool.free(set);

set = pool.getHashSet<int>();
SLANG_CHECK(set->getBucketCount() < largeBucketCount);
SLANG_CHECK(set->getBucketCount() < kContainerPoolHashSetMinRetireBucketCount);
pool.free(set);
}

SLANG_UNIT_TEST(containerPoolHashSetHysteresisResetsAfterSubstantialUse)
{
ContainerPool pool;
List<int> values;

auto largeBucketCount = _growAndReturnLargeHashSet(pool, values);

auto set = pool.getHashSet<int>();
SLANG_CHECK(set->getBucketCount() == largeBucketCount);
_fillPointerSet(set, values, 1);
pool.free(set);

set = pool.getHashSet<int>();
SLANG_CHECK(set->getBucketCount() == largeBucketCount);
auto substantialUseCount =
Index(largeBucketCount / kContainerPoolHashSetRetireUnderuseDivisor + 1);
_fillPointerSet(set, values, substantialUseCount);
pool.free(set);

set = pool.getHashSet<int>();
SLANG_CHECK(set->getBucketCount() == largeBucketCount);
_fillPointerSet(set, values, 1);
pool.free(set);

set = pool.getHashSet<int>();
SLANG_CHECK(set->getBucketCount() == largeBucketCount);
_fillPointerSet(set, values, 1);
pool.free(set);

set = pool.getHashSet<int>();
SLANG_CHECK(set->getBucketCount() < largeBucketCount);
SLANG_CHECK(set->getBucketCount() < kContainerPoolHashSetMinRetireBucketCount);
pool.free(set);
}
Loading