Skip to content

Commit

Permalink
Add IN_QUEUE mark
Browse files Browse the repository at this point in the history
Also, `TRACED` is renamed to `IN_LIST` and the `DROPPED` marking is moved to a special value of the tracing counter
  • Loading branch information
frengor committed Aug 1, 2024
1 parent 9493d24 commit bc810c2
Show file tree
Hide file tree
Showing 6 changed files with 209 additions and 119 deletions.
19 changes: 9 additions & 10 deletions src/cc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl<T: Trace> Cc<T> {
assert!(self.is_unique(), "Cc<_> is not unique");

assert!(
!self.counter_marker().is_traced(),
!self.counter_marker().is_in_list_or_queue(),
"Cc<_> is being used by the collector and inner value cannot be taken out (this might have happen inside Trace, Finalize or Drop implementations)."
);

Expand Down Expand Up @@ -259,10 +259,9 @@ impl<T: ?Sized + Trace> Drop for Cc<T> {
add_to_list(cc.inner.cast());
}

// A CcBox can be marked traced only during collections while being into a list different than POSSIBLE_CYCLES.
// A CcBox can be in list or queue only during collections while being into a list different than POSSIBLE_CYCLES.
// In this case, no further action has to be taken, except decrementing the reference counter.
// Skip the rest of the code also when the value has already been dropped
if self.counter_marker().is_traced_or_dropped() {
if self.counter_marker().is_in_list_or_queue() {
decrement_counter(self);
return;
}
Expand Down Expand Up @@ -298,7 +297,7 @@ impl<T: ?Sized + Trace> Drop for Cc<T> {
{
// Set the object as dropped before dropping and deallocating it
// This feature is used only in weak pointers, so do this only if they're enabled
self.counter_marker().mark(Mark::Dropped);
self.counter_marker().set_dropped(true);

self.inner().drop_metadata();
}
Expand Down Expand Up @@ -590,7 +589,7 @@ impl CcBox<()> {
{
// Set the object as dropped before dropping it
// This feature is used only in weak pointers, so do this only if they're enabled
ptr.as_ref().counter_marker().mark(Mark::Dropped);
ptr.as_ref().counter_marker().set_dropped(true);
}

CcBox::get_traceable(ptr).as_mut().drop_elem();
Expand Down Expand Up @@ -623,7 +622,7 @@ impl CcBox<()> {
counter_marker.reset_tracing_counter();

// Element is surely not already marked, marking
counter_marker.mark(Mark::Traced);
counter_marker.mark(Mark::InList);
},
ContextInner::RootTracing { .. } => {
// ptr is a root
Expand Down Expand Up @@ -659,7 +658,7 @@ impl CcBox<()> {
root_list,
non_root_list,
} => {
if !counter_marker.is_traced() {
if !counter_marker.is_in_list() {
// Not already marked

// Make sure ptr is not in POSSIBLE_CYCLES list
Expand All @@ -680,7 +679,7 @@ impl CcBox<()> {

// Marking here since the previous debug_asserts might panic
// before ptr is actually added to root_list or non_root_list
counter_marker.mark(Mark::Traced);
counter_marker.mark(Mark::InList);

// Continue tracing
true
Expand All @@ -703,7 +702,7 @@ impl CcBox<()> {
}
},
ContextInner::RootTracing { non_root_list, root_list } => {
if counter_marker.is_traced() {
if counter_marker.is_in_list() {
// Marking NonMarked since ptr will be removed from any list it's into. Also, marking
// NonMarked will avoid tracing this CcBox again (thanks to the if condition)
counter_marker.mark(Mark::NonMarked);
Expand Down
100 changes: 63 additions & 37 deletions src/counter_marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use crate::utils;

const NON_MARKED: u32 = 0u32;
const IN_POSSIBLE_CYCLES: u32 = 1u32 << (u32::BITS - 2);
const TRACED: u32 = 2u32 << (u32::BITS - 2);
#[allow(dead_code)] // Used only in weak ptrs, silence warnings
const DROPPED: u32 = 3u32 << (u32::BITS - 2);
const IN_LIST: u32 = 2u32 << (u32::BITS - 2);
#[allow(dead_code)] // TODO Remove when actually used
const IN_QUEUE: u32 = 3u32 << (u32::BITS - 2);

const COUNTER_MASK: u32 = 0b11111111111111u32; // First 14 bits set to 1
const TRACING_COUNTER_MASK: u32 = COUNTER_MASK << 14; // 14 bits set to 1 followed by 14 bits set to 0
Expand All @@ -19,7 +19,7 @@ const INITIAL_VALUE: u32 = COUNTER_MASK + 2; // +2 means that tracing counter an
const INITIAL_VALUE_FINALIZED: u32 = INITIAL_VALUE | FINALIZED_MASK;

// pub(crate) to make it available in tests
pub(crate) const MAX: u32 = COUNTER_MASK;
pub(crate) const MAX: u32 = COUNTER_MASK - 1;

/// Internal representation:
/// ```text
Expand All @@ -31,12 +31,14 @@ pub(crate) const MAX: u32 = COUNTER_MASK;
/// * `A` has 4 possible states:
/// * `NON_MARKED`
/// * `IN_POSSIBLE_CYCLES`: in `possible_cycles` list (implies `NON_MARKED`)
/// * `TRACED`: in `root_list` or `non_root_list`
/// * `DROPPED`: allocated value has already been dropped (but not yet deallocated)
/// * `IN_LIST`: in `root_list` or `non_root_list`
/// * `IN_QUEUE`: in queue to be traced
/// * `B` is `1` when metadata has been allocated, `0` otherwise
/// * `C` is `1` when the element inside `CcBox` has already been finalized, `0` otherwise
/// * `D` is the tracing counter
/// * `E` is the counter (last one for sum/subtraction efficiency)
/// * `D` is the tracing counter. The max value (the one with every bit set to 1) is reserved
/// and indicates that the allocated value has already been dropped (but not yet deallocated)
/// * `E` is the reference counter (last one for sum/subtraction efficiency). The max value (the
/// one with every bit set to 1) is reserved and should not be used
#[derive(Clone, Debug)]
#[repr(transparent)]
pub(crate) struct CounterMarker {
Expand All @@ -60,6 +62,8 @@ impl CounterMarker {

#[inline]
pub(crate) fn increment_counter(&self) -> Result<(), OverflowError> {
debug_assert!(self.counter() != COUNTER_MASK); // Check for reserved value

if self.counter() == MAX {
utils::cold(); // This branch of the if is rarely taken
Err(OverflowError)
Expand All @@ -71,6 +75,8 @@ impl CounterMarker {

#[inline]
pub(crate) fn decrement_counter(&self) -> Result<(), OverflowError> {
debug_assert!(self.counter() != COUNTER_MASK); // Check for reserved value

if self.counter() == 0 {
utils::cold(); // This branch of the if is rarely taken
Err(OverflowError)
Expand All @@ -82,6 +88,8 @@ impl CounterMarker {

#[inline]
pub(crate) fn increment_tracing_counter(&self) -> Result<(), OverflowError> {
debug_assert!(self.tracing_counter() != COUNTER_MASK); // Check for reserved value

if self.tracing_counter() == MAX {
utils::cold(); // This branch of the if is rarely taken
Err(OverflowError)
Expand All @@ -94,6 +102,8 @@ impl CounterMarker {

#[inline]
pub(crate) fn _decrement_tracing_counter(&self) -> Result<(), OverflowError> {
debug_assert!(self.tracing_counter() != COUNTER_MASK); // Check for reserved value

if self.tracing_counter() == 0 {
utils::cold(); // This branch of the if is rarely taken
Err(OverflowError)
Expand All @@ -106,24 +116,24 @@ impl CounterMarker {

#[inline]
pub(crate) fn counter(&self) -> u32 {
self.counter.get() & COUNTER_MASK
let rc = self.counter.get() & COUNTER_MASK;
debug_assert!(rc != COUNTER_MASK); // Check for reserved value
rc
}

#[inline]
pub(crate) fn tracing_counter(&self) -> u32 {
(self.counter.get() & TRACING_COUNTER_MASK) >> 14
let tc = (self.counter.get() & TRACING_COUNTER_MASK) >> 14;
debug_assert!(tc != COUNTER_MASK); // Check for reserved value
tc
}

#[inline]
pub(crate) fn reset_tracing_counter(&self) {
debug_assert!(self.tracing_counter() != COUNTER_MASK); // Check for reserved value
self.counter.set(self.counter.get() & !TRACING_COUNTER_MASK);
}

#[inline]
pub(crate) fn is_in_possible_cycles(&self) -> bool {
(self.counter.get() & BITS_MASK) == IN_POSSIBLE_CYCLES
}

#[cfg(feature = "finalization")]
#[inline]
pub(crate) fn needs_finalization(&self) -> bool {
Expand All @@ -133,7 +143,7 @@ impl CounterMarker {
#[cfg(feature = "finalization")]
#[inline]
pub(crate) fn set_finalized(&self, finalized: bool) {
self.set_bit(finalized, FINALIZED_MASK);
self.set_bits(finalized, FINALIZED_MASK);
}

#[cfg(feature = "weak-ptrs")]
Expand All @@ -145,17 +155,19 @@ impl CounterMarker {
#[cfg(feature = "weak-ptrs")]
#[inline]
pub(crate) fn set_allocated_for_metadata(&self, allocated_for_metadata: bool) {
self.set_bit(allocated_for_metadata, METADATA_MASK);
self.set_bits(allocated_for_metadata, METADATA_MASK);
}

#[cfg(any(feature = "weak-ptrs", feature = "finalization"))]
#[inline(always)]
fn set_bit(&self, value: bool, mask: u32) {
if value {
self.counter.set(self.counter.get() | mask);
} else {
self.counter.set(self.counter.get() & !mask);
}
#[cfg(feature = "weak-ptrs")]
#[inline]
pub(crate) fn is_dropped(&self) -> bool {
(self.counter.get() & TRACING_COUNTER_MASK) == TRACING_COUNTER_MASK
}

#[cfg(feature = "weak-ptrs")]
#[inline]
pub(crate) fn set_dropped(&self, dropped: bool) {
self.set_bits(dropped, TRACING_COUNTER_MASK);
}

#[inline]
Expand All @@ -166,35 +178,49 @@ impl CounterMarker {
}

#[inline]
pub(crate) fn is_traced(&self) -> bool {
(self.counter.get() & BITS_MASK) == TRACED
pub(crate) fn is_in_possible_cycles(&self) -> bool {
(self.counter.get() & BITS_MASK) == IN_POSSIBLE_CYCLES
}

#[inline]
pub(crate) fn is_traced_or_dropped(&self) -> bool {
// true if (self.counter & BITS_MASK) is equal to 10 or 11,
// so if the first bit is 1
(self.counter.get() & FIRST_BIT_MASK) == FIRST_BIT_MASK
pub(crate) fn is_in_list(&self) -> bool {
(self.counter.get() & BITS_MASK) == IN_LIST
}

#[cfg(any(feature = "weak-ptrs", all(test, feature = "std")))]
#[allow(dead_code)] // TODO Remove when actually used
#[inline]
pub(crate) fn is_dropped(&self) -> bool {
(self.counter.get() & BITS_MASK) == DROPPED
pub(crate) fn is_in_queue(&self) -> bool {
(self.counter.get() & BITS_MASK) == IN_QUEUE
}

#[inline]
pub(crate) fn is_in_list_or_queue(&self) -> bool {
// true if (self.counter & BITS_MASK) is equal to 10 or 11,
// so if the first bit is 1
(self.counter.get() & FIRST_BIT_MASK) == FIRST_BIT_MASK
}

#[inline]
pub(crate) fn mark(&self, new_mark: Mark) {
self.counter.set((self.counter.get() & !BITS_MASK) | (new_mark as u32));
}

#[inline(always)]
fn set_bits(&self, value: bool, mask: u32) {
if value {
self.counter.set(self.counter.get() | mask);
} else {
self.counter.set(self.counter.get() & !mask);
}
}
}

#[derive(Copy, Clone, Debug)]
#[repr(u32)]
pub(crate) enum Mark {
NonMarked = NON_MARKED,
PossibleCycles = IN_POSSIBLE_CYCLES,
Traced = TRACED,
#[allow(dead_code)] // Used only in weak ptrs, silence warnings
Dropped = DROPPED,
InList = IN_LIST,
#[allow(dead_code)] // TODO Remove when actually used
InQueue = IN_QUEUE,
}
6 changes: 3 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ fn __collect(state: &State, possible_cycles: &PossibleCycles) {
counter_marker.tracing_counter(),
counter_marker.counter()
);
debug_assert!(counter_marker.is_traced());
debug_assert!(counter_marker.is_in_list());
});

#[cfg(feature = "finalization")]
Expand Down Expand Up @@ -382,7 +382,7 @@ fn deallocate_list(to_deallocate_list: LinkedList, state: &State) {
#[cfg(feature = "weak-ptrs")]
while let Some(ptr) = self.list.remove_first() {
// Always set the mark, since it has been cleared by remove_first
unsafe { ptr.as_ref() }.counter_marker().mark(Mark::Dropped);
unsafe { ptr.as_ref() }.counter_marker().set_dropped(true);
}

// If not using weak pointers, just call the list's drop implementation
Expand All @@ -404,7 +404,7 @@ fn deallocate_list(to_deallocate_list: LinkedList, state: &State) {
to_deallocate_list.iter().for_each(|ptr| {
// SAFETY: ptr is valid to access and drop in place
unsafe {
debug_assert!(ptr.as_ref().counter_marker().is_traced());
debug_assert!(ptr.as_ref().counter_marker().is_in_list());

#[cfg(feature = "weak-ptrs")]
ptr.as_ref().drop_metadata();
Expand Down
Loading

0 comments on commit bc810c2

Please sign in to comment.