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

mycpp: add 128B pool #1958

Open
wants to merge 3 commits into
base: soil-staging
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
17 changes: 15 additions & 2 deletions mycpp/mark_sweep_heap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ void* MarkSweepHeap::Allocate(size_t num_bytes, int* obj_id, int* pool_id) {
*pool_id = 2;
return pool2_.Allocate(obj_id);
}
if (num_bytes <= pool3_.kMaxObjSize) {
*pool_id = 3;
return pool3_.Allocate(obj_id);
}
*pool_id = 0; // malloc(), not a pool
#endif

Expand Down Expand Up @@ -148,6 +152,11 @@ void MarkSweepHeap::MaybeMarkAndPush(RawObject* obj) {
return;
}
pool2_.Mark(obj_id);
} else if (header->pool_id == 3) {
if (pool3_.IsMarked(obj_id)) {
return;
}
pool3_.Mark(obj_id);
} else
#endif
{
Expand Down Expand Up @@ -215,6 +224,7 @@ void MarkSweepHeap::Sweep() {
#ifndef NO_POOL_ALLOC
pool1_.Sweep();
pool2_.Sweep();
pool3_.Sweep();
#endif

int last_live_index = 0;
Expand Down Expand Up @@ -263,6 +273,7 @@ int MarkSweepHeap::Collect() {
#ifndef NO_POOL_ALLOC
pool1_.PrepareForGc();
pool2_.PrepareForGc();
pool3_.PrepareForGc();
#endif

// Mark roots.
Expand Down Expand Up @@ -335,7 +346,7 @@ void MarkSweepHeap::PrintStats(int fd) {

#ifndef NO_POOL_ALLOC
dprintf(fd, " num allocated = %10d\n",
num_allocated_ + pool1_.num_allocated() + pool2_.num_allocated());
num_allocated_ + pool1_.num_allocated() + pool2_.num_allocated() + pool3_.num_allocated());
dprintf(fd, " num in heap = %10d\n", num_allocated_);
#else
dprintf(fd, " num allocated = %10d\n", num_allocated_);
Expand All @@ -344,9 +355,10 @@ void MarkSweepHeap::PrintStats(int fd) {
#ifndef NO_POOL_ALLOC
dprintf(fd, " num in pool 1 = %10d\n", pool1_.num_allocated());
dprintf(fd, " num in pool 2 = %10d\n", pool2_.num_allocated());
dprintf(fd, " num in pool 3 = %10d\n", pool3_.num_allocated());
dprintf(
fd, "bytes allocated = %10" PRId64 "\n",
bytes_allocated_ + pool1_.bytes_allocated() + pool2_.bytes_allocated());
bytes_allocated_ + pool1_.bytes_allocated() + pool2_.bytes_allocated() + pool3_.bytes_allocated());
#else
dprintf(fd, "bytes allocated = %10" PRId64 "\n", bytes_allocated_);
#endif
Expand Down Expand Up @@ -404,6 +416,7 @@ void MarkSweepHeap::FreeEverything() {
#ifndef NO_POOL_ALLOC
pool1_.Free();
pool2_.Free();
pool3_.Free();
#endif
}

Expand Down
4 changes: 3 additions & 1 deletion mycpp/mark_sweep_heap.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ class MarkSweepHeap {
int num_live() {
return num_live_
#ifndef NO_POOL_ALLOC
+ pool1_.num_live() + pool2_.num_live()
+ pool1_.num_live() + pool2_.num_live() + pool3_.num_live()
#endif
;
}
Expand Down Expand Up @@ -265,10 +265,12 @@ class MarkSweepHeap {
#ifndef NO_POOL_ALLOC
// 16,384 / 24 bytes = 682 cells (rounded), 16,368 bytes
// 16,384 / 48 bytes = 341 cells (rounded), 16,368 bytes
// 16,384 / 96 bytes = 171 cells (rounded), 16,368 bytes
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm this still says 96

I kinda want to do some evaluation, make sure it's not over-tuned for Linux / glibc / 64-bit

And does this waste memory on any workloads?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also I think we had the theory again that malloc() needed some slack, to be strictly less than 16,384 bytes, to allow for headers

But I guess we never measured that effect

We already run on many different libc with different malloc(), so yeah I think we can at least survey Alpine Linux

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh whoops. That's a typo leftover from a previous version. Will fix.

Happy to leave this open until we have a better idea of the effect with different allocators. I suspect that it will be a win in most cases, though. Since the write to the allocation header (whether it's below the buffer or elsewhere) is going to cause page faults (the reduction of which was the main win from this) regardless.

// Conveniently, the glibc malloc header is 16 bytes, giving exactly 16 Ki
// differences
Pool<682, 24> pool1_;
Pool<341, 48> pool2_;
Pool<128, 128> pool3_;
#endif

std::vector<RawObject**> roots_;
Expand Down
Loading