implement a feature to allocate an extra word ahead of each individua…

[tianleq]

It does not support malloc marksweep

[qinsoon]

The PR generally looks okay, but one pity is that the code/mechanism is not reused for mark compact to add the policy-specific extra header word. I suggest ask @wks to review this PR once it is ready.

[qinsoon]

Better to clarify that VM::EXTRA_HEADER_BYTES will control the exact size of extra header, and there is no control on the alignment of the extra header. We just guarantee there are EXTRA_HEADER_BYTES before the address we return. It could be unaligned access if the binding attempts to write pointers to the extra bytes.

[qinsoon]

Is this feature only intended for debugging, or can be used in a performance setting?

[tianleq]

I guess it is only intended for debugging.

[qinsoon]

The binding can config EXTRA_HEADER_BYTES, and there is no requirement on what value a binding can use. If a binding uses a value that is not a multiple of the fixed alignment, this code is wrong. You can either introduce some requirements for a 'valid EXTRA_HEADER_BYTES value', or simply change the condition in L264 so we use native_brute_force_sweep if extra_header is enabled.

[tianleq]

The PR generally looks okay, but one pity is that the code/mechanism is not reused for mark compact to add the policy-specific extra header word. I suggest ask @wks to review this PR once it is ready.
Steve has pointed out that markcompact only affects objects in markcompactspace but this one is a global setting, it affects all objects

[wks]

@tianleq Can you remind me what was the purpose of this feature? I see the size of the extra word is specified in VMBinding. This means it is controlled by the VM. But if the VM wants an additional word before every object, it can implement it in its own function that calculates the object size. For example, currently the Ruby binding puts an extra word before every object to record some extra information. I implemented this in the Ruby-side C function newobj_of0 which allocates Ruby objects, so that when calling mmtk_alloc, the passed object size is always one word larger than the original Ruby object size. So I wonder why this feature is necessary.

If the extra word is required by the MMTk for debug purposes, it shouldn't be set in VMBinding, but somewhere in the MMTk core. If it is plan-specific or space-specific, it should be specified in the respective plan or space.

[tianleq]

@wks
It allows storing info in the extra header, for various reasons(primarily debugging purpose). It is neither plan-specific nor policy-specific. It is more like a global setting. Once you enable the feature, all mmtk objects will have the extra header, regardless of the plan used.

[qinsoon]

The PR generally looks okay, but one pity is that the code/mechanism is not reused for mark compact to add the policy-specific extra header word. I suggest ask @wks to review this PR once it is ready.
Steve has pointed out that markcompact only affects objects in markcompactspace but this one is a global setting, it affects all objects

Right. But we could have some code that is used by both on how to compute a new result based on the extra bytes, and how to align the extra header size.

Computing result:

mmtk-core/src/util/alloc/markcompact_allocator.rs

Lines 55 to 61 in cacb8f6

	// Check if the result is valid and return the actual object start address
	// Note that `rtn` can be null in the case of OOM
	if !rtn.is_zero() {
	rtn + Self::HEADER_RESERVED_IN_BYTES
	} else {
	rtn
	}

mmtk-core/src/util/alloc/bumpallocator.rs

Lines 73 to 79 in f1359c5

	// Check if the result is valid and return the actual object start address
	// Note that `rtn` can be null in the case of OOM
	if !rtn.is_zero() {
	rtn + VM::EXTRA_HEADER_BYTES
	} else {
	rtn
	}

Computing header size:

mmtk-core/src/policy/markcompactspace.rs

Lines 151 to 158 in cacb8f6

	/// We need one extra header word for each object. Considering the alignment requirement, this is
	/// the actual bytes we need to reserve for each allocation.
	pub const HEADER_RESERVED_IN_BYTES: usize = if VM::MAX_ALIGNMENT > GC_EXTRA_HEADER_BYTES {
	VM::MAX_ALIGNMENT
	} else {
	GC_EXTRA_HEADER_BYTES
	}
	.next_power_of_two();

mmtk-core/src/vm/mod.rs

Lines 75 to 82 in f1359c5

	#[cfg(feature = "extra_header")]
	const EXTRA_HEADER_BYTES: usize =
	if Self::MAX_ALIGNMENT > crate::util::constants::BYTES_IN_WORD {
	Self::MAX_ALIGNMENT
	} else {
	crate::util::constants::BYTES_IN_WORD
	}
	.next_power_of_two();