Global allocation bit #390

YiluoWei · 2021-07-28T03:32:02Z

Create new module util::alloc_bit.
Create new feature global_alloc_bit.
Create a new global side metadata for the alloc bit. 1 bit per min object size.
Add the alloc bit spec to plans.
Set alloc bit in initialize_object_metadata() for each policy.
Unset/zero bits for each policy.
Remove the alloc bit spec in MallocSpace, and change all the related functions to use the functions from the new alloc bit module.
issue:
Need to set the alloc bit in the allocation fastpath in OpenJDK, since we do not call post_alloc in the fastpath.
issue:
https://github.com/mmtk/mmtk-core/blob/master/src/policy/space.rs#L238

pub fn is_in_space(&self, object: ObjectReference) -> bool {
        let not_in_space = object.to_address().chunk_index() >= self.sft.len()
            || self.get(object.to_address()).name() == EMPTY_SPACE_SFT.name();

        if not_in_space {
            // special case - we do not yet have SFT entries for malloc space
            use crate::policy::mallocspace::is_alloced_by_malloc;
            is_alloced_by_malloc(object)
        } else {
            true
        }
    }

This function works for now, becuase we only use it to see whether an address is mapped, but it might be problematic.

qinsoon · 2021-07-28T04:08:54Z

Can you merge with master and resolve conflicts?

qinsoon · 2021-08-02T04:07:58Z

The build still fails. You can probably try run .github/scripts/ci-build.sh locally to figure out the issue. @YiluoWei

YiluoWei · 2021-08-02T06:18:44Z

This should fix the problem. I ran .github/scripts/ci-build.sh locally and it successfully built.

qinsoon · 2021-08-04T02:54:35Z

The JikesRVM tests still failed. I don't think it is the known issue as mmtk/mmtk-jikesrvm#78. Probably worth looking into that. The error message about 'not in RVM space' basically means it is possibly a segfault in Rust code. @YiluoWei

YiluoWei · 2021-08-04T05:49:27Z

yiluow@rat:~/mmtk-dev/mmtk-jikesrvm/repos/jikesrvm$ export RUST_BACKTRACE=1; ./dist/RBaseBaseMarkSweep_x86_64_m32-linux/rvm -X:gc:threads=16 -Xms75M -Xmx75M -jar /usr/share/benchmarks/dacapo/dacapo-2006-10-MR2.jar bloat
[2021-08-04T05:34:46Z INFO  mmtk::memory_manager] Initialized MMTk with MarkSweep
thread '<unnamed>' panicked at '0x2b400000 of size 4096 is not mapped', /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/util/memory.rs:121:18
stack backtrace:
   0: rust_begin_unwind
             at /rustc/952fdf2a1119affa1b37bcacb0c49cf9f0168ac8/library/std/src/panicking.rs:515:5
   1: std::panicking::begin_panic_fmt
             at /rustc/952fdf2a1119affa1b37bcacb0c49cf9f0168ac8/library/std/src/panicking.rs:457:5
   2: mmtk::util::memory::panic_if_unmapped
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/util/memory.rs:121:18
   3: mmtk::util::metadata::side_metadata::global::ensure_metadata_is_mapped
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/util/metadata/side_metadata/global.rs:325:5
   4: mmtk::util::metadata::side_metadata::global::store_atomic
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/util/metadata/side_metadata/global.rs:377:9
   5: mmtk::policy::mallocspace::metadata::set_page_mark
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/policy/mallocspace/metadata.rs:204:5
   6: <mmtk::policy::mallocspace::global::MallocSpace<VM> as mmtk::policy::space::SFT>::initialize_object_metadata
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/policy/mallocspace/global.rs:77:9
   7: <mmtk::plan::mutator_context::Mutator<VM> as mmtk::plan::mutator_context::MutatorContext<VM>>::post_alloc
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/plan/mutator_context.rs:75:9
   8: mmtk::memory_manager::post_alloc
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/memory_manager.rs:147:5
   9: post_alloc
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/mmtk/src/api.rs:80:5
  10: <unknown>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
fatal runtime error: failed to initiate panic, error 5
Aborted (core dumped)

Seems that sometimes, the (probably the first) page_mark metadata is not mapped. On rat, it has an around 50% chance to pass.

yiluow@rat:~/mmtk-dev/mmtk-jikesrvm/repos/jikesrvm$ export RUST_BACKTRACE=1; ./dist/RFastAdaptiveMarkSweep_x86_64_m32-linux/rvm -X:gc:threads=16 -Xms75M -Xmx75M -jar /usr/share/benchmarks/dacapo/dacapo-2006-10-MR2.jar bloat
===== DaCapo bloat starting =====
Optimized with: EDU.purdue.cs.bloat.optimize.Main -only EDU.purdue.cs.bloat.trans -pre -dce -diva -prop -stack-alloc -peel-loops all -f EDU.purdue.cs.bloat.trans.CompactArrayInitializer ./scratch/optimizedcode
===== DaCapo bloat PASSED in 15909 msec =====
yiluow@rat:~/mmtk-dev/mmtk-jikesrvm/repos/jikesrvm$ export RUST_BACKTRACE=1; ./dist/RFastAdaptiveMarkSweep_x86_64_m32-linux/rvm -X:gc:threads=16 -Xms75M -Xmx75M -jar /usr/share/benchmarks/dacapo/dacapo-2006-10-MR2.jar bloat
===== DaCapo bloat starting =====
Optimized with: EDU.purdue.cs.bloat.optimize.Main -only EDU.purdue.cs.bloat.trans -pre -dce -diva -prop -stack-alloc -peel-loops all -f EDU.purdue.cs.bloat.trans.CompactArrayInitializer ./scratch/optimizedcode
===== DaCapo bloat PASSED in 14435 msec =====

qinsoon · 2021-08-04T05:51:22Z

yiluow@rat:~/mmtk-dev/mmtk-jikesrvm/repos/jikesrvm$ export RUST_BACKTRACE=1; ./dist/RBaseBaseMarkSweep_x86_64_m32-linux/rvm -X:gc:threads=16 -Xms75M -Xmx75M -jar /usr/share/benchmarks/dacapo/dacapo-2006-10-MR2.jar bloat
[2021-08-04T05:34:46Z INFO  mmtk::memory_manager] Initialized MMTk with MarkSweep
thread '<unnamed>' panicked at '0x2b400000 of size 4096 is not mapped', /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/util/memory.rs:121:18
stack backtrace:
   0: rust_begin_unwind
             at /rustc/952fdf2a1119affa1b37bcacb0c49cf9f0168ac8/library/std/src/panicking.rs:515:5
   1: std::panicking::begin_panic_fmt
             at /rustc/952fdf2a1119affa1b37bcacb0c49cf9f0168ac8/library/std/src/panicking.rs:457:5
   2: mmtk::util::memory::panic_if_unmapped
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/util/memory.rs:121:18
   3: mmtk::util::metadata::side_metadata::global::ensure_metadata_is_mapped
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/util/metadata/side_metadata/global.rs:325:5
   4: mmtk::util::metadata::side_metadata::global::store_atomic
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/util/metadata/side_metadata/global.rs:377:9
   5: mmtk::policy::mallocspace::metadata::set_page_mark
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/policy/mallocspace/metadata.rs:204:5
   6: <mmtk::policy::mallocspace::global::MallocSpace<VM> as mmtk::policy::space::SFT>::initialize_object_metadata
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/policy/mallocspace/global.rs:77:9
   7: <mmtk::plan::mutator_context::Mutator<VM> as mmtk::plan::mutator_context::MutatorContext<VM>>::post_alloc
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/plan/mutator_context.rs:75:9
   8: mmtk::memory_manager::post_alloc
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/repos/mmtk-core/src/memory_manager.rs:147:5
   9: post_alloc
             at /home/yiluow/mmtk-dev/mmtk-jikesrvm/mmtk/src/api.rs:80:5
  10: <unknown>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
fatal runtime error: failed to initiate panic, error 5
Aborted (core dumped)

Seems that sometimes, the (probably the first) page_mark metadata is not mapped. On rat, it has an around 50% chance to pass.

yiluow@rat:~/mmtk-dev/mmtk-jikesrvm/repos/jikesrvm$ export RUST_BACKTRACE=1; ./dist/RFastAdaptiveMarkSweep_x86_64_m32-linux/rvm -X:gc:threads=16 -Xms75M -Xmx75M -jar /usr/share/benchmarks/dacapo/dacapo-2006-10-MR2.jar bloat
===== DaCapo bloat starting =====
Optimized with: EDU.purdue.cs.bloat.optimize.Main -only EDU.purdue.cs.bloat.trans -pre -dce -diva -prop -stack-alloc -peel-loops all -f EDU.purdue.cs.bloat.trans.CompactArrayInitializer ./scratch/optimizedcode
===== DaCapo bloat PASSED in 15909 msec =====
yiluow@rat:~/mmtk-dev/mmtk-jikesrvm/repos/jikesrvm$ export RUST_BACKTRACE=1; ./dist/RFastAdaptiveMarkSweep_x86_64_m32-linux/rvm -X:gc:threads=16 -Xms75M -Xmx75M -jar /usr/share/benchmarks/dacapo/dacapo-2006-10-MR2.jar bloat
===== DaCapo bloat starting =====
Optimized with: EDU.purdue.cs.bloat.optimize.Main -only EDU.purdue.cs.bloat.trans -pre -dce -diva -prop -stack-alloc -peel-loops all -f EDU.purdue.cs.bloat.trans.CompactArrayInitializer ./scratch/optimizedcode
===== DaCapo bloat PASSED in 14435 msec =====

I see. Then it is possible that this is the same issue as mmtk/mmtk-jikesrvm#78.

qinsoon

There are two things that are missing.

The alloc bit is not implemented for the Immix policy, which was added recently. The alloc bit is not implemented for LockFreeImmortalSpace as well.
There is no test/assertion to check if the alloc bit is implemented correctly. This is why 1 would happen. As we are not using the alloc bit right now (except marksweep and 'malloc space'), testing it directly would not be easy. But it is possible to add assertions to check alloc bit (guarded by extreme_assertions).

I would suggest adding some assertions to check if alloc bit is correct. These are just some examples:

before we set alloc bit for an object, the bit should be 0 before we set it (add the assertion to set_alloc_bit()).
before we unset alloc bit for an object, the bit should be 1 before we unset it (add the assertion to unset_alloc_bit()).
in trace_object() for each policy, if global_alloc_bit is enabled, the bit should be 1 before tracing.

Then I would suggest add alloc bit implementation for the immix space and the lockfree immortal space.

qinsoon · 2021-08-15T22:59:56Z

src/policy/copyspace.rs

@@ -38,7 +40,11 @@ impl<VM: VMBinding> SFT for CopySpace<VM> {
    fn is_sane(&self) -> bool {
        !self.from_space()
    }
-    fn initialize_object_metadata(&self, _object: ObjectReference, _alloc: bool) {}
+    fn initialize_object_metadata(&self, _object: ObjectReference, _alloc: bool) {
+        // This is only effective in slow path. We do not call post_alloc in fast path


I don't understand this line of comment. Did you mean for OpenJDK, we do not call post_alloc in fastpath? If so, I feel this is irrelevant to mmtk-core and this line should be removed.

qinsoon · 2021-08-15T23:28:23Z

src/plan/marksweep/global.rs

@@ -136,8 +138,14 @@ impl<VM: VMBinding> MarkSweep<VM> {
        options: Arc<UnsafeOptionsWrapper>,
    ) -> Self {
        let heap = HeapMeta::new(HEAP_START, HEAP_END);
+        #[cfg(feature = "global_alloc_bit")]


Some comments are needed here because this looks counter-intuitive. Normally people will expect that, with global_alloc_bit, we need the alloc bit spec. But we are not adding the alloc bit spec for global_alloc_bit, as in that case, the spec is already added to the global specs in new_global_specs().

qinsoon · 2021-08-15T23:29:27Z

src/policy/mallocspace/global.rs

@@ -172,7 +172,7 @@ impl<VM: VMBinding> MallocSpace<VM> {
            metadata: SideMetadataContext {
                global: global_side_metadata_specs,
                local: metadata::extract_side_metadata(&[
-                    MetadataSpec::OnSide(ALLOC_SIDE_METADATA_SPEC),
+                    // MetadataSpec::OnSide(ALLOC_SIDE_METADATA_SPEC),


Remove this instead of commenting it out.

qinsoon · 2021-08-15T23:30:06Z

src/policy/mallocspace/metadata.rs

-    log_num_of_bits: 0,
-    log_min_obj_size: constants::LOG_MIN_OBJECT_SIZE as usize,
-};
+// pub(crate) const ALLOC_SIDE_METADATA_SPEC: SideMetadataSpec = SideMetadataSpec {


Remove this instead of commenting it out.

qinsoon · 2021-08-15T23:31:52Z

src/policy/mallocspace/metadata.rs

@@ -153,11 +153,11 @@ pub fn is_alloced(object: ObjectReference) -> bool {
 }

 pub fn is_alloced_object(address: Address) -> bool {


I feel most of these functions can be removed. You can directly call to alloc_bit::...

qinsoon · 2021-08-15T23:32:53Z

src/util/metadata/side_metadata/constants.rs

@@ -106,6 +113,7 @@ pub const GLOBAL_SIDE_METADATA_VM_BASE_OFFSET: SideMetadataOffset =
 //      - Offset after Immix block mark byte
 //  6 - Immix chumk-map mark byte:
 //      - Offset after Immix chumk-map mark byte
+//  1 - MarkSweep Active Page byte


This seems to be a merging problem.

qinsoon · 2021-08-15T23:44:21Z

src/util/alloc_bit.rs

+    log_num_of_bits: 0,
+    log_min_obj_size: constants::LOG_MIN_OBJECT_SIZE as usize,
+};
+


Need comments for the functions below. For unsafe functions, we also need comments to explain why they are unsafe, and how to correctly call them.

…llocbit

qinsoon · 2021-08-19T00:20:05Z

Is there any other change you would like to add to this PR? Otherwise, I think we can merge it. @YiluoWei

YiluoWei · 2021-08-19T06:18:37Z

Is there any other change you would like to add to this PR? Otherwise, I think we can merge it. @YiluoWei

I think we can merge it.

Yiluo Wei added 7 commits July 22, 2021 17:31

add alloc_bit module

e66413c

make alloc_bit global

a973d85

add global_alloc_bit feature

ea9258f

set alloc bit

a6b7331

unset alloc bit

f472a3d

fix copyspace unset alloc bit

5ccff85

clean

da2a137

Yiluo Wei added 3 commits August 1, 2021 16:14

reset alloc bit in copyspace

298da1d

merge master resolve conflict

46fd071

fix metadata offset in 32-bit

9c1402e

fix metadata offset in 32-bit

703e50b

Yiluo Wei added 2 commits August 3, 2021 01:18

clean warnings

95a7c7b

format

028f441

qinsoon added the PR-testing Run binding tests for the pull request (deprecated: use PR-extended-testing instead) label Aug 3, 2021

Yiluo Wei added 2 commits August 12, 2021 20:31

debug

09efea3

merge master

1b31e9d

YiluoWei mentioned this pull request Aug 13, 2021

Fix a random local side metadata not mapped bug #411

Merged

Merge branch 'master' into dev-allocbit

77113bc

qinsoon requested changes Aug 16, 2021

View reviewed changes

Yiluo Wei added 4 commits August 17, 2021 19:33

Merge branch 'master' of https://github.com/mmtk/mmtk-core into dev-a…

74702b6

…llocbit

better comment

80d4202

update

3261fcc

add assertions;support Immix

bf3e038

qinsoon approved these changes Aug 19, 2021

View reviewed changes

qinsoon merged commit d2324cf into mmtk:master Aug 19, 2021

qinsoon mentioned this pull request Oct 17, 2021

Efficient allocation bits #251

Closed

qinsoon mentioned this pull request Jun 21, 2023

Fix VO bits for Immix #849

Merged

5 tasks

		@@ -153,11 +153,11 @@ pub fn is_alloced(object: ObjectReference) -> bool {
		}

		pub fn is_alloced_object(address: Address) -> bool {

Global allocation bit #390

Global allocation bit #390

Uh oh!

Conversation

YiluoWei commented Jul 28, 2021

Uh oh!

qinsoon commented Jul 28, 2021

Uh oh!

qinsoon commented Aug 2, 2021

Uh oh!

YiluoWei commented Aug 2, 2021

Uh oh!

qinsoon commented Aug 4, 2021

Uh oh!

YiluoWei commented Aug 4, 2021

Uh oh!

qinsoon commented Aug 4, 2021

Uh oh!

qinsoon left a comment

Choose a reason for hiding this comment

Uh oh!

qinsoon Aug 15, 2021

Choose a reason for hiding this comment

Uh oh!

qinsoon Aug 15, 2021

Choose a reason for hiding this comment

Uh oh!

qinsoon Aug 15, 2021

Choose a reason for hiding this comment

Uh oh!

qinsoon Aug 15, 2021

Choose a reason for hiding this comment

Uh oh!

qinsoon Aug 15, 2021

Choose a reason for hiding this comment

Uh oh!

qinsoon Aug 15, 2021

Choose a reason for hiding this comment

Uh oh!

qinsoon Aug 15, 2021

Choose a reason for hiding this comment

Uh oh!

qinsoon commented Aug 19, 2021

Uh oh!

YiluoWei commented Aug 19, 2021

Uh oh!

Uh oh!