Skip to content

Commit 9dba22a

Browse files
qinsoonqinsoon
committed
Assert marksweep block list
1 parent 5ab62f9 commit 9dba22a

File tree

4 files changed

+138
-17
lines changed

4 files changed

+138
-17
lines changed

‎Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ immix_smaller_block = []
125125
# Zero the unmarked lines after a GC cycle in immix. This helps debug untraced objects.
126126
immix_zero_on_release = []
127127

128+
# Run sanity checks for BlockList in mark sweep.
129+
ms_block_list_sanity = []
130+
128131
# Run sanity GC
129132
sanity = []
130133
# Run analysis

‎src/policy/marksweepspace/native_ms/block.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -136,35 +136,35 @@ impl Block {
136136
.is_ok()
137137
}
138138

139-
pub fn load_prev_block(&self) -> Option<Block> {
139+
pub(in crate::policy::marksweepspace::native_ms) fn load_prev_block(&self) -> Option<Block> {
140140
let prev = unsafe { Block::PREV_BLOCK_TABLE.load::<usize>(self.start()) };
141141
NonZeroUsize::new(prev).map(Block)
142142
}
143143

144-
pub fn load_next_block(&self) -> Option<Block> {
144+
pub(in crate::policy::marksweepspace::native_ms) fn load_next_block(&self) -> Option<Block> {
145145
let next = unsafe { Block::NEXT_BLOCK_TABLE.load::<usize>(self.start()) };
146146
NonZeroUsize::new(next).map(Block)
147147
}
148148

149-
pub fn store_next_block(&self, next: Block) {
149+
pub(in crate::policy::marksweepspace::native_ms) fn store_next_block(&self, next: Block) {
150150
unsafe {
151151
Block::NEXT_BLOCK_TABLE.store::<usize>(self.start(), next.start().as_usize());
152152
}
153153
}
154154

155-
pub fn clear_next_block(&self) {
155+
pub(in crate::policy::marksweepspace::native_ms) fn clear_next_block(&self) {
156156
unsafe {
157157
Block::NEXT_BLOCK_TABLE.store::<usize>(self.start(), 0);
158158
}
159159
}
160160

161-
pub fn store_prev_block(&self, prev: Block) {
161+
pub(in crate::policy::marksweepspace::native_ms) fn store_prev_block(&self, prev: Block) {
162162
unsafe {
163163
Block::PREV_BLOCK_TABLE.store::<usize>(self.start(), prev.start().as_usize());
164164
}
165165
}
166166

167-
pub fn clear_prev_block(&self) {
167+
pub(in crate::policy::marksweepspace::native_ms) fn clear_prev_block(&self) {
168168
unsafe {
169169
Block::PREV_BLOCK_TABLE.store::<usize>(self.start(), 0);
170170
}

‎src/policy/marksweepspace/native_ms/block_list.rs

Lines changed: 125 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,23 @@ use crate::util::linear_scan::Region;
44
use crate::vm::VMBinding;
55
use std::sync::atomic::AtomicBool;
66
use std::sync::atomic::Ordering;
7+
#[cfg(feature = "ms_block_list_sanity")]
8+
use std::sync::Mutex;
79

810
/// List of blocks owned by the allocator
911
#[repr(C)]
1012
pub struct BlockList {
11-
pub first: Option<Block>,
12-
pub last: Option<Block>,
13-
pub size: usize,
14-
pub lock: AtomicBool,
13+
first: Option<Block>,
14+
last: Option<Block>,
15+
size: usize,
16+
lock: AtomicBool,
17+
#[cfg(feature = "ms_block_list_sanity")]
18+
sanity_list: Mutex<Vec<Block>>,
1519
}
1620

1721
impl std::fmt::Debug for BlockList {
1822
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
19-
write!(f, "BlockList {:?}", self.iter().collect::<Vec<Block>>())
23+
write!(f, "{:?}", self.iter().collect::<Vec<Block>>())
2024
}
2125
}
2226

@@ -27,12 +31,52 @@ impl BlockList {
2731
last: None,
2832
size,
2933
lock: AtomicBool::new(false),
34+
#[cfg(feature = "ms_block_list_sanity")]
35+
sanity_list: Mutex::new(vec![]),
36+
}
37+
}
38+
39+
// fn access_block_list<R: Copy, F: FnOnce() -> R>(&self, access_func: F) -> R {
40+
// #[cfg(feature = "ms_block_list_sanity")]
41+
// let mut sanity_list = self.sanity_list.lock().unwrap();
42+
43+
// let ret = access_func();
44+
45+
// // Verify the block list is the same as the sanity list
46+
// #[cfg(feature = "ms_block_list_sanity")]
47+
// {
48+
// if !sanity_list.iter().map(|b| *b).eq(BlockListIterator { cursor: self.first }) {
49+
// eprintln!("Sanity block list: {:?}", &mut sanity_list as &mut Vec<Block>);
50+
// eprintln!("Actual block list: {:?}", self);
51+
// panic!("Incorrect block list");
52+
// }
53+
// }
54+
55+
// ret
56+
// }
57+
#[cfg(feature = "ms_block_list_sanity")]
58+
fn verify_block_list(&self, sanity_list: &mut Vec<Block>) {
59+
if !sanity_list.iter().map(|b| *b).eq(BlockListIterator { cursor: self.first }) {
60+
eprintln!("Sanity block list: {:?}", sanity_list);
61+
eprintln!("First {:?}", sanity_list.get(0));
62+
eprintln!("Actual block list: {:?}", self);
63+
eprintln!("First {:?}", self.first);
64+
panic!("Incorrect block list");
3065
}
3166
}
3267

3368
/// List has no blocks
3469
pub fn is_empty(&self) -> bool {
35-
self.first.is_none()
70+
let ret = self.first.is_none();
71+
72+
#[cfg(feature = "ms_block_list_sanity")]
73+
{
74+
let mut sanity_list = self.sanity_list.lock().unwrap();
75+
self.verify_block_list(&mut sanity_list);
76+
assert_eq!(sanity_list.is_empty(), ret);
77+
}
78+
79+
ret
3680
}
3781

3882
/// Remove a block from the list
@@ -57,11 +101,22 @@ impl BlockList {
57101
next.store_prev_block(prev);
58102
}
59103
}
104+
105+
#[cfg(feature = "ms_block_list_sanity")]
106+
{
107+
let mut sanity_list = self.sanity_list.lock().unwrap();
108+
if let Some((index, _)) = sanity_list.iter().enumerate().find(|&(_, &val)| val == block) {
109+
sanity_list.remove(index);
110+
} else {
111+
panic!("Cannot find {:?} in the block list", block);
112+
}
113+
self.verify_block_list(&mut sanity_list);
114+
}
60115
}
61116

62117
/// Pop the first block in the list
63118
pub fn pop(&mut self) -> Option<Block> {
64-
if let Some(head) = self.first {
119+
let ret = if let Some(head) = self.first {
65120
if let Some(next) = head.load_next_block() {
66121
self.first = Some(next);
67122
next.clear_prev_block();
@@ -75,7 +130,21 @@ impl BlockList {
75130
Some(head)
76131
} else {
77132
None
133+
};
134+
135+
#[cfg(feature = "ms_block_list_sanity")]
136+
{
137+
let mut sanity_list = self.sanity_list.lock().unwrap();
138+
let sanity_ret = if sanity_list.is_empty() {
139+
None
140+
} else {
141+
Some(sanity_list.remove(0)) // pop first
142+
};
143+
self.verify_block_list(&mut sanity_list);
144+
assert_eq!(sanity_ret, ret);
78145
}
146+
147+
ret
79148
}
80149

81150
/// Push block to the front of the list
@@ -93,10 +162,28 @@ impl BlockList {
93162
self.first = Some(block);
94163
}
95164
block.store_block_list(self);
165+
166+
#[cfg(feature = "ms_block_list_sanity")]
167+
{
168+
let mut sanity_list = self.sanity_list.lock().unwrap();
169+
sanity_list.insert(0, block); // push front
170+
self.verify_block_list(&mut sanity_list);
171+
}
96172
}
97173

98174
/// Moves all the blocks of `other` into `self`, leaving `other` empty.
99175
pub fn append(&mut self, other: &mut BlockList) {
176+
#[cfg(feature = "ms_block_list_sanity")]
177+
{
178+
// Check before merging
179+
let mut sanity_list = self.sanity_list.lock().unwrap();
180+
self.verify_block_list(&mut sanity_list);
181+
let mut sanity_list_other = other.sanity_list.lock().unwrap();
182+
other.verify_block_list(&mut sanity_list_other);
183+
}
184+
#[cfg(feature = "ms_block_list_sanity")]
185+
let mut sanity_list_in_other = other.sanity_list.lock().unwrap().clone();
186+
100187
debug_assert_eq!(self.size, other.size);
101188
if !other.is_empty() {
102189
debug_assert!(
@@ -128,12 +215,25 @@ impl BlockList {
128215
}
129216
other.reset();
130217
}
218+
219+
#[cfg(feature = "ms_block_list_sanity")]
220+
{
221+
let mut sanity_list = self.sanity_list.lock().unwrap();
222+
sanity_list.append(&mut sanity_list_in_other);
223+
self.verify_block_list(&mut sanity_list);
224+
}
131225
}
132226

133227
/// Remove all blocks
134228
fn reset(&mut self) {
135229
self.first = None;
136230
self.last = None;
231+
232+
#[cfg(feature = "ms_block_list_sanity")]
233+
{
234+
let mut sanity_list = self.sanity_list.lock().unwrap();
235+
sanity_list.clear();
236+
}
137237
}
138238

139239
/// Lock the list. The MiMalloc allocator mostly uses thread-local block lists, and those operations on the list
@@ -172,6 +272,24 @@ impl BlockList {
172272
}
173273
}
174274
}
275+
276+
/// Get the size of this block list.
277+
pub fn size(&self) -> usize {
278+
let ret = self.size;
279+
280+
#[cfg(feature = "ms_block_list_sanity")]
281+
{
282+
let mut sanity_list = self.sanity_list.lock().unwrap();
283+
self.verify_block_list(&mut sanity_list);
284+
}
285+
286+
ret
287+
}
288+
289+
/// Get the first block in the list.
290+
pub fn first(&self) -> Option<Block> {
291+
self.first
292+
}
175293
}
176294

177295
pub struct BlockListIterator {

‎src/util/alloc/free_list_allocator.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,10 +218,10 @@ impl<VM: VMBinding> FreeListAllocator<VM> {
218218
debug_assert!(bin <= MAX_BIN);
219219

220220
let available = &mut available_blocks[bin];
221-
debug_assert!(available.size >= size);
221+
debug_assert!(available.size() >= size);
222222

223223
if !available.is_empty() {
224-
let mut cursor = available.first;
224+
let mut cursor = available.first();
225225

226226
while let Some(block) = cursor {
227227
if block.has_free_cells() {
@@ -230,7 +230,7 @@ impl<VM: VMBinding> FreeListAllocator<VM> {
230230
available.pop();
231231
consumed_blocks.get_mut(bin).unwrap().push(block);
232232

233-
cursor = available.first;
233+
cursor = available.first();
234234
}
235235
}
236236

@@ -303,7 +303,7 @@ impl<VM: VMBinding> FreeListAllocator<VM> {
303303

304304
crate::policy::marksweepspace::native_ms::BlockAcquireResult::Fresh(block) => {
305305
self.add_to_available_blocks(bin, block, stress_test);
306-
self.init_block(block, self.available_blocks[bin].size);
306+
self.init_block(block, self.available_blocks[bin].size());
307307

308308
return Some(block);
309309
}

0 commit comments

Comments
 (0)