Skip to content

Commit 6a8c306

Browse files
qinsoonqinsoon
committed
Revert "Assert marksweep block list"
This reverts commit 1e05752.
1 parent f4fa7f2 commit 6a8c306

File tree

4 files changed

+17
-153
lines changed

4 files changed

+17
-153
lines changed

‎Cargo.toml

-3
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,6 @@ 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-
131128
# Run sanity GC
132129
sanity = []
133130
# Run analysis

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

+6-6
Original file line numberDiff line numberDiff line change
@@ -136,35 +136,35 @@ impl Block {
136136
.is_ok()
137137
}
138138

139-
pub(in crate::policy::marksweepspace::native_ms) fn load_prev_block(&self) -> Option<Block> {
139+
pub 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(in crate::policy::marksweepspace::native_ms) fn load_next_block(&self) -> Option<Block> {
144+
pub 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(in crate::policy::marksweepspace::native_ms) fn store_next_block(&self, next: Block) {
149+
pub 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(in crate::policy::marksweepspace::native_ms) fn clear_next_block(&self) {
155+
pub fn clear_next_block(&self) {
156156
unsafe {
157157
Block::NEXT_BLOCK_TABLE.store::<usize>(self.start(), 0);
158158
}
159159
}
160160

161-
pub(in crate::policy::marksweepspace::native_ms) fn store_prev_block(&self, prev: Block) {
161+
pub 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(in crate::policy::marksweepspace::native_ms) fn clear_prev_block(&self) {
167+
pub 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

+7-140
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,19 @@ 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;
97

108
/// List of blocks owned by the allocator
119
#[repr(C)]
1210
pub struct BlockList {
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>>,
11+
pub first: Option<Block>,
12+
pub last: Option<Block>,
13+
pub size: usize,
14+
pub lock: AtomicBool,
1915
}
2016

2117
impl std::fmt::Debug for BlockList {
2218
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
23-
write!(f, "{:?}", self.iter().collect::<Vec<Block>>())
19+
write!(f, "BlockList {:?}", self.iter().collect::<Vec<Block>>())
2420
}
2521
}
2622

@@ -31,57 +27,12 @@ impl BlockList {
3127
last: None,
3228
size,
3329
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
60-
.iter()
61-
.map(|b| *b)
62-
.eq(BlockListIterator { cursor: self.first })
63-
{
64-
eprintln!("Sanity block list: {:?}", sanity_list);
65-
eprintln!("First {:?}", sanity_list.get(0));
66-
eprintln!("Actual block list: {:?}", self);
67-
eprintln!("First {:?}", self.first);
68-
eprintln!("Block list {:?}", self as *const _);
69-
panic!("Incorrect block list");
7030
}
7131
}
7232

7333
/// List has no blocks
7434
pub fn is_empty(&self) -> bool {
75-
let ret = self.first.is_none();
76-
77-
#[cfg(feature = "ms_block_list_sanity")]
78-
{
79-
let mut sanity_list = self.sanity_list.lock().unwrap();
80-
self.verify_block_list(&mut sanity_list);
81-
assert_eq!(sanity_list.is_empty(), ret);
82-
}
83-
84-
ret
35+
self.first.is_none()
8536
}
8637

8738
/// Remove a block from the list
@@ -109,26 +60,11 @@ impl BlockList {
10960
next.store_prev_block(prev);
11061
}
11162
}
112-
113-
#[cfg(feature = "ms_block_list_sanity")]
114-
{
115-
let mut sanity_list = self.sanity_list.lock().unwrap();
116-
if let Some((index, _)) = sanity_list
117-
.iter()
118-
.enumerate()
119-
.find(|&(_, &val)| val == block)
120-
{
121-
sanity_list.remove(index);
122-
} else {
123-
panic!("Cannot find {:?} in the block list", block);
124-
}
125-
self.verify_block_list(&mut sanity_list);
126-
}
12763
}
12864

12965
/// Pop the first block in the list
13066
pub fn pop(&mut self) -> Option<Block> {
131-
let ret = if let Some(head) = self.first {
67+
if let Some(head) = self.first {
13268
if let Some(next) = head.load_next_block() {
13369
self.first = Some(next);
13470
next.clear_prev_block();
@@ -142,22 +78,7 @@ impl BlockList {
14278
Some(head)
14379
} else {
14480
None
145-
};
146-
147-
#[cfg(feature = "ms_block_list_sanity")]
148-
{
149-
let mut sanity_list = self.sanity_list.lock().unwrap();
150-
let sanity_ret = if sanity_list.is_empty() {
151-
None
152-
} else {
153-
Some(sanity_list.remove(0)) // pop first
154-
};
155-
self.verify_block_list(&mut sanity_list);
156-
assert_eq!(sanity_ret, ret);
15781
}
158-
159-
trace!("Blocklist {:?}: Pop = {:?}", self as *const _, ret);
160-
ret
16182
}
16283

16384
/// Push block to the front of the list
@@ -176,33 +97,10 @@ impl BlockList {
17697
self.first = Some(block);
17798
}
17899
block.store_block_list(self);
179-
180-
#[cfg(feature = "ms_block_list_sanity")]
181-
{
182-
let mut sanity_list = self.sanity_list.lock().unwrap();
183-
sanity_list.insert(0, block); // push front
184-
self.verify_block_list(&mut sanity_list);
185-
}
186100
}
187101

188102
/// Moves all the blocks of `other` into `self`, leaving `other` empty.
189103
pub fn append(&mut self, other: &mut BlockList) {
190-
trace!(
191-
"Blocklist {:?}: Append Blocklist {:?}",
192-
self as *const _,
193-
other as *const _
194-
);
195-
#[cfg(feature = "ms_block_list_sanity")]
196-
{
197-
// Check before merging
198-
let mut sanity_list = self.sanity_list.lock().unwrap();
199-
self.verify_block_list(&mut sanity_list);
200-
let mut sanity_list_other = other.sanity_list.lock().unwrap();
201-
other.verify_block_list(&mut sanity_list_other);
202-
}
203-
#[cfg(feature = "ms_block_list_sanity")]
204-
let mut sanity_list_in_other = other.sanity_list.lock().unwrap().clone();
205-
206104
debug_assert_eq!(self.size, other.size);
207105
if !other.is_empty() {
208106
debug_assert!(
@@ -234,26 +132,13 @@ impl BlockList {
234132
}
235133
other.reset();
236134
}
237-
238-
#[cfg(feature = "ms_block_list_sanity")]
239-
{
240-
let mut sanity_list = self.sanity_list.lock().unwrap();
241-
sanity_list.append(&mut sanity_list_in_other);
242-
self.verify_block_list(&mut sanity_list);
243-
}
244135
}
245136

246137
/// Remove all blocks
247138
fn reset(&mut self) {
248139
trace!("Blocklist {:?}: Reset", self as *const _);
249140
self.first = None;
250141
self.last = None;
251-
252-
#[cfg(feature = "ms_block_list_sanity")]
253-
{
254-
let mut sanity_list = self.sanity_list.lock().unwrap();
255-
sanity_list.clear();
256-
}
257142
}
258143

259144
/// Lock the list. The MiMalloc allocator mostly uses thread-local block lists, and those operations on the list
@@ -294,24 +179,6 @@ impl BlockList {
294179
}
295180
}
296181
}
297-
298-
/// Get the size of this block list.
299-
pub fn size(&self) -> usize {
300-
let ret = self.size;
301-
302-
#[cfg(feature = "ms_block_list_sanity")]
303-
{
304-
let mut sanity_list = self.sanity_list.lock().unwrap();
305-
self.verify_block_list(&mut sanity_list);
306-
}
307-
308-
ret
309-
}
310-
311-
/// Get the first block in the list.
312-
pub fn first(&self) -> Option<Block> {
313-
self.first
314-
}
315182
}
316183

317184
pub struct BlockListIterator {

‎src/util/alloc/free_list_allocator.rs

+4-4
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)