Skip to content

Commit

Permalink
Speed up deck list by rendering it with the backend
Browse files Browse the repository at this point in the history
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 ankidroid#11599
  • Loading branch information
dae committed Jun 28, 2022
1 parent f570a2b commit d4b31ee
Show file tree
Hide file tree
Showing 9 changed files with 125 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<TreeNode<DeckDueTreeNode>>, 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<TreeNode<DeckDueTreeNode>>, 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
}
Expand All @@ -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
Expand Down
15 changes: 6 additions & 9 deletions AnkiDroid/src/main/java/com/ichi2/libanki/sched/AbstractSched.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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<DeckDueTreeNode>

/**
* @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<TreeNode<DeckDueTreeNode>>?

/**
* @return the due tree. null if task is cancelled.
* @return the due tree. Never null.
*/
abstract fun deckDueTree(): List<TreeNode<DeckDueTreeNode>>
fun deckDueTree(): List<TreeNode<DeckDueTreeNode>> {
return deckDueTree(null)!!
}

/**
* @return The tree of decks, without numbers
*/
abstract fun quickDeckDueTree(): List<TreeNode<DeckTreeNode>>
abstract fun<T : AbstractDeckTreeNode> quickDeckDueTree(): List<TreeNode<T>>

/** New count for a single deck.
* @param did The deck to consider (descendants and ancestors are ignored)
Expand Down
55 changes: 55 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/libanki/sched/BackendSched.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/***************************************************************************************
* Copyright (c) 2012 Ankitects Pty Ltd <http://apps.ankiweb.net> *
* *
* 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 <http://www.gnu.org/licenses/>. *
****************************************************************************************/

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<TreeNode<DeckDueTreeNode>> {
fun toLegacyNode(node: anki.decks.DeckTreeNode, parentName: String): TreeNode<DeckDueTreeNode> {
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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion AnkiDroid/src/main/java/com/ichi2/libanki/sched/Sched.java
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ private void unburyCardsForDeck(@NonNull List<Long> allDecks) {
* Returns [deckname, did, rev, lrn, new]
*/
@Override
public @Nullable List<DeckDueTreeNode> deckDueList(@Nullable CancelListener cancelListener) {
protected @Nullable List<DeckDueTreeNode> deckDueList(@Nullable CancelListener cancelListener) {
_checkDay();
getCol().getDecks().checkIntegrity();
List<Deck> allDecksSorted = getCol().getDecks().allSorted();
Expand Down
35 changes: 20 additions & 15 deletions AnkiDroid/src/main/java/com/ichi2/libanki/sched/SchedV2.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -523,14 +525,14 @@ protected int _walkingCount(@NonNull LimitMethod limFn, @NonNull CountMethod cnt
*
* Return nulls when deck task is cancelled.
*/
public @NonNull List<DeckDueTreeNode> deckDueList() {
private @NonNull List<DeckDueTreeNode> deckDueList() {
return deckDueList(null);
}

// Overridden
/**
* Return sorted list of all decks.*/
public @Nullable List<DeckDueTreeNode> deckDueList(@Nullable CancelListener collectionTask) {
protected @Nullable List<DeckDueTreeNode> deckDueList(@Nullable CancelListener collectionTask) {
_checkDay();
getCol().getDecks().checkIntegrity();
List<Deck> allDecksSorted = getCol().getDecks().allSorted();
Expand Down Expand Up @@ -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<TreeNode<DeckTreeNode>> quickDeckDueTree() {
// Similar to deckDueTree, ignoring the numbers

public @NonNull
List<? extends TreeNode<? extends AbstractDeckTreeNode>> quickDeckDueTree() {
if (!BackendFactory.getDefaultLegacySchema()) {
return BackendSchedKt.deckTreeLegacy(getCol().getNewBackend(), false);
}
// Similar to deckDueList
ArrayList<DeckTreeNode> allDecksSorted = new ArrayList<>();
for (JSONObject deck : getCol().getDecks().allSorted()) {
Expand All @@ -588,18 +592,19 @@ protected int _walkingCount(@NonNull LimitMethod limFn, @NonNull CountMethod cnt
return _groupChildren(allDecksSorted, false);
}


public @NonNull List<TreeNode<DeckDueTreeNode>> 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<TreeNode<DeckDueTreeNode>> deckDueTree(@Nullable CancelListener cancelListener) {
List<DeckDueTreeNode> allDecksSorted = deckDueList(cancelListener);
if (allDecksSorted == null) {
return null;
if (!BackendFactory.getDefaultLegacySchema()) {
return BackendSchedKt.deckTreeLegacy(getCol().getNewBackend(), true);
} else {
List<DeckDueTreeNode> allDecksSorted = deckDueList(cancelListener);
if (allDecksSorted == null) {
return null;
}
return _groupChildren(allDecksSorted, true);
}
return _groupChildren(allDecksSorted, true);
}

/**
Expand Down Expand Up @@ -666,7 +671,7 @@ public List<TreeNode<DeckDueTreeNode>> deckDueTree(@Nullable CancelListener canc
TreeNode<T> toAdd = new TreeNode<>(child);
toAdd.getChildren().addAll(childrenNode);
List<T> 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);
}
Expand Down
2 changes: 1 addition & 1 deletion AnkiDroid/src/main/java/com/ichi2/libanki/sync/Syncer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ public void deckDueTreeInconsistentDecksPasses() {
addDeckWithExactName(child);

getCol().getDecks().checkIntegrity();
assertDoesNotThrow(() -> getCol().getSched().deckDueList());
assertDoesNotThrow(() -> getCol().getSched().deckDueTree());
}


Expand Down
10 changes: 7 additions & 3 deletions AnkiDroid/src/test/java/com/ichi2/libanki/sched/SchedV2Test.java
Original file line number Diff line number Diff line change
Expand Up @@ -822,8 +822,12 @@ public void test_review_limits() throws Exception {
c.flush();
}

// position 0 is default deck. Different from upstream
TreeNode<DeckDueTreeNode> tree = col.getSched().deckDueTree().get(1);
int parentIndex = 0;
if (BackendFactory.getDefaultLegacySchema()) {
// position 0 is default deck. Different from upstream
parentIndex = 1;
}
TreeNode<DeckDueTreeNode> 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
Expand All @@ -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());
}
Expand Down

0 comments on commit d4b31ee

Please sign in to comment.