From 78106dc14ba22f94e1d5a8f438e8bee5caa1c0cc Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Mon, 20 Jun 2022 19:42:22 +1000 Subject: [PATCH] Speed up deck list by rendering it with the backend This delegates to the backend to render the deck tree, instead of calculating it in Java. On @Arthur-Milchior's large collection with the daily limits turned up, the speed difference in an x86_64 sim is dramatic: 2.4s with the old Java code, and 70ms with the new Rust code. - Make deckDueList() private; switch callers to use deckDueTree() The backend only provides a deckDueTree(), and switching the calls to deckDueList() now will make migration easier in the future. Squashed from #11599 --- .../anki/provider/CardContentProvider.kt | 48 ++++++++++------ .../com/ichi2/libanki/sched/AbstractSched.kt | 15 ++--- .../com/ichi2/libanki/sched/BackendSched.kt | 55 +++++++++++++++++++ .../ichi2/libanki/sched/DeckDueTreeNode.kt | 2 + .../java/com/ichi2/libanki/sched/Sched.java | 2 +- .../java/com/ichi2/libanki/sched/SchedV2.java | 35 +++++++----- .../java/com/ichi2/libanki/sync/Syncer.java | 2 +- .../libanki/sched/AbstractSchedTest.java | 2 +- .../com/ichi2/libanki/sched/SchedV2Test.java | 10 +++- 9 files changed, 125 insertions(+), 46 deletions(-) create mode 100644 AnkiDroid/src/main/java/com/ichi2/libanki/sched/BackendSched.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/provider/CardContentProvider.kt b/AnkiDroid/src/main/java/com/ichi2/anki/provider/CardContentProvider.kt index c2acecabd3fa..5d7bb6aeba43 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/provider/CardContentProvider.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/provider/CardContentProvider.kt @@ -37,6 +37,7 @@ import com.ichi2.libanki.backend.exception.DeckRenameException import com.ichi2.libanki.exception.EmptyMediaException import com.ichi2.libanki.sched.AbstractSched import com.ichi2.libanki.sched.DeckDueTreeNode +import com.ichi2.libanki.sched.TreeNode import com.ichi2.libanki.utils.TimeManager import com.ichi2.utils.FileUtil.internalizeUri import com.ichi2.utils.JSONArray @@ -375,28 +376,44 @@ class CardContentProvider : ContentProvider() { rv } DECKS -> { - val allDecks = col.sched.deckDueList() val columns = projection ?: FlashCardsContract.Deck.DEFAULT_PROJECTION - val rv = MatrixCursor(columns, allDecks.size) - for (deck: DeckDueTreeNode? in allDecks) { - val id = deck!!.did - val name = deck.fullDeckName - addDeckToCursor(id, name, getDeckCountsFromDueTreeNode(deck), rv, col, columns) + val allDecks = col.sched.deckDueTree() + val rv = MatrixCursor(columns, 1) + fun forEach(nodeList: List>, fn: (DeckDueTreeNode) -> Unit) { + for (node in nodeList) { + fn(node.value) + forEach(node.children, fn) + } + } + forEach(allDecks) { + addDeckToCursor( + it.did, + it.fullDeckName, + getDeckCountsFromDueTreeNode(it), + rv, + col, + columns + ) } rv } DECKS_ID -> { - /* Direct access deck */ val columns = projection ?: FlashCardsContract.Deck.DEFAULT_PROJECTION val rv = MatrixCursor(columns, 1) - val allDecks = col.sched.deckDueList() - val deckId = uri.pathSegments[1].toLong() - for (deck: DeckDueTreeNode? in allDecks) { - if (deck!!.did == deckId) { - addDeckToCursor(deckId, deck.fullDeckName, getDeckCountsFromDueTreeNode(deck), rv, col, columns) - return rv + val allDecks = col.sched.deckDueTree() + val desiredDeckId = uri.pathSegments[1].toLong() + fun find(nodeList: List>, id: Long): DeckDueTreeNode? { + for (node in nodeList) { + if (node.value.did == id) { + return node.value + } + return find(node.children, id) } + return null + } + find(allDecks, desiredDeckId)?.let { + addDeckToCursor(it.did, it.fullDeckName, getDeckCountsFromDueTreeNode(it), rv, col, columns) } rv } @@ -413,10 +430,9 @@ class CardContentProvider : ContentProvider() { } } - private fun getDeckCountsFromDueTreeNode(deck: DeckDueTreeNode?): JSONArray { - @KotlinCleanup("use a scope function") + private fun getDeckCountsFromDueTreeNode(deck: DeckDueTreeNode): JSONArray { val deckCounts = JSONArray() - deckCounts.put(deck!!.lrnCount) + deckCounts.put(deck.lrnCount) deckCounts.put(deck.revCount) deckCounts.put(deck.newCount) return deckCounts diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/sched/AbstractSched.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/sched/AbstractSched.kt index 3d038b665a6c..654fb8a17844 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/sched/AbstractSched.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/sched/AbstractSched.kt @@ -175,26 +175,23 @@ abstract class AbstractSched(val col: Collection) { */ abstract fun extendLimits(newc: Int, rev: Int) - /** - * @return [deckname, did, rev, lrn, new] - */ - abstract fun deckDueList(): List - /** * @param cancelListener A task that is potentially cancelled - * @return the due tree. null if task is cancelled + * @return the due tree. null only if task is cancelled */ abstract fun deckDueTree(cancelListener: CancelListener?): List>? /** - * @return the due tree. null if task is cancelled. + * @return the due tree. Never null. */ - abstract fun deckDueTree(): List> + fun deckDueTree(): List> { + return deckDueTree(null)!! + } /** * @return The tree of decks, without numbers */ - abstract fun quickDeckDueTree(): List> + abstract fun quickDeckDueTree(): List> /** New count for a single deck. * @param did The deck to consider (descendants and ancestors are ignored) diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/sched/BackendSched.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/sched/BackendSched.kt new file mode 100644 index 000000000000..50aecd2d65a9 --- /dev/null +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/sched/BackendSched.kt @@ -0,0 +1,55 @@ +/*************************************************************************************** + * Copyright (c) 2012 Ankitects Pty Ltd * + * * + * This program is free software; you can redistribute it and/or modify it under * + * the terms of the GNU General Public License as published by the Free Software * + * Foundation; either version 3 of the License, or (at your option) any later * + * version. * + * * + * This program is distributed in the hope that it will be useful, but WITHOUT ANY * + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A * + * PARTICULAR PURPOSE. See the GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License along with * + * this program. If not, see . * + ****************************************************************************************/ + +package com.ichi2.libanki.sched + +import anki.decks.DeckTreeNode +import com.ichi2.libanki.CollectionV16 +import com.ichi2.libanki.utils.TimeManager + +// The desktop code stores these routines in sched/base.py, and all schedulers inherit them. +// The presence of AbstractSched is going to complicate the introduction of the v3 scheduler, +// so for now these are stored in a separate file. + +fun CollectionV16.deckTree(includeCounts: Boolean): DeckTreeNode { + return backend.deckTree(if (includeCounts) TimeManager.time.intTime() else 0) +} + +/** + * Mutate the backend reply into a format expected by legacy code. This is less efficient, + * and AnkiDroid may wish to use .deckTree() in the future instead. + */ +fun CollectionV16.deckTreeLegacy(includeCounts: Boolean): List> { + fun toLegacyNode(node: anki.decks.DeckTreeNode, parentName: String): TreeNode { + val thisName = if (parentName.isEmpty()) { + node.name + } else { + "$parentName::${node.name}" + } + val treeNode = TreeNode( + DeckDueTreeNode( + thisName, + node.deckId, + node.reviewCount, + node.learnCount, + node.newCount, + ) + ) + treeNode.children.addAll(node.childrenList.asSequence().map { toLegacyNode(it, thisName) }) + return treeNode + } + return toLegacyNode(deckTree(includeCounts), "").children +} diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/sched/DeckDueTreeNode.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/sched/DeckDueTreeNode.kt index c3d5d445f976..7e3de4f9b7b6 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/sched/DeckDueTreeNode.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/sched/DeckDueTreeNode.kt @@ -18,6 +18,7 @@ package com.ichi2.libanki.sched import com.ichi2.libanki.Collection import com.ichi2.libanki.Decks import com.ichi2.utils.KotlinCleanup +import net.ankiweb.rsdroid.RustCleanup import java.util.* import kotlin.math.max import kotlin.math.min @@ -34,6 +35,7 @@ import kotlin.math.min */ @KotlinCleanup("maybe possible to remove gettres for revCount/lrnCount") @KotlinCleanup("rename name -> fullDeckName") +@RustCleanup("after migration, consider dropping this and using backend tree structure directly") class DeckDueTreeNode(name: String, did: Long, override var revCount: Int, override var lrnCount: Int, override var newCount: Int) : AbstractDeckTreeNode(name, did) { override fun toString(): String { return String.format( diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/sched/Sched.java b/AnkiDroid/src/main/java/com/ichi2/libanki/sched/Sched.java index d7749e166ce2..c4f0abea3f36 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/sched/Sched.java +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/sched/Sched.java @@ -213,7 +213,7 @@ private void unburyCardsForDeck(@NonNull List allDecks) { * Returns [deckname, did, rev, lrn, new] */ @Override - public @Nullable List deckDueList(@Nullable CancelListener cancelListener) { + protected @Nullable List deckDueList(@Nullable CancelListener cancelListener) { _checkDay(); getCol().getDecks().checkIntegrity(); List allDecksSorted = getCol().getDecks().allSorted(); diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/sched/SchedV2.java b/AnkiDroid/src/main/java/com/ichi2/libanki/sched/SchedV2.java index 0b6e3cae7a60..0c1e369b814f 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/sched/SchedV2.java +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/sched/SchedV2.java @@ -31,6 +31,7 @@ import android.text.style.StyleSpan; import android.util.Pair; +import com.ichi2.anki.AnkiDroidApp; import com.ichi2.anki.R; import com.ichi2.async.CancelListener; import com.ichi2.async.CollectionTask; @@ -57,6 +58,7 @@ import com.ichi2.utils.JSONObject; import com.ichi2.utils.SyncStatus; +import net.ankiweb.rsdroid.BackendFactory; import net.ankiweb.rsdroid.RustCleanup; import net.ankiweb.rsdroid.RustV1Cleanup; @@ -523,14 +525,14 @@ protected int _walkingCount(@NonNull LimitMethod limFn, @NonNull CountMethod cnt * * Return nulls when deck task is cancelled. */ - public @NonNull List deckDueList() { + private @NonNull List deckDueList() { return deckDueList(null); } // Overridden /** * Return sorted list of all decks.*/ - public @Nullable List deckDueList(@Nullable CancelListener collectionTask) { + protected @Nullable List deckDueList(@Nullable CancelListener collectionTask) { _checkDay(); getCol().getDecks().checkIntegrity(); List allDecksSorted = getCol().getDecks().allSorted(); @@ -574,9 +576,11 @@ protected int _walkingCount(@NonNull LimitMethod limFn, @NonNull CountMethod cnt requires multiple database access by deck. Ignoring this number lead to the creation of a tree more quickly.*/ @Override - public @NonNull List> quickDeckDueTree() { - // Similar to deckDueTree, ignoring the numbers - + public @NonNull + List> quickDeckDueTree() { + if (!BackendFactory.getDefaultLegacySchema()) { + return BackendSchedKt.deckTreeLegacy(getCol().getNewBackend(), false); + } // Similar to deckDueList ArrayList allDecksSorted = new ArrayList<>(); for (JSONObject deck : getCol().getDecks().allSorted()) { @@ -588,18 +592,19 @@ protected int _walkingCount(@NonNull LimitMethod limFn, @NonNull CountMethod cnt return _groupChildren(allDecksSorted, false); } - - public @NonNull List> deckDueTree() { - return deckDueTree(null); - } - @Nullable + @RustCleanup("enable for v2 once backend is updated to 2.1.41+") + @RustCleanup("once both v1 and v2 are using backend, cancelListener can be removed") public List> deckDueTree(@Nullable CancelListener cancelListener) { - List allDecksSorted = deckDueList(cancelListener); - if (allDecksSorted == null) { - return null; + if (!BackendFactory.getDefaultLegacySchema()) { + return BackendSchedKt.deckTreeLegacy(getCol().getNewBackend(), true); + } else { + List allDecksSorted = deckDueList(cancelListener); + if (allDecksSorted == null) { + return null; + } + return _groupChildren(allDecksSorted, true); } - return _groupChildren(allDecksSorted, true); } /** @@ -666,7 +671,7 @@ public List> deckDueTree(@Nullable CancelListener canc TreeNode toAdd = new TreeNode<>(child); toAdd.getChildren().addAll(childrenNode); List childValues = childrenNode.stream().map(TreeNode::getValue).collect(Collectors.toList()); - child.processChildren(mCol, childValues, "std".equals(getName())); + child.processChildren(getCol(), childValues, "std".equals(getName())); sortedChildren.add(toAdd); } diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/sync/Syncer.java b/AnkiDroid/src/main/java/com/ichi2/libanki/sync/Syncer.java index fc5f3969e40b..c066ba7f0995 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/sync/Syncer.java +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/sync/Syncer.java @@ -449,7 +449,7 @@ public JSONObject sanityCheck() { mCol.getModels().save(); } // check for missing parent decks - mCol.getSched().deckDueList(); + mCol.getSched().quickDeckDueTree(); // return summary of deck JSONArray check = new JSONArray(); JSONArray counts = new JSONArray(); diff --git a/AnkiDroid/src/test/java/com/ichi2/libanki/sched/AbstractSchedTest.java b/AnkiDroid/src/test/java/com/ichi2/libanki/sched/AbstractSchedTest.java index b1b1430963c0..ae8f788ac2ad 100644 --- a/AnkiDroid/src/test/java/com/ichi2/libanki/sched/AbstractSchedTest.java +++ b/AnkiDroid/src/test/java/com/ichi2/libanki/sched/AbstractSchedTest.java @@ -204,7 +204,7 @@ public void deckDueTreeInconsistentDecksPasses() { addDeckWithExactName(child); getCol().getDecks().checkIntegrity(); - assertDoesNotThrow(() -> getCol().getSched().deckDueList()); + assertDoesNotThrow(() -> getCol().getSched().deckDueTree()); } diff --git a/AnkiDroid/src/test/java/com/ichi2/libanki/sched/SchedV2Test.java b/AnkiDroid/src/test/java/com/ichi2/libanki/sched/SchedV2Test.java index e16361654a04..1a1c2d483af8 100644 --- a/AnkiDroid/src/test/java/com/ichi2/libanki/sched/SchedV2Test.java +++ b/AnkiDroid/src/test/java/com/ichi2/libanki/sched/SchedV2Test.java @@ -822,8 +822,12 @@ public void test_review_limits() throws Exception { c.flush(); } - // position 0 is default deck. Different from upstream - TreeNode tree = col.getSched().deckDueTree().get(1); + int parentIndex = 0; + if (BackendFactory.getDefaultLegacySchema()) { + // position 0 is default deck. Different from upstream + parentIndex = 1; + } + TreeNode tree = col.getSched().deckDueTree().get(parentIndex); // (('parent', 1514457677462, 5, 0, 0, (('child', 1514457677463, 5, 0, 0, ()),))) assertEquals("parent", tree.getValue().getFullDeckName()); assertEquals(5, tree.getValue().getRevCount()); // paren, tree.review_count)t @@ -839,7 +843,7 @@ public void test_review_limits() throws Exception { col.getSched().answerCard(c, BUTTON_THREE); assertEquals(new Counts(0, 0, 9), col.getSched().counts()); - tree = col.getSched().deckDueTree().get(1); + tree = col.getSched().deckDueTree().get(parentIndex); assertEquals(4, tree.getValue().getRevCount()); assertEquals(9, tree.getChildren().get(0).getValue().getRevCount()); }