Skip to content

Fix HXCPP_GC_BIG_BLOCKS row count regression #1409

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

Open
dimensionscape wants to merge 1 commit into
base: master
Choose a base branch
from
+13 −5
Open

Fix HXCPP_GC_BIG_BLOCKS row count regression #1409

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 13 additions & 5 deletions src/hx/gc/Immix.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <thread>
#include <condition_variable>
#include <inttypes.h>
#include <limits>

#ifdef __EMSCRIPTEN__
#include <emscripten/stack.h>
Expand Down Expand Up @@ -754,11 +755,11 @@ struct BlockDataInfo
HoleRange mRanges[MAX_HOLES];
uint8_t mHoles;

uint8_t mUsedRows;
uint16_t mUsedRows;
uint16_t mMaxHoleSize;
int mMoveScore;
uint16_t mUsedBytes;
uint8_t mFraggedRows;
uint16_t mFraggedRows;
bool mPinned;
unsigned char mZeroed;
bool mReclaimed;
Expand Down Expand Up @@ -1418,6 +1419,13 @@ struct BlockDataInfo
#endif
};

// A block's row counts run up to IMMIX_USEFUL_LINES: 254 with the default 32k blocks, but 508 with
// HXCPP_GC_BIG_BLOCKS' 64k blocks, more than a uint8_t holds.

@tobil4sk tobil4sk Oct 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment doesn't seem very useful to keep, since it refers to the uint8_t that is removed by this PR. I think the static assert is more helpful at documenting the requirement

@dimensionscape dimensionscape Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's fair. It was originally a warning against regression, but the static asserts keep us safe. I'll remove it.

static_assert(IMMIX_USEFUL_LINES <= std::numeric_limits<decltype(BlockDataInfo::mUsedRows)>::max(),
"BlockDataInfo::mUsedRows must hold IMMIX_USEFUL_LINES");
static_assert(IMMIX_USEFUL_LINES <= std::numeric_limits<decltype(BlockDataInfo::mFraggedRows)>::max(),
"BlockDataInfo::mFraggedRows must hold IMMIX_USEFUL_LINES");

static bool SmallestFreeFirst(BlockDataInfo *inA, BlockDataInfo *inB)
{
return inA->mMaxHoleSize < inB->mMaxHoleSize;
Expand Down Expand Up @@ -5768,7 +5776,7 @@ class LocalAllocator : public hx::StackContext
uint8_t mCurrentHole;
uint8_t mCurrentHoles;
HoleRange *mCurrentRange;
uint8_t *mFraggedRows;
uint16_t *mFraggedRows;

bool mMoreHoles;

Expand Down Expand Up @@ -6259,7 +6267,7 @@ class LocalAllocator : public hx::StackContext
// spaceOversize might have been set to zero for quick-termination of alloc.
unsigned char* s{ spaceOversize };
if (s>spaceFirst && mFraggedRows)
*mFraggedRows += static_cast<uint8_t>((s - spaceFirst) >> IMMIX_LINE_BITS);
*mFraggedRows += static_cast<uint16_t>((s - spaceFirst) >> IMMIX_LINE_BITS);
#else
#ifdef HXCPP_ALIGN_ALLOC
if (!(size_t{ spaceStart } & 0x4))
Expand Down Expand Up @@ -6296,7 +6304,7 @@ class LocalAllocator : public hx::StackContext
}
if (mFraggedRows && spaceEnd > spaceStart)
{
*mFraggedRows += static_cast<uint8_t>((spaceEnd - spaceStart) >> IMMIX_LINE_BITS);
*mFraggedRows += static_cast<uint16_t>((spaceEnd - spaceStart) >> IMMIX_LINE_BITS);
}
#endif

Expand Down
Loading