push review randomizing into SQL

This makes the review backlog case more expensive, since we end up
shuffling items outside the daily limit, but for the common case it's
about the same speed, and it means we don't need two separate sorting
steps. New cards remain handled the same way, since a backlog
is common there.

Also ensures that interday learning cards honor the deck sorting, and
that the non-default sort orders shuffle at the end.

rslib/src/scheduler/queue/builder/mod.rs

@@ -116,7 +116,6 @@ impl QueueBuilder {
         current_day: u32,
     ) -> CardQueues {
         self.sort_new();
-        self.sort_reviews(current_day);
 
         // intraday learning
         let learning = sort_learning(self.learning);

rslib/src/scheduler/queue/builder/sorting.rs

@@ -5,7 +5,7 @@ use std::{cmp::Ordering, hash::Hasher};
 
 use fnv::FnvHasher;
 
-use super::{DueCard, NewCard, NewCardSortOrder, QueueBuilder, ReviewCardOrder};
+use super::{NewCard, NewCardSortOrder, QueueBuilder};
 
 impl QueueBuilder {
     pub(super) fn sort_new(&mut self) {
@@ -24,19 +24,6 @@ impl QueueBuilder {
             }
         }
     }
-
-    pub(super) fn sort_reviews(&mut self, _current_day: u32) {
-        self.day_learning
-            .iter_mut()
-            .for_each(DueCard::hash_id_and_mtime);
-        self.day_learning.sort_unstable_by(day_then_hash);
-
-        // other sorting is done in SQL
-        if self.sort_options.review_order == ReviewCardOrder::Day {
-            self.review.iter_mut().for_each(DueCard::hash_id_and_mtime);
-            self.review.sort_unstable_by(day_then_hash);
-        }
-    }
 }
 
 fn template_then_due(a: &NewCard, b: &NewCard) -> Ordering {
@@ -55,28 +42,10 @@ fn new_hash(a: &NewCard, b: &NewCard) -> Ordering {
     a.hash.cmp(&b.hash)
 }
 
-fn day_then_hash(a: &DueCard, b: &DueCard) -> Ordering {
-    (a.due, a.hash).cmp(&(b.due, b.hash))
-}
-
 // We sort based on a hash so that if the queue is rebuilt, remaining
 // cards come back in the same approximate order (mixing + due learning cards
 // may still result in a different card)
 
-impl DueCard {
-    fn hash_id_and_mtime(&mut self) {
-        let mut hasher = FnvHasher::default();
-        hasher.write_i64(self.id.0);
-        hasher.write_i64(self.mtime.0);
-        self.hash = hasher.finish();
-    }
-
-    #[allow(dead_code)]
-    fn set_hash_to_relative_overdue(&mut self, _current_day: u32) {
-        todo!()
-    }
-}
-
 impl NewCard {
     fn hash_id_and_mtime(&mut self) {
         let mut hasher = FnvHasher::default();

rslib/src/storage/card/mod.rs

@@ -199,17 +199,7 @@ impl super::SqliteStorage {
     where
         F: FnMut(CardQueue, DueCard) -> bool,
     {
-        let order_clause = match order {
+        let order_clause = review_order_sql(order);
-            ReviewCardOrder::Day => "due",
-            ReviewCardOrder::DayThenDeck => {
-                "due, (select rowid from active_decks ad where ad.id = did)"
-            }
-            ReviewCardOrder::DeckThenDay => {
-                "(select rowid from active_decks ad where ad.id = did), due"
-            }
-            ReviewCardOrder::IntervalsAscending => "ivl asc",
-            ReviewCardOrder::IntervalsDescending => "ivl desc",
-        };
         let mut stmt = self.db.prepare_cached(&format!(
             "{} order by {}",
             include_str!("due_cards.sql"),
@@ -552,6 +542,44 @@ impl super::SqliteStorage {
     }
 }
 
+#[derive(Clone, Copy)]
+enum ReviewOrderSubclause {
+    Day,
+    Deck,
+    Random,
+    IntervalsAscending,
+    IntervalsDescending,
+}
+
+impl ReviewOrderSubclause {
+    fn to_str(self) -> &'static str {
+        match self {
+            ReviewOrderSubclause::Day => "due",
+            ReviewOrderSubclause::Deck => "(select rowid from active_decks ad where ad.id = did)",
+            ReviewOrderSubclause::Random => "fnvhash(id, mod)",
+            ReviewOrderSubclause::IntervalsAscending => "ivl asc",
+            ReviewOrderSubclause::IntervalsDescending => "ivl desc",
+        }
+    }
+}
+
+fn review_order_sql(order: ReviewCardOrder) -> String {
+    let mut subclauses = match order {
+        ReviewCardOrder::Day => vec![ReviewOrderSubclause::Day],
+        ReviewCardOrder::DayThenDeck => vec![ReviewOrderSubclause::Day, ReviewOrderSubclause::Deck],
+        ReviewCardOrder::DeckThenDay => vec![ReviewOrderSubclause::Deck, ReviewOrderSubclause::Day],
+        ReviewCardOrder::IntervalsAscending => vec![ReviewOrderSubclause::IntervalsAscending],
+        ReviewCardOrder::IntervalsDescending => vec![ReviewOrderSubclause::IntervalsDescending],
+    };
+    subclauses.push(ReviewOrderSubclause::Random);
+
+    let v: Vec<_> = subclauses
+        .into_iter()
+        .map(ReviewOrderSubclause::to_str)
+        .collect();
+    v.join(", ")
+}
+
 #[cfg(test)]
 mod test {
     use std::path::Path;

rslib/src/storage/sqlite.rs

@@ -1,8 +1,9 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
-use std::{borrow::Cow, cmp::Ordering, path::Path, sync::Arc};
+use std::{borrow::Cow, cmp::Ordering, hash::Hasher, path::Path, sync::Arc};
 
+use fnv::FnvHasher;
 use regex::Regex;
 use rusqlite::{functions::FunctionFlags, params, Connection, NO_PARAMS};
 use unicase::UniCase;
@@ -51,6 +52,7 @@ fn open_or_create_collection_db(path: &Path) -> Result<Connection> {
     add_field_index_function(&db)?;
     add_regexp_function(&db)?;
     add_without_combining_function(&db)?;
+    add_fnvhash_function(&db)?;
 
     db.create_collation("unicase", unicase_compare)?;
 
@@ -88,6 +90,16 @@ fn add_without_combining_function(db: &Connection) -> rusqlite::Result<()> {
     )
 }
 
+fn add_fnvhash_function(db: &Connection) -> rusqlite::Result<()> {
+    db.create_scalar_function("fnvhash", -1, FunctionFlags::SQLITE_DETERMINISTIC, |ctx| {
+        let mut hasher = FnvHasher::default();
+        for idx in 0..ctx.len() {
+            hasher.write_i64(ctx.get(idx)?);
+        }
+        Ok(hasher.finish() as i64)
+    })
+}
+
 /// Adds sql function regexp(regex, string) -> is_match
 /// Taken from the rusqlite docs
 type BoxError = Box<dyn std::error::Error + Send + Sync + 'static>;