Allow triggering LRU eviction without increasing the current revision #685
Conversation
✅ Deploy Preview for salsa-rs canceled.
|
CodSpeed Performance ReportMerging #685 will not alter performanceComparing Summary
|
Veykril
force-pushed
the
veykril/push-lzwyywmqwmuk
branch
2 times, most recently
from
da47143 to
077978d
Compare
Veykril
changed the title
Veykril
force-pushed
the
veykril/push-lzwyywmqwmuk
branch
from
e3cdab6 to
8c83055
Compare
| /// Sets the lru capacity | ||
| /// | ||
| /// **WARNING:** Just like an ordinary write, this method triggers | ||
| /// cancellation. If you invoke it while a snapshot exists, it | ||
| /// will block until that snapshot is dropped -- if that snapshot | ||
| /// is owned by the current thread, this could trigger deadlock. |
There was a problem hiding this comment.
I am a bit iffy about how our cancellation triggering functions all need this comment. I wonder if it would make sense to introduce a CancellationToken type (not to be confused with async cancellation tokens...) that one would need to supply to acknowledge the deadlock conditions as such an API forces the caller to think about this, where as the current status quo is having to have read the docs of the function.
There was a problem hiding this comment.
I worry that a CancellationToken would be annoying because you'd have to pass it to every function taking a &mut db.
It would be nice if our concurrent db design wouldn't even permit having multiple &mut db checkers by relying on the borrow checker.
There was a problem hiding this comment.
Right, that would infest all the input setters I believe hmm
Veykril
force-pushed
the
veykril/push-lzwyywmqwmuk
branch
from
8c83055 to
eec8181
Compare
Veykril
force-pushed
the
veykril/push-lzwyywmqwmuk
branch
from
eec8181 to
2927a4a
Compare
src/zalsa.rs
Outdated
| @@ -45,6 +45,8 @@ pub unsafe trait ZalsaDatabase: Any { | |||
| /// | |||
| /// **WARNING:** Triggers cancellation to other database handles. | |||
| /// This can lead to deadlock! | |||
| /// | |||
| /// Note that this needs to be paired with a call to `Zalsa::new_revision` or `Zalsa::clear_cancellation_flag`. | |||
There was a problem hiding this comment.
This comment I think is on the wrong commit?
|
In r-a we basically want to trigger LRU if no cancellation have occurred for some while as the LRU won't trigger if no inputs change anymore, this gives us the flexibility to do that |
Best reviewed by commit as it contains a couple cleanup commits