Skip to content

Commit dfbfead

Browse files
VeykrilVeykril
committed
Do not Option wrap Memo::value when no LRU is required
1 parent 29331c8 commit dfbfead

File tree

10 files changed

+213
-74
lines changed

10 files changed

+213
-74
lines changed

Diff for: ‎src/function.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -160,17 +160,17 @@ where
160160
/// only cleared with `&mut self`.
161161
unsafe fn extend_memo_lifetime<'this>(
162162
&'this self,
163-
memo: &memo::Memo<C::Output<'this>>,
164-
) -> &'this memo::Memo<C::Output<'this>> {
163+
memo: &memo::Memo<C::Lru, C::Output<'this>>,
164+
) -> &'this memo::Memo<C::Lru, C::Output<'this>> {
165165
std::mem::transmute(memo)
166166
}
167167

168168
fn insert_memo<'db>(
169169
&'db self,
170170
zalsa: &'db Zalsa,
171171
id: Id,
172-
memo: memo::Memo<C::Output<'db>>,
173-
) -> &'db memo::Memo<C::Output<'db>> {
172+
memo: memo::Memo<C::Lru, C::Output<'db>>,
173+
) -> &'db memo::Memo<C::Lru, C::Output<'db>> {
174174
let memo = Arc::new(memo);
175175
let db_memo = unsafe {
176176
// Unsafety conditions: memo must be in the map (it's not yet, but it will be by the time this
@@ -189,6 +189,7 @@ where
189189
impl<C> Ingredient for IngredientImpl<C>
190190
where
191191
C: Configuration,
192+
for<'lt> <C::Lru as LruChoice>::LruCtor<<C as Configuration>::Output<'lt>>: Send + Sync,
192193
{
193194
fn ingredient_index(&self) -> IngredientIndex {
194195
self.index

Diff for: ‎src/function/backdate.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::zalsa_local::QueryRevisions;
22

3-
use super::{memo::Memo, Configuration, IngredientImpl};
3+
use super::{memo::Memo, Configuration, IngredientImpl, LruChoice};
44

55
impl<C> IngredientImpl<C>
66
where
@@ -11,11 +11,11 @@ where
1111
/// on an old memo when a new memo has been produced to check whether there have been changed.
1212
pub(super) fn backdate_if_appropriate(
1313
&self,
14-
old_memo: &Memo<C::Output<'_>>,
14+
old_memo: &Memo<C::Lru, C::Output<'_>>,
1515
revisions: &mut QueryRevisions,
1616
value: &C::Output<'_>,
1717
) {
18-
if let Some(old_value) = &old_memo.value {
18+
C::Lru::with_value(&old_memo.value, |old_value| {
1919
// Careful: if the value became less durable than it
2020
// used to be, that is a "breaking change" that our
2121
// consumers must be aware of. Becoming *more* durable
@@ -31,6 +31,6 @@ where
3131
assert!(old_memo.revisions.changed_at <= revisions.changed_at);
3232
revisions.changed_at = old_memo.revisions.changed_at;
3333
}
34-
}
34+
})
3535
}
3636
}

Diff for: ‎src/function/diff_outputs.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ where
1717
&self,
1818
db: &C::DbView,
1919
key: DatabaseKeyIndex,
20-
old_memo: &Memo<C::Output<'_>>,
20+
old_memo: &Memo<C::Lru, C::Output<'_>>,
2121
revisions: &mut QueryRevisions,
2222
) {
2323
// Iterate over the outputs of the `old_memo` and put them into a hashset

Diff for: ‎src/function/execute.rs

+8-4
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::{
44
zalsa::ZalsaDatabase, zalsa_local::ActiveQueryGuard, Cycle, Database, Event, EventKind,
55
};
66

7-
use super::{memo::Memo, Configuration, IngredientImpl};
7+
use super::{memo::Memo, Configuration, IngredientImpl, LruChoice};
88

99
impl<C> IngredientImpl<C>
1010
where
@@ -23,8 +23,8 @@ where
2323
&'db self,
2424
db: &'db C::DbView,
2525
active_query: ActiveQueryGuard<'_>,
26-
opt_old_memo: Option<Arc<Memo<C::Output<'_>>>>,
27-
) -> &'db Memo<C::Output<'db>> {
26+
opt_old_memo: Option<Arc<Memo<C::Lru, C::Output<'_>>>>,
27+
) -> &'db Memo<C::Lru, C::Output<'db>> {
2828
let zalsa = db.zalsa();
2929
let revision_now = zalsa.current_revision();
3030
let database_key_index = active_query.database_key_index;
@@ -84,6 +84,10 @@ where
8484

8585
tracing::debug!("{database_key_index:?}: read_upgrade: result.revisions = {revisions:#?}");
8686

87-
self.insert_memo(zalsa, id, Memo::new(Some(value), revision_now, revisions))
87+
self.insert_memo(
88+
zalsa,
89+
id,
90+
Memo::new(C::Lru::make_value(value), revision_now, revisions),
91+
)
8892
}
8993
}

Diff for: ‎src/function/fetch.rs

+18-6
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ where
1717
value,
1818
durability,
1919
changed_at,
20-
} = memo.revisions.stamped_value(memo.value.as_ref().unwrap());
20+
} = memo
21+
.revisions
22+
.stamped_value(C::Lru::assert_ref(&memo.value));
2123

2224
self.lru.record_use(id);
2325

@@ -39,7 +41,7 @@ where
3941
&'db self,
4042
db: &'db C::DbView,
4143
id: Id,
42-
) -> &'db Memo<C::Output<'db>> {
44+
) -> &'db Memo<C::Lru, C::Output<'db>> {
4345
loop {
4446
if let Some(memo) = self.fetch_hot(db, id).or_else(|| self.fetch_cold(db, id)) {
4547
return memo;
@@ -48,11 +50,15 @@ where
4850
}
4951

5052
#[inline]
51-
fn fetch_hot<'db>(&'db self, db: &'db C::DbView, id: Id) -> Option<&'db Memo<C::Output<'db>>> {
53+
fn fetch_hot<'db>(
54+
&'db self,
55+
db: &'db C::DbView,
56+
id: Id,
57+
) -> Option<&'db Memo<C::Lru, C::Output<'db>>> {
5258
let zalsa = db.zalsa();
5359
let memo_guard = self.get_memo_from_table_for(zalsa, id);
5460
if let Some(memo) = &memo_guard {
55-
if memo.value.is_some()
61+
if !C::Lru::is_evicted(&memo.value)
5662
&& self.shallow_verify_memo(db, zalsa, self.database_key_index(id), memo)
5763
{
5864
// Unsafety invariant: memo is present in memo_map and we have verified that it is
@@ -63,7 +69,11 @@ where
6369
None
6470
}
6571

66-
fn fetch_cold<'db>(&'db self, db: &'db C::DbView, id: Id) -> Option<&'db Memo<C::Output<'db>>> {
72+
fn fetch_cold<'db>(
73+
&'db self,
74+
db: &'db C::DbView,
75+
id: Id,
76+
) -> Option<&'db Memo<C::Lru, C::Output<'db>>> {
6777
let (zalsa, zalsa_local) = db.zalsas();
6878
let database_key_index = self.database_key_index(id);
6979

@@ -82,7 +92,9 @@ where
8292
let zalsa = db.zalsa();
8393
let opt_old_memo = self.get_memo_from_table_for(zalsa, id);
8494
if let Some(old_memo) = &opt_old_memo {
85-
if old_memo.value.is_some() && self.deep_verify_memo(db, old_memo, &active_query) {
95+
if !C::Lru::is_evicted(&old_memo.value)
96+
&& self.deep_verify_memo(db, old_memo, &active_query)
97+
{
8698
// Unsafety invariant: memo is present in memo_map and we have verified that it is
8799
// still valid for the current revision.
88100
return unsafe { Some(self.extend_memo_lifetime(old_memo)) };

Diff for: ‎src/function/lru.rs

+100
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,36 @@ mod sealed {
88
}
99

1010
pub trait LruChoice: sealed::Sealed + Default {
11+
type LruCtor<T>: Send + Sync
12+
where
13+
T: Send + Sync;
1114
/// Records the `index` into this LRU, returning the index to evict if any.
1215
fn record_use(&self, index: Id);
1316
/// Fetches all elements that should be evicted.
1417
fn to_be_evicted(&self, cb: impl FnMut(Id));
1518
/// Sets the capacity of the LRU.
1619
fn set_capacity(&self, capacity: usize);
20+
fn if_enabled(f: impl FnOnce());
21+
22+
fn evicted<T>() -> Self::LruCtor<T>
23+
where
24+
T: Send + Sync;
25+
26+
fn is_evicted<T>(it: &Self::LruCtor<T>) -> bool
27+
where
28+
T: Send + Sync;
29+
30+
fn assert_ref<T>(v: &Self::LruCtor<T>) -> &T
31+
where
32+
T: Send + Sync;
33+
34+
fn make_value<T>(v: T) -> Self::LruCtor<T>
35+
where
36+
T: Send + Sync;
37+
38+
fn with_value<T>(v: &Self::LruCtor<T>, cb: impl FnOnce(&T))
39+
where
40+
T: Send + Sync;
1741
}
1842

1943
/// An LRU choice that does not evict anything.
@@ -22,9 +46,45 @@ pub struct NoLru;
2246

2347
impl sealed::Sealed for NoLru {}
2448
impl LruChoice for NoLru {
49+
type LruCtor<T>
50+
= T
51+
where
52+
T: Send + Sync;
2553
fn record_use(&self, _: Id) {}
2654
fn to_be_evicted(&self, _: impl FnMut(Id)) {}
2755
fn set_capacity(&self, _: usize) {}
56+
fn if_enabled(_: impl FnOnce()) {}
57+
58+
fn evicted<T>() -> Self::LruCtor<T>
59+
where
60+
T: Send + Sync,
61+
{
62+
unreachable!("NoLru::evicted should never be called")
63+
}
64+
fn is_evicted<T>(_: &Self::LruCtor<T>) -> bool
65+
where
66+
T: Send + Sync,
67+
{
68+
false
69+
}
70+
fn assert_ref<T>(v: &Self::LruCtor<T>) -> &T
71+
where
72+
T: Send + Sync,
73+
{
74+
v
75+
}
76+
fn make_value<T>(v: T) -> Self::LruCtor<T>
77+
where
78+
T: Send + Sync,
79+
{
80+
v
81+
}
82+
fn with_value<T>(v: &Self::LruCtor<T>, cb: impl FnOnce(&T))
83+
where
84+
T: Send + Sync,
85+
{
86+
cb(v);
87+
}
2888
}
2989

3090
/// An LRU choice that tracks elements but does not evict on its own.
@@ -38,6 +98,10 @@ pub struct Lru {
3898

3999
impl sealed::Sealed for Lru {}
40100
impl LruChoice for Lru {
101+
type LruCtor<T>
102+
= Option<T>
103+
where
104+
T: Send + Sync;
41105
fn record_use(&self, index: Id) {
42106
self.set.lock().insert(index);
43107
}
@@ -60,4 +124,40 @@ impl LruChoice for Lru {
60124
*self.set.lock() = FxLinkedHashSet::default();
61125
}
62126
}
127+
fn if_enabled(f: impl FnOnce()) {
128+
f();
129+
}
130+
131+
fn evicted<T>() -> Self::LruCtor<T>
132+
where
133+
T: Send + Sync,
134+
{
135+
None
136+
}
137+
fn is_evicted<T>(it: &Self::LruCtor<T>) -> bool
138+
where
139+
T: Send + Sync,
140+
{
141+
it.is_none()
142+
}
143+
fn assert_ref<T>(v: &Self::LruCtor<T>) -> &T
144+
where
145+
T: Send + Sync,
146+
{
147+
v.as_ref().unwrap()
148+
}
149+
fn make_value<T>(v: T) -> Self::LruCtor<T>
150+
where
151+
T: Send + Sync,
152+
{
153+
Some(v)
154+
}
155+
fn with_value<T>(v: &Self::LruCtor<T>, cb: impl FnOnce(&T))
156+
where
157+
T: Send + Sync,
158+
{
159+
if let Some(v) = v {
160+
cb(v);
161+
}
162+
}
63163
}

Diff for: ‎src/function/maybe_changed_after.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{
77
AsDynDatabase as _, Id, Revision,
88
};
99

10-
use super::{memo::Memo, Configuration, IngredientImpl};
10+
use super::{memo::Memo, Configuration, IngredientImpl, LruChoice};
1111

1212
impl<C> IngredientImpl<C>
1313
where
@@ -91,7 +91,7 @@ where
9191
// It is possible the result will be equal to the old value and hence
9292
// backdated. In that case, although we will have computed a new memo,
9393
// the value has not logically changed.
94-
if old_memo.value.is_some() {
94+
if !C::Lru::is_evicted(&old_memo.value) {
9595
let memo = self.execute(db, active_query, Some(old_memo));
9696
let changed_at = memo.revisions.changed_at;
9797

@@ -117,7 +117,7 @@ where
117117
db: &C::DbView,
118118
zalsa: &Zalsa,
119119
database_key_index: DatabaseKeyIndex,
120-
memo: &Memo<C::Output<'_>>,
120+
memo: &Memo<C::Lru, C::Output<'_>>,
121121
) -> bool {
122122
let verified_at = memo.verified_at.load();
123123
let revision_now = zalsa.current_revision();
@@ -159,7 +159,7 @@ where
159159
pub(super) fn deep_verify_memo(
160160
&self,
161161
db: &C::DbView,
162-
old_memo: &Memo<C::Output<'_>>,
162+
old_memo: &Memo<C::Lru, C::Output<'_>>,
163163
active_query: &ActiveQueryGuard<'_>,
164164
) -> bool {
165165
let zalsa = db.zalsa();

0 commit comments

Comments
 (0)