Skip to content

Commit

Permalink
Fixed slow load of flashcard modal (bug introduced in 1.12.0) (#926)
Browse files Browse the repository at this point in the history
* Possibly fixed, for beta testing

* Added logging of detailed timing debug

* Problem fixed, still needs tidy up

* Removed detailed timing logging

* Removed detailed timing logging

* lint, format and update change log

* Removed unused code caught by test coverage checks
  • Loading branch information
ronzulu authored Apr 9, 2024
1 parent 57bbfc2 commit d5fdb06
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 18 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
4 changes: 4 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. Dates are d

Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).

#### [Unreleased]

- [BUG] Very slow load of flashcard modal (since 1.11.2) [`#914`](https://github.com/st3v3nmw/obsidian-spaced-repetition/issues/914)

#### [1.12.2](https://github.com/st3v3nmw/obsidian-spaced-repetition/compare/1.12.1...1.12.2)

- fix bug with recognizing frontmatter topic tags (led to missing cards shown for review) [`#920`](https://github.com/st3v3nmw/obsidian-spaced-repetition/pull/920)
Expand Down
22 changes: 14 additions & 8 deletions src/NoteQuestionParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,21 @@ export class NoteQuestionParser {
onlyKeepQuestionsWithTopicPath: boolean,
): Promise<Question[]> {
this.noteFile = noteFile;
const noteText: string = await noteFile.read();

// Get the list of tags, and analyse for the topic list
const tagCacheList: TagCache[] = noteFile.getAllTagsFromText();

const hasTopicPaths =
tagCacheList.some((item) => SettingsUtil.isFlashcardTag(this.settings, item.tag)) ||
// For efficiency, we first get the tag list from the Obsidian cache
// (this only gives the tag names, not the line numbers, but this is sufficient for this first step)
const tagCacheList: string[] = noteFile.getAllTagsFromCache();
const hasTopicPaths: boolean =
tagCacheList.some((item) => SettingsUtil.isFlashcardTag(this.settings, item)) ||
folderTopicPath.hasPath;

if (hasTopicPaths) {
// Reading the file is relatively an expensive operation, so we only do this when needed
const noteText: string = await noteFile.read();

// Now that we know there are relevant flashcard tags in the file, we can get the more detailed info
// that includes the line numbers of each tag
const tagCompleteList: TagCache[] = noteFile.getAllTagsFromText();

// The following analysis can require fair computation.
// There is no point doing it if there aren't any topic paths

Expand All @@ -51,7 +57,7 @@ export class NoteQuestionParser {

// For each question, determine it's TopicPathList
[this.frontmatterTopicPathList, this.contentTopicPathInfo] =
this.analyseTagCacheList(tagCacheList);
this.analyseTagCacheList(tagCompleteList);
for (const question of this.questionList) {
question.topicPathList = this.determineQuestionTopicPathList(question);
}
Expand Down
17 changes: 16 additions & 1 deletion src/SRFile.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
import { MetadataCache, TFile, Vault, HeadingCache, TagCache, FrontMatterCache } from "obsidian";
import {
MetadataCache,
TFile,
Vault,
HeadingCache,
getAllTags as ObsidianGetAllTags,
TagCache,
FrontMatterCache,
} from "obsidian";
import { parseObsidianFrontmatterTag } from "./util/utils";

export interface ISRFile {
get path(): string;
get basename(): string;
getAllTagsFromCache(): string[];
getAllTagsFromText(): TagCache[];
getQuestionContext(cardLine: number): string[];
read(): Promise<string>;
Expand All @@ -29,6 +38,12 @@ export class SrTFile implements ISRFile {
return this.file.basename;
}

getAllTagsFromCache(): string[] {
const fileCachedData = this.metadataCache.getFileCache(this.file) || {};
const result: string[] = ObsidianGetAllTags(fileCachedData) || [];
return result;
}

getAllTagsFromText(): TagCache[] {
const result: TagCache[] = [] as TagCache[];
const fileCachedData = this.metadataCache.getFileCache(this.file) || {};
Expand Down
31 changes: 31 additions & 0 deletions src/util/TimeTestUtil.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import moment from "moment";

// These functions were used to diagnose performance issue
// https://github.com/st3v3nmw/obsidian-spaced-repetition/issues/914

let testTimeInfo: [string, number][];

export function testTimeStart(): void {
testTimeInfo = [["Start", moment().valueOf()]];
}

export function testTimeLog(desc: string): void {
if (testTimeInfo == null) testTimeStart();
testTimeInfo.push([desc, moment().valueOf()]);
}

export function testTimeGetLapTime(): number {
return moment().valueOf() - testTimeInfo[0][1];
}

export function testTimeFormatLapInfo(): string {
let prevTime: number = testTimeInfo[0][1];
let result: string = "";
for (let i = 1; i < testTimeInfo.length; i++) {
const thisTime: number = testTimeInfo[i][1];
result += `\t${testTimeInfo[i][0]}\t${thisTime - prevTime}`;
prevTime = thisTime;
}
result += `\tLapTime\t${testTimeGetLapTime()}|`;
return result;
}
7 changes: 0 additions & 7 deletions tests/unit/helpers/UnitTestHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,3 @@ export function unitTest_GetAllTagsFromTextEx(text: string): TagCache[] {
}
return result;
}

export function unitTest_GetAllTagsFromText(text: string): string[] {
const tagRegex = /#[^\s#]+/gi;
const result: RegExpMatchArray = text.match(tagRegex);
if (!result) return [];
return result;
}
4 changes: 4 additions & 0 deletions tests/unit/helpers/UnitTestSRFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ export class UnitTestSRFile implements ISRFile {
return "";
}

getAllTagsFromCache(): string[] {
return unitTest_GetAllTagsFromTextEx(this.content).map((item) => item.tag);
}

getAllTagsFromText(): TagCache[] {
return unitTest_GetAllTagsFromTextEx(this.content);
}
Expand Down

0 comments on commit d5fdb06

Please sign in to comment.