Slot no more

Cargo.toml

@@ -11,6 +11,10 @@ description = "A generic framework for on-demand, incrementalized computation (e
 name = "salsa"
 
 [dependencies]
+arc-swap = "1.4.0"
+crossbeam-utils = { version = "0.8", default-features = false }
+dashmap = "5.5.3"
+hashlink = "0.8.4"
 indexmap = "2.1.0"
 lock_api = "0.4"
 log = "0.4.5"
@@ -19,15 +23,15 @@ rustc-hash = "1.0"
 smallvec = "1.0.0"
 oorandom = "11"
 
-rust-analyzer-salsa-macros = { version = "0.17.0-pre.2", path = "components/salsa-macros" }
+rust-analyzer-salsa-macros = { version = "0.17.0-pre.4", path = "components/salsa-macros" }
 
 [dev-dependencies]
 diff = "0.1.0"
 env_logger = "0.7"
 linked-hash-map = "0.5.2"
 rand = "0.7"
 rand_distr = "0.2.1"
-test-env-log = "0.2.7"
+test-log = "0.2.14"
 insta = "1.8.0"
 
 [workspace]

book/netlify.sh

@@ -0,0 +1,16 @@
+#!/bin/bash
+#
+# Script meant to be run from netlify
+
+set -x
+
+MDBOOK_VERSION='0.4.12'
+MDBOOK_LINKCHECK_VERSION='0.7.4'
+MDBOOK_MERMAID_VERSION='0.8.3'
+
+curl -L https://github.com/rust-lang/mdBook/releases/download/v$MDBOOK_VERSION/mdbook-v$MDBOOK_VERSION-x86_64-unknown-linux-gnu.tar.gz | tar xz -C ~/.cargo/bin
+curl -L https://github.com/badboy/mdbook-mermaid/releases/download/v$MDBOOK_MERMAID_VERSION/mdbook-mermaid-v$MDBOOK_MERMAID_VERSION-x86_64-unknown-linux-gnu.tar.gz | tar xz -C ~/.cargo/bin
+curl -L https://github.com/Michael-F-Bryan/mdbook-linkcheck/releases/download/v$MDBOOK_LINKCHECK_VERSION/mdbook-linkcheck.v$MDBOOK_LINKCHECK_VERSION.x86_64-unknown-linux-gnu.zip -O
+unzip mdbook-linkcheck.v$MDBOOK_LINKCHECK_VERSION.x86_64-unknown-linux-gnu.zip -d ~/.cargo/bin
+chmod +x ~/.cargo/bin/mdbook-linkcheck
+mdbook build

book/src/SUMMARY.md

@@ -8,6 +8,7 @@
 - [Common patterns](./common_patterns.md)
   - [Selection](./common_patterns/selection.md)
   - [On-demand (Lazy) inputs](./common_patterns/on_demand_inputs.md)
+- [Tuning](./tuning.md)
 - [Cycle handling](./cycles.md)
   - [Recovering via fallback](./cycles/fallback.md)
 
@@ -54,6 +55,7 @@
   - [RFC 0007: Opinionated cancelation](./rfcs/RFC0007-Opinionated-Cancelation.md)
   - [RFC 0008: Remove garbage collection](./rfcs/RFC0008-Remove-Garbage-Collection.md)
   - [RFC 0009: Cycle recovery](./rfcs/RFC0009-Cycle-recovery.md)
+  - [RFC 0010: Slot no more](./rfcs/RFC0010-Slot-no-more.md)
 
 # Appendices
 

book/src/cycles.md

@@ -1,3 +1,3 @@
 # Cycle handling
 
-By default, when Salsa detects a cycle in the computation graph, Salsa will panic with a [`salsa::Cycle`] as the panic value. The [`salsa::Cycle`] structure that describes the cycle, which can be useful for diagnosing what went wrong. 
+By default, when Salsa detects a cycle in the computation graph, Salsa will panic with a `salsa::Cycle` as the panic value. The `salsa::Cycle` structure that describes the cycle, which can be useful for diagnosing what went wrong. 

book/src/rfcs/RFC0007-Opinionated-Cancelation.md

@@ -28,18 +28,18 @@ We propose to make this model of cancelation the *only* model of cancelation.
 
 ## User's guide
 
-When you do a write to the salsa database, that write will block until any queries running in background threads have completed. You really want those queries to complete quickly, though, because they are now operating on stale data and their results are therefore not meaningful. To expedite the process, salsa will *cancel* those queries. That means that the queries will panic as soon as they try to execute another salsa query. Those panics occur using a sentinel value that you can check for if you wish. If you have a query that contains a long loop which does not execute any intermediate queries, salsa won't be able to cancel it automatically. You may wish to check for cancelation yourself by invoking the `unwind_if_canceled` method.
+When you do a write to the salsa database, that write will block until any queries running in background threads have completed. You really want those queries to complete quickly, though, because they are now operating on stale data and their results are therefore not meaningful. To expedite the process, salsa will *cancel* those queries. That means that the queries will panic as soon as they try to execute another salsa query. Those panics occur using a sentinel value that you can check for if you wish. If you have a query that contains a long loop which does not execute any intermediate queries, salsa won't be able to cancel it automatically. You may wish to check for cancelation yourself by invoking the `unwind_if_cancelled` method.
 
 ## Reference guide
 
 The changes required to implement this RFC are as follows:
 
 * Remove on `is_current_revision_canceled`.
 * Introduce a sentinel cancellation token that can be used with [`resume_unwind`](https://doc.rust-lang.org/std/panic/fn.resume_unwind.html)
-* Introduce a `unwind_if_canceled` method into the `Database` which checks whether cancelation has occured and panics if so.
+* Introduce a `unwind_if_cancelled` method into the `Database` which checks whether cancelation has occured and panics if so.
     * This method also triggers a `salsa_event` callback.
     * This should probably be inline for the `if` with an outlined function to do the actual panic.
-* Modify the code for the various queries to invoke `unwind_if_canceled` when they are invoked or validated.
+* Modify the code for the various queries to invoke `unwind_if_cancelled` when they are invoked or validated.
 
 ## Frequently asked questions
 

book/src/rfcs/RFC0010-Slot-no-more.md

@@ -0,0 +1,175 @@
+# Parallel friendly caching
+
+## Metadata
+
+* Author: nikomatsakis
+* Date: 2021-05-29
+* Introduced in: (please update once you open your PR)
+
+## Summary
+
+* Rework query storage to be based on concurrent hashmaps instead of slots with read-write locked state.
+
+## Motivation
+
+Two-fold:
+
+* Simpler, cleaner, and hopefully faster algorithm.
+* Enables some future developments that are not part of this RFC:
+    * Derived queries whose keys are known to be integers.
+    * Fixed point cycles so that salsa and chalk can be deeply integrated.
+    * Non-synchronized queries that potentially execute on many threads in parallel (required for fixed point cycles, but potentially valuable in their own right).
+
+## User's guide
+
+No user visible changes.
+
+## Reference guide
+
+### Background: Current structure
+
+Before this RFC, the **overall structure** of derived queries is as follows:
+
+* Each derived query has a `DerivedStorage<Q>` (stored in the database) that contains:
+    * the `slot_map`, a monotonically growing, indexable map from keys (`Q::Key`) to the `Slot<Q>` for the given key
+    * lru list
+* Each `Slot<Q>` has
+    * r-w locked query-state that can be:
+        * not-computed
+        * in-progress with synchronization storage:
+            * `id` of the runtime computing the value
+            * `anyone_waiting`: `AtomicBool` set to true if other threads are awaiting result
+        * a `Memo<Q>`
+* A `Memo<Q>` has
+    * an optional value `Option<Q::Value>`
+    * dependency information:
+        * verified-at
+        * changed-at
+        * durability
+        * input set (typically a `Arc<[DatabaseKeyIndex]>`)
+
+[maybe changed after]: ../plumbing/maybe_changed_after.md
+[fetch]: ../plumbing/fetch.md
+
+**Fetching the value for a query** currently works as follows:
+
+* Acquire the read lock on the (indexable) `slot_map` and hash key to find the slot.
+    * If no slot exists, acquire write lock and insert.
+* Acquire the slot's internal lock to perform the [fetch] operation.
+
+**Verifying a dependency** uses a scheme introduced in [RFC #6](./RFC0006-Dynamic-Databases.md). Each dependency is represented as a `DatabaseKeyIndex` which contains three indices (group, query, and key). The group and query indices are used to find the query storage via `match` statements and then the next operation depends on the query type:
+
+* Acquire the read lock on the (indexable) `slot_map` and use key index to load the slot. Read lock is released afterwards.
+* Acquire the slot's internal lock to perform the [maybe changed after] operation.
+
+### New structure (introduced by this RFC)
+
+The **overall structure** of derived queries after this RFC is as follows:
+
+* Each derived query has a `DerivedStorage<Q>` (stored in the database) that contains:
+    * a set of concurrent hashmaps:
+        * `key_map`: maps from `Q::Key` to an internal key index `K`
+        * `memo_map`: maps from `K` to cached memo `ArcSwap<Memo<Q>>`
+        * `sync_map`: maps from `K` to a `Sync<Q>` synchronization value
+    * lru set
+* A `Memo<Q>` has
+    * an *immutable* optional value `Option<Q::Value>`
+    * dependency information:
+        * *updatable* verified-at (`AtomicCell<Option<Revision>>`)
+        * *immutable* changed-at (`Revision`)
+        * *immutable* durability (`Durability`)
+        * *immutable* input set (typically a `Arc<[DatabaseKeyIndex]>`)
+    * information for LRU:
+        * `DatabaseKeyIndex`
+        * `lru_index`, an `AtomicUsize`
+* A `Sync<Q>` has
+    * `id` of the runtime computing the value
+    * `anyone_waiting`: `AtomicBool` set to true if other threads are awaiting result
+
+**Fetching the value for a *derived* query** will work as follows:
+
+1. Find internal index `K` by hashing key, as today.
+    * Precise operation for this will depend on the concurrent hashmap implementation.
+2. Load memo `M: Arc<Memo<Q>>` from `memo_map[K]` (if present):
+    * If verified is `None`, then another thread has found this memo to be invalid; ignore it.
+    * Else, let `Rv` be the "last verified revision".
+    * If `Rv` is the current revision, or last change to an input with durability `M.durability` was before `Rv`:
+        * Update "last verified revision" and **return** memoized value.
+3. Atomically check `sync_map` for an existing `Sync<Q>`:
+    * If one exists, block on the thread within and return to step 2 after it completes:
+        * If this results in a cycle, unwind as today.
+    * If none exists, insert a new entry with current runtime-id.
+4. Check dependencies deeply
+    * Iterate over each dependency `D` and check `db.maybe_changed_after(D, Rv)`.
+        * If no dependency has changed, update `verified_at` to current revision and **return** memoized value.
+    * Mark memo as invalid by storing `None` in the verified-at.
+5. Construct the new memo:
+    * Push query onto the local stack and execute the query function:
+        * If this query is found to be a cycle participant, execute recovery function.
+    * Backdate result if it is equal to the old memo's value.
+    * Allocate new memo.
+6. Store results:
+    * Store new memo into `memo_map[K]`.
+    * Remove query from the `sync_map`.
+7. **Return** newly constructed value._
+
+**Verifying a dependency for a *derived* query** will work as follows:
+
+1. Find internal index `K` by hashing key, as today.
+    * Precise operation for this will depend on the concurrent hashmap implementation.
+2. Load memo `M: Arc<Memo<Q>>` from `memo_map[K]` (if present):
+    * If verified is `None`, then another thread has found this memo to be invalid; ignore it.
+    * Else, let `Rv` be the "last verified revision".
+    * If `Rv` is the current revision, **return** true or false depending on whether changed-at from memo.
+    * If last change to an input with durability `M.durability` was before `Rv`: 
+        * Update `verified_at` to current revision and **return** memoized value.
+    * Iterate over each dependency `D` and check `db.maybe_changed_after(D, Rv)`.
+        * If no dependency has changed, update `verified_at` to current revision and **return** memoized value.
+    * Mark memo as invalid by storing `None` in the verified-at.
+3. Atomically check `sync_map` for an existing `Sync<Q>`:
+    * If one exists, block on the thread within and return to step 2 after it completes:
+        * If this results in a cycle, unwind as today.
+    * If none exists, insert a new entry with current runtime-id.
+4. Construct the new memo:
+    * Push query onto the local stack and execute the query function:
+        * If this query is found to be a cycle participant, execute recovery function.
+    * Backdate result if it is equal to the old memo's value.
+    * Allocate new memo.
+5. Store results:
+    * Store new memo into `memo_map[K]`.
+    * Remove query from the `sync_map`.
+6. **Return** true or false depending on whether memo was backdated.
+
+## Frequently asked questions
+
+### Why use `ArcSwap`?
+
+It's a relatively minor implementation detail, but the code in this PR uses `ArcSwap` to store the values in the memo-map. In the case of a cache hit or other transient operations, this allows us to read from the arc while avoiding a full increment of the ref count. It adds a small bit of complexity because we have to be careful to do a full load before any recursive operations, since arc-swap only gives a fixed number of "guards" per thread before falling back to more expensive loads.
+
+### Do we really need `maybe_changed_after` *and* `fetch`?
+
+Yes, we do. "maybe changed after" is very similar to "fetch", but it doesn't require that we have a memoized value. This is important for LRU.
+
+### The LRU map in the code is just a big lock!
+
+That's not a question. But it's true, I simplified the LRU code to just use a mutex. My assumption is that there are relatively few LRU-ified queries, and their values are relatively expensive to compute, so this is ok. If we find it's a bottleneck, though, I believe we could improve it by using a similar "zone scheme" to what we use now. We would add a `lru_index` to the `Memo` so that we can easily check if the memo is in the "green zone" when reading (if so, no updates are needed). The complexity there is that when we produce a replacement memo, we have to install it and swap the index. Thinking about that made my brain hurt a little so I decided to just take the simple option for now.
+
+### How do the synchronized / atomic operations compare after this RFC?
+
+After this RFC, to perform a read, in the best case:
+
+* We do one "dashmap get" to map key to key index.
+* We do another "dashmap get" from key index to memo.
+* We do an "arcswap load" to get the memo.
+* We do an "atomiccell read" to load the current revision or durability information.
+
+dashmap is implemented with a striped set of read-write locks, so this is roughly the same (two read locks) as before this RFC. However:
+
+* We no longer do any atomic ref count increments.
+* It is theoretically possible to replace dashmap with something that doesn't use locks.
+* The first dashmap get should be removable, if we know that the key is a 32 bit integer.
+    * I plan to propose this in a future RFC.
+
+### Yeah yeah, show me some benchmarks!
+
+I didn't run any. I'll get on that.

book/src/tuning.md

@@ -0,0 +1,53 @@
+# Tuning Salsa
+
+## LRU Cache
+
+You can specify an LRU cache size for any non-input query:
+
+```rs
+let lru_capacity: usize = 128;
+base_db::ParseQuery.in_db_mut(self).set_lru_capacity(lru_capacity);
+```
+
+The default is `0`, which disables LRU-caching entirely.
+
+See [The LRU RFC for more details](./rfcs/RFC0004-LRU.md).
+
+Note that there is no garbage collection for keys and
+results of old queries, so LRU caches are currently the
+only knob available for avoiding unbounded memory usage
+for long-running apps built on Salsa.
+
+## Intern Queries
+
+Intern queries can make key lookup cheaper, save memory, and
+avoid the need for [`Arc`](https://doc.rust-lang.org/std/sync/struct.Arc.html).
+
+Interning is especially useful for queries that involve nested,
+tree-like data structures.
+
+See:
+- The [Intern Queries RFC](./rfcs/RFC0002-Intern-Queries.md)
+- The [`compiler` example](https://github.com/salsa-rs/salsa/blob/master/examples/compiler/main.rs),
+which uses interning.
+
+## Granularity of Incrementality
+
+See:
+- [common patterns: selection](./common_patterns/selection.md) and
+- The [`selection` example](https://github.com/salsa-rs/salsa/blob/master/examples/selection/main.rs)
+
+## Cancellation
+
+Queries that are no longer needed due to concurrent writes or changes in dependencies are cancelled
+by Salsa. Each accesss of an intermediate query is a potential cancellation point. cancellation is
+implemented via panicking, and Salsa internals are intended to be panic-safe.
+
+If you have a query that contains a long loop which does not execute any intermediate queries,
+salsa won't be able to cancel it automatically. You may wish to check for cancellation yourself
+by invoking `db.unwind_if_cancelled()`.
+
+For more details on cancellation, see:
+- [the Opinionated cancellation RFC](./rfcs/RFC0007-Opinionated-Cancelation.md).
+- The tests for cancellation behavior in the Salsa repo.
+

components/salsa-macros/src/database_storage.rs

@@ -66,6 +66,11 @@ pub(crate) fn database(args: TokenStream, input: TokenStream) -> TokenStream {
                 fn group_storage(&self) -> &#group_storage {
                     &self.#db_storage_field.query_store().#group_name_snake
                 }
+
+                fn group_storage_mut(&mut self) -> (&#group_storage, &mut salsa::Runtime) {
+                    let (query_store_mut, runtime) = self.#db_storage_field.query_store_mut();
+                    (&query_store_mut.#group_name_snake, runtime)
+                }
             }
         });
         // ANCHOR_END:HasQueryGroup

components/salsa-macros/src/query_group.rs

@@ -43,7 +43,8 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream
             let mut cycle = None;
             let mut invoke = None;
 
-            let mut query_type = format_ident!("{}Query", query_name.to_string().to_upper_camel_case());
+            let mut query_type =
+                format_ident!("{}Query", query_name.to_string().to_upper_camel_case());
             let mut num_storages = 0;
 
             // Extract attributes.
@@ -163,8 +164,10 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream
             //
             //     fn lookup_foo(&self, x: u32) -> (Key1, Key2)
             let lookup_query = if let QueryStorage::Interned = storage {
-                let lookup_query_type =
-                    format_ident!("{}LookupQuery", query_name.to_string().to_upper_camel_case());
+                let lookup_query_type = format_ident!(
+                    "{}LookupQuery",
+                    query_name.to_string().to_upper_camel_case()
+                );
                 let lookup_fn_name = format_ident!("lookup_{}", query_name);
                 let keys = keys.iter().map(|(_, ty)| ty);
                 let lookup_value: Type = parse_quote!((#(#keys),*));
@@ -463,7 +466,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream
 
                 fn query_storage<'a>(
                     group_storage: &'a <Self as salsa::QueryDb<'_>>::GroupStorage,
-                ) -> &'a std::sync::Arc<Self::Storage> {
+                ) -> &'a Self::Storage {
                     &group_storage.#fn_name
                 }
             }