mirrors/salsa
1
0
Fork 0
mirror of https://github.com/salsa-rs/salsa.git synced 2025-08-04 19:08:32 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

use atomic loads only for late fields, fix backdating

Browse source
This commit is contained in:
puuuuh 2025-07-06 09:20:45 +03:00
parent d6eeae3e5a
commit a674556b0a
No known key found for this signature in database
GPG key ID: 171E3E1356CEE151
10 changed files with 137 additions and 68 deletions
Download patch file Download diff file Expand all files Collapse all files

15
components/salsa-macro-rules/src/maybe_backdate.rs
View file

@ -62,10 +62,19 @@ macro_rules! maybe_backdate_late {
$new_field_place:expr,
$revision_place:expr,
$current_revision:expr,
$deps_changed_at:expr,
$zalsa:ident,
) => {
if $zalsa::LateField::maybe_update(&mut $old_field_place, $new_field_place, $maybe_update, $revision_place.load()) {
$revision_place.store($current_revision);
}
$revision_place.non_atomic_store(match $zalsa::LateField::maybe_update(&mut $old_field_place, $new_field_place, $maybe_update, $revision_place.load()) {
$zalsa::late_field::UpdateResult::Dirty => {
$current_revision
}
$zalsa::late_field::UpdateResult::Update => {
$deps_changed_at
}
$zalsa::late_field::UpdateResult::Backdate(rev) => {
rev
}
})
};
}

28
components/salsa-macro-rules/src/setup_tracked_struct.rs
View file

@ -147,7 +147,7 @@ macro_rules! setup_tracked_struct {
type Fields<$db_lt> = ($($zalsa::macro_if!(if $field_is_late { $zalsa::LateField<$field_ty> } else { $field_ty }),)*);
type Revisions = [$zalsa::AtomicRevision; $N];
type Revisions = [$zalsa::MaybeAtomicRevision; $N];
type Struct<$db_lt> = $Struct<$db_lt>;
@ -156,11 +156,24 @@ macro_rules! setup_tracked_struct {
}
fn new_revisions(current_revision: $Revision) -> Self::Revisions {
std::array::from_fn(|_| $zalsa::AtomicRevision::from(current_revision))
std::array::from_fn(|_| $zalsa::MaybeAtomicRevision::from(current_revision))
}
#[inline(always)]
fn field_revision(data: &Self::Revisions, i: usize) -> $Revision {
$(if $tracked_is_late && (i == $relative_tracked_index) {
return data[i].load();
})*;
// SAFETY: there is no writes to non-late field revision
unsafe {
data[i].non_atomic_load()
}
}
unsafe fn update_fields<$db_lt>(
current_revision: $Revision,
deps_changed_at: $Revision,
revisions: &mut Self::Revisions,
old_fields: *mut Self::Fields<$db_lt>,
new_fields: Self::Fields<$db_lt>,
@ -177,6 +190,7 @@ macro_rules! setup_tracked_struct {
new_fields.$absolute_tracked_index,
revisions[$relative_tracked_index],
current_revision,
deps_changed_at,
$zalsa,
);
} else {
@ -186,7 +200,7 @@ macro_rules! setup_tracked_struct {
(*old_fields).$absolute_tracked_index,
new_fields.$absolute_tracked_index,
revisions[$relative_tracked_index],
current_revision,
deps_changed_at,
$zalsa,
);
}
@ -300,7 +314,9 @@ macro_rules! setup_tracked_struct {
$crate::return_mode_expression!(
$tracked_option,
$tracked_ty,
&fields.$absolute_tracked_index.get().expect("can't get late field without initialization"),
&fields
.$absolute_tracked_index
.get().expect("can't get late field without initialization"),
)
} else {
$crate::return_mode_expression!(
@ -323,8 +339,10 @@ macro_rules! setup_tracked_struct {
let ingredient = $Configuration::ingredient(db);
if let Some(old_rev) = ingredient.tracked_field(db, self, $relative_tracked_index)
.$absolute_tracked_index
.set_maybe_backdate(value) {
.set_and_maybe_backdate(value, $tracked_maybe_update) {
ingredient.revisions(db, self)[$relative_tracked_index].store(old_rev);
} else {
// TODO: Backdate to deps.changed_at here
}
}
} else {}

21
components/salsa-macros/src/salsa_struct.rs
View file

@ -251,27 +251,6 @@ where
Ok(())
}
/// Disallow `#[default]` attributes on the fields of this struct.
///
/// If an `#[default]` field is found, return an error.
///
/// # Parameters
///
/// * `kind`, the attribute name (e.g., `input` or `interned`)
fn disallow_late_id_fields(&self) -> syn::Result<()> {
// Check if any field has the `#[default]` attribute.
for ef in &self.fields {
if ef.has_late_attr && ef.has_tracked_attr {
return Err(syn::Error::new_spanned(
ef.field,
format!("`#[late]` cannot be used with `#[tracked]`"),
));
}
}
Ok(())
}
/// Check that the generic parameters look as expected for this kind of struct.
fn check_generics(&self) -> syn::Result<()> {
if A::HAS_LIFETIME {

5
components/salsa-macros/src/tracked_struct.rs
View file

@ -128,12 +128,11 @@ impl Macro {
let tracked_maybe_update = salsa_struct.tracked_fields_iter().map(|(_, field)| {
let field_ty = &field.field.ty;
let update = if let Some((with_token, maybe_update)) = &field.maybe_update_attr {
if let Some((with_token, maybe_update)) = &field.maybe_update_attr {
quote_spanned! { with_token.span() => ({ let maybe_update: unsafe fn(*mut #field_ty, #field_ty) -> bool = #maybe_update; maybe_update }) }
} else {
quote! {(#zalsa::UpdateDispatch::<#field_ty>::maybe_update)}
};
update
}
});
let untracked_maybe_update = salsa_struct.untracked_fields_iter().map(|(_, field)| {

3
src/lib.rs
View file

@ -95,7 +95,7 @@ pub mod plumbing {
IngredientIndices, MemoIngredientIndices, MemoIngredientMap, MemoIngredientSingletonIndex,
NewMemoIngredientIndices,
};
pub use crate::revision::{Revision, AtomicRevision};
pub use crate::revision::{Revision, MaybeAtomicRevision};
pub use crate::runtime::{stamp, Runtime, Stamp};
pub use crate::salsa_struct::SalsaStructInDb;
pub use crate::storage::{HasStorage, Storage};
@ -107,6 +107,7 @@ pub mod plumbing {
};
pub use crate::zalsa_local::ZalsaLocal;
pub use crate::tracked_struct::late_field::LateField;
pub use crate::tracked_struct::late_field;
pub mod accumulator {
pub use crate::accumulator::{IngredientImpl, JarImpl};

49
src/revision.rs
View file

@ -61,7 +61,7 @@ impl std::fmt::Debug for Revision {
}
#[derive(Debug)]
pub struct AtomicRevision {
pub(crate) struct AtomicRevision {
data: AtomicUsize,
}
@ -74,13 +74,13 @@ impl From<Revision> for AtomicRevision {
}
impl AtomicRevision {
pub const fn start() -> Self {
pub(crate) const fn start() -> Self {
Self {
data: AtomicUsize::new(START),
}
}
pub fn load(&self) -> Revision {
pub(crate) fn load(&self) -> Revision {
Revision {
// SAFETY: We know that the value is non-zero because we only ever store `START` which 1, or a
// Revision which is guaranteed to be non-zero.
@ -88,7 +88,7 @@ impl AtomicRevision {
}
}
pub fn store(&self, r: Revision) {
pub(crate) fn store(&self, r: Revision) {
self.data.store(r.as_usize(), Ordering::Release);
}
}
@ -141,6 +141,47 @@ impl OptionalAtomicRevision {
}
}
pub struct MaybeAtomicRevision {
data: AtomicUsize,
}
impl From<Revision> for MaybeAtomicRevision {
fn from(value: Revision) -> Self {
Self {
data: AtomicUsize::new(value.as_usize()),
}
}
}
impl MaybeAtomicRevision {
pub fn load(&self) -> Revision {
Revision {
// SAFETY: we only store Revision.as_usize, so it always valid
generation: unsafe { NonZeroUsize::new_unchecked(self.data.load(Ordering::Relaxed)) }
}
}
pub fn store(&self, revision: Revision) {
self.data.store(revision.as_usize(), Ordering::Relaxed);
}
/// # Safety
/// Caller must ensure that there are no unsyncronized writes to this variable.
pub unsafe fn non_atomic_load(&self) -> Revision {
Revision {
// SAFETY: we only store Revision.as_usize, so it always valid
generation: unsafe { NonZeroUsize::new_unchecked(std::ptr::read(self.data.as_ptr())) }
}
}
pub fn non_atomic_store(&mut self, revision: Revision) {
// SAFETY: we have &mut self, so there can't be any other refs
unsafe {
std::ptr::write(self.data.as_ptr(), revision.as_usize());
}
}
}
#[cfg(test)]
mod tests {
use super::*;

12
src/tracked_struct.rs
View file

@ -3,7 +3,6 @@
use std::any::TypeId;
use std::hash::Hash;
use std::marker::PhantomData;
use std::ops::Index;
use std::{fmt, mem};
use crossbeam_queue::SegQueue;
@ -16,7 +15,7 @@ use crate::id::{AsId, FromId};
use crate::ingredient::{Ingredient, Jar};
use crate::key::DatabaseKeyIndex;
use crate::plumbing::ZalsaLocal;
use crate::revision::{AtomicRevision, OptionalAtomicRevision};
use crate::revision::OptionalAtomicRevision;
use crate::runtime::Stamp;
use crate::salsa_struct::SalsaStructInDb;
use crate::sync::Arc;
@ -51,7 +50,7 @@ pub trait Configuration: Sized + 'static {
/// When a struct is re-recreated in a new revision, the corresponding
/// entries for each field are updated to the new revision if their
/// values have changed (or if the field is marked as `#[no_eq]`).
type Revisions: Send + Sync + Index<usize, Output = AtomicRevision>;
type Revisions: Send + Sync;
type Struct<'db>: Copy + FromId + AsId;
@ -60,6 +59,9 @@ pub trait Configuration: Sized + 'static {
/// Create a new value revision array where each element is set to `current_revision`.
fn new_revisions(current_revision: Revision) -> Self::Revisions;
/// Extract field revision by index.
fn field_revision(revisions: &Self::Revisions, index: usize) -> Revision;
/// Update the field data and, if the value has changed,
/// the appropriate entry in the `revisions` array (tracked fields only).
///
@ -86,6 +88,7 @@ pub trait Configuration: Sized + 'static {
/// for any field that changed.
unsafe fn update_fields<'db>(
current_revision: Revision,
deps_changed_at: Revision,
revisions: &mut Self::Revisions,
old_fields: *mut Self::Fields<'db>,
new_fields: Self::Fields<'db>,
@ -585,6 +588,7 @@ where
// to meet its safety invariant.
let untracked_update = unsafe {
C::update_fields(
current_revision,
current_deps.changed_at,
&mut data.revisions,
mem::transmute::<*mut C::Fields<'static>, *mut C::Fields<'db>>(
@ -755,7 +759,7 @@ where
data.read_lock(zalsa.current_revision());
let field_changed_at = data.revisions[relative_tracked_index].load();
let field_changed_at = C::field_revision(&data.revisions, relative_tracked_index);
zalsa_local.report_tracked_read_simple(
DatabaseKeyIndex::new(field_ingredient_index, id),

64
src/tracked_struct/late_field.rs
View file

@ -7,6 +7,15 @@ const ACQUIRED: usize = 1;
const SET: usize = 2;
const DIRTY: usize = 3;
pub enum UpdateResult {
// Was set in some revision, then updated to empty, then set to the same value as before
Backdate(Revision),
// Set to some new value
Update,
// New state is empty or dirty
Dirty
}
#[derive(Debug)]
pub struct LateField<T: Update> {
state: AtomicUsize,
@ -19,8 +28,8 @@ pub struct LateField<T: Update> {
unsafe impl<T: Update + Send> Send for LateField<T> {}
unsafe impl<T: Update + Sync> Sync for LateField<T> {}
impl<T: Update> LateField<T> {
pub fn new() -> LateField<T> {
impl<T: Update> Default for LateField<T> {
fn default() -> LateField<T> {
LateField {
state: AtomicUsize::new(EMPTY),
old_revision: None,
@ -28,41 +37,48 @@ impl<T: Update> LateField<T> {
data: UnsafeCell::new(MaybeUninit::uninit())
}
}
}
// Update self, store old revision to probably backdate later
pub fn maybe_update(&mut self, mut value: Self, maybe_update_inner: unsafe fn(*mut T, T) -> bool, old_revision: Revision) -> bool {
impl<T: Update> LateField<T> {
pub fn new() -> LateField<T> {
Self::default()
}
// Update self, store old revision if new field is empty, probably backdate later
pub fn maybe_update(&mut self, mut value: Self, maybe_update_inner: unsafe fn(*mut T, T) -> bool, prev_changed_at: Revision) -> UpdateResult {
let old_state = self.state.load(Ordering::Relaxed);
let new_state = value.state.load(Ordering::Relaxed);
let t = match (old_state, new_state) {
(EMPTY, EMPTY) => {
self.old_revision = None;
self.state.store(EMPTY, Ordering::Release);
false
UpdateResult::Dirty
},
(SET, EMPTY) => {
self.old_revision = Some(prev_changed_at);
// Save old value to probably backdate later
self.state.store(DIRTY, Ordering::Release);
UpdateResult::Dirty
}
(EMPTY, SET) => {
self.old_revision = None;
self.data = value.data;
self.state.store(SET, Ordering::Release);
true
UpdateResult::Update
},
(DIRTY, SET) => {
// SAFETY: DIRTY and SET state assumes that data is initialized
// SAFETY: new value in SET state, so its completely valid at this point
let changed = unsafe {
maybe_update_inner(self.data.get_mut().assume_init_mut(), value.data.get_mut().assume_init_read())
maybe_update_inner(self.data.get_mut().as_mut_ptr(), value.data.get_mut().assume_init_read())
};
self.state.store(SET, Ordering::Release);
changed
if changed {
UpdateResult::Update
} else {
UpdateResult::Backdate(self.old_revision.take().expect("dirty value should always have old_revision"))
}
},
(SET, EMPTY) => {
self.old_revision = Some(old_revision);
// Save old value to probably backdate later
self.state.store(DIRTY, Ordering::Release);
true
}
_ => panic!("unexpected state"),
};
@ -70,7 +86,7 @@ impl<T: Update> LateField<T> {
}
/// Set new value and returns saved revision if its not updated
pub fn set_maybe_backdate(&self, value: T) -> Option<Revision> {
pub fn set_and_maybe_backdate(&self, value: T, maybe_update_inner: unsafe fn(*mut T, T) -> bool) -> Option<Revision> {
let old_state = self.state.load(Ordering::Relaxed);
match old_state {
EMPTY => {},
@ -86,12 +102,13 @@ impl<T: Update> LateField<T> {
self.state.compare_exchange(old_state, ACQUIRED, Ordering::Acquire, Ordering::Relaxed).expect("concurrent set on late field is not allowed");
let updated = if old_state == EMPTY {
unsafe {
(&mut *self.data.get()).write(value);
(*self.data.get()).write(value);
}
true
} else {
// SAFETY: Only one thread can set state to ACQUIRE
unsafe {
Update::maybe_update((&mut *self.data.get()).assume_init_mut(), value)
maybe_update_inner((*self.data.get()).as_mut_ptr(), value)
}
};
@ -100,6 +117,7 @@ impl<T: Update> LateField<T> {
if updated {
None
} else {
debug_assert!(self.old_revision.is_some(), "dirty value should always have old_revision");
self.old_revision
}
}
@ -111,7 +129,7 @@ impl<T: Update> LateField<T> {
// SAFETY: we can't move from SET to any other state while we have ref to self
Some(unsafe {
(&*self.data.get()).assume_init_ref()
(*self.data.get()).assume_init_ref()
})
}
}

2
src/tracked_struct/tracked_field.rs
View file

@ -64,7 +64,7 @@ where
) -> VerifyResult {
let zalsa = db.zalsa();
let data = <super::IngredientImpl<C>>::data(zalsa.table(), input);
let field_changed_at = data.revisions[self.field_index].load();
let field_changed_at = C::field_revision(&data.revisions, self.field_index);
VerifyResult::changed_if(field_changed_at > revision)
}

6
src/update.rs
View file

@ -8,7 +8,7 @@ use std::path::PathBuf;
#[cfg(feature = "rayon")]
use rayon::iter::Either;
use crate::revision::AtomicRevision;
use crate::revision::MaybeAtomicRevision;
use crate::sync::Arc;
use crate::Revision;
@ -105,12 +105,12 @@ where
/// and updates `*old_revision` with `new_revision.` Used for fields
/// tagged with `#[no_eq]`
pub fn always_update<T>(
old_revision: &mut AtomicRevision,
old_revision: &mut MaybeAtomicRevision,
new_revision: Revision,
old_pointer: &mut T,
new_value: T,
) {
old_revision.store(new_revision);
old_revision.non_atomic_store(new_revision);
*old_pointer = new_value;
}