Skip to content

Commit 473fd58

Browse files
Fetch before trying to atomically mark in Compressor (#1438)
Tracing is exceedingly slow on biojava if we don't test-and-test-and-set. My guess is that we get cache line ping-pong when many workers try to mark the same popular objects; but every policy I checked has some sort of test-and-test-and-set pattern, and this seems like a common sense thing to do. [Plotty](http://squirrel.anu.edu.au/plotty/hayleyp/plots/p/GkUHfS) for a detailed before-and-after, although both configurations have some other changes to Compressor which I will upstream later. But this change still speeds up upstream - here is before: ``` ============================ MMTk Statistics Totals ============================ GC time.other time.stw total-work.time.min total-work.time.total total-work.time.max total-work.count 14 8575.41 14920.23 inf 0.000 -inf 0 Total time: 23495.63 ms ------------------------------ End MMTk Statistics ----------------------------- ``` And after: ``` ============================ MMTk Statistics Totals ============================ GC time.other time.stw total-work.time.total total-work.time.max total-work.time.min total-work.count 14 8826.60 9548.96 0.000 -inf inf 0 Total time: 18375.56 ms ------------------------------ End MMTk Statistics ----------------------------- ```
1 parent 2ad6d36 commit 473fd58

File tree

2 files changed

+19
-13
lines changed

2 files changed

+19
-13
lines changed

src/policy/compressor/compressorspace.rs

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,6 @@ pub struct CompressorSpace<VM: VMBinding> {
4848
scheduler: Arc<GCWorkScheduler<VM>>,
4949
}
5050

51-
pub(crate) const GC_MARK_BIT_MASK: u8 = 1;
52-
5351
impl<VM: VMBinding> SFT for CompressorSpace<VM> {
5452
fn name(&self) -> &'static str {
5553
self.get_name()
@@ -271,19 +269,26 @@ impl<VM: VMBinding> CompressorSpace<VM> {
271269
}
272270

273271
pub fn test_and_mark(object: ObjectReference) -> bool {
274-
let old = forwarding::MARK_SPEC.fetch_or_atomic(
275-
object.to_raw_address(),
276-
GC_MARK_BIT_MASK,
277-
Ordering::SeqCst,
278-
);
279-
(old & GC_MARK_BIT_MASK) == 0
272+
forwarding::MARK_SPEC
273+
.fetch_update_atomic::<u8, _>(
274+
object.to_raw_address(),
275+
Ordering::SeqCst,
276+
Ordering::Relaxed,
277+
|v| {
278+
if v == 0 {
279+
Some(1)
280+
} else {
281+
None
282+
}
283+
},
284+
)
285+
.is_ok()
280286
}
281287

282288
pub fn is_marked(object: ObjectReference) -> bool {
283-
let old_value =
289+
let mark_bit =
284290
forwarding::MARK_SPEC.load_atomic::<u8>(object.to_raw_address(), Ordering::SeqCst);
285-
let mark_bit = old_value & GC_MARK_BIT_MASK;
286-
mark_bit != 0
291+
mark_bit == 1
287292
}
288293

289294
fn generate_tasks(

src/policy/compressor/forwarding.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use crate::policy::compressor::GC_MARK_BIT_MASK;
21
use crate::util::constants::BYTES_IN_WORD;
32
use crate::util::linear_scan::{Region, RegionIterator};
43
use crate::util::metadata::side_metadata::spec_defs::{COMPRESSOR_MARK, COMPRESSOR_OFFSET_VECTOR};
@@ -144,7 +143,9 @@ impl<VM: VMBinding> ForwardingMetadata<VM> {
144143
);
145144
}
146145

147-
MARK_SPEC.fetch_or_atomic(last_word_of_object, GC_MARK_BIT_MASK, Ordering::SeqCst);
146+
// We only mark the last word as input to computing forwarding
147+
// information, so relaxed consistency is okay.
148+
MARK_SPEC.fetch_or_atomic::<u8>(last_word_of_object, 1, Ordering::Relaxed);
148149
}
149150

150151
pub fn calculate_offset_vector(&self, region: CompressorRegion, cursor: Address) {

0 commit comments

Comments
 (0)