mmtk / mmtk-core

Memory Management ToolKit
https://www.mmtk.io
Other
374 stars 68 forks source link

Make Mutator struct smaller #1001

Open caizixian opened 11 months ago

caizixian commented 11 months ago

This can be addressed either in the core or the binding, but the current Mutator struct is too big, that the offsets in the TLS of the allocator fields we care about might not be able to be encoded efficiently on platforms like riscv64.

My current hack is to reduce the number of allocators to be enough to run the OpenJDK binding. But perhaps we can also put some allocator information only needed for the slow path behind an indirection.

889 is not sufficient to address this problem, because it just changes how the offsets of fields are exposed, but the Mutator struct remains the same.

diff --git a/src/util/alloc/allocators.rs b/src/util/alloc/allocators.rs
index 209cd5b33..2c3b83b16 100644
--- a/src/util/alloc/allocators.rs
+++ b/src/util/alloc/allocators.rs
@@ -14,12 +14,12 @@ use crate::vm::VMBinding;
 use super::FreeListAllocator;
 use super::MarkCompactAllocator;

-pub(crate) const MAX_BUMP_ALLOCATORS: usize = 6;
-pub(crate) const MAX_LARGE_OBJECT_ALLOCATORS: usize = 2;
-pub(crate) const MAX_MALLOC_ALLOCATORS: usize = 1;
+pub(crate) const MAX_BUMP_ALLOCATORS: usize = 3;
+pub(crate) const MAX_LARGE_OBJECT_ALLOCATORS: usize = 1;
+pub(crate) const MAX_MALLOC_ALLOCATORS: usize = 0;
 pub(crate) const MAX_IMMIX_ALLOCATORS: usize = 1;
 pub(crate) const MAX_FREE_LIST_ALLOCATORS: usize = 2;
-pub(crate) const MAX_MARK_COMPACT_ALLOCATORS: usize = 1;
+pub(crate) const MAX_MARK_COMPACT_ALLOCATORS: usize = 0;

 // The allocators set owned by each mutator. We provide a fixed number of allocators for each allocator type in the mutator,
 // and each plan will select part of the allocators to use.
wks commented 11 months ago

What about we only embed the BumpPointer in TLS, and synchronise it with the Allocator instance in the slow path? https://docs.mmtk.io/portingguide/perf_tuning/alloc.html#option-3-embed-the-fastpath-struct

caizixian commented 11 months ago

That's also fine. We should implement this in the OpenJDK binding.