From 0ac42608f7f7558823a1f61c02ff700b79a8fa7b Mon Sep 17 00:00:00 2001 From: zadam Date: Thu, 10 Dec 2020 21:27:21 +0100 Subject: [PATCH] faster tree loading of many notes at once #1480 --- src/public/app/services/tree_cache.js | 38 ----------- src/routes/api/tree.js | 91 +++++++++++++++------------ src/services/sql.js | 26 +++++++- src/services/sql_init.js | 2 + src/services/tree.js | 36 +++++++++++ 5 files changed, 114 insertions(+), 79 deletions(-) diff --git a/src/public/app/services/tree_cache.js b/src/public/app/services/tree_cache.js index 22c332ddd..9a83b0eb8 100644 --- a/src/public/app/services/tree_cache.js +++ b/src/public/app/services/tree_cache.js @@ -20,9 +20,6 @@ class TreeCache { async loadInitialTree() { const resp = await server.get('tree'); - // FIXME: we need to do this to cover for ascendants of template notes which are not loaded - await this.loadParents(resp, false); - // clear the cache only directly before adding new content which is important for e.g. switching to protected session /** @type {Object.} */ @@ -43,45 +40,11 @@ class TreeCache { async loadSubTree(subTreeNoteId) { const resp = await server.get('tree?subTreeNoteId=' + subTreeNoteId); - await this.loadParents(resp, true); - this.addResp(resp); return this.notes[subTreeNoteId]; } - async loadParents(resp, additiveLoad) { - const noteIds = new Set(resp.notes.map(note => note.noteId)); - const missingNoteIds = []; - const existingNotes = additiveLoad ? this.notes : {}; - - for (const branch of resp.branches) { - if (!(branch.parentNoteId in existingNotes) && !noteIds.has(branch.parentNoteId) && branch.parentNoteId !== 'none') { - missingNoteIds.push(branch.parentNoteId); - } - } - - for (const attr of resp.attributes) { - if (attr.type === 'relation' && attr.name === 'template' && !(attr.value in existingNotes) && !noteIds.has(attr.value)) { - missingNoteIds.push(attr.value); - } - - if (!(attr.noteId in existingNotes) && !noteIds.has(attr.noteId)) { - missingNoteIds.push(attr.noteId); - } - } - - if (missingNoteIds.length > 0) { - const newResp = await server.post('tree/load', { noteIds: missingNoteIds }); - - resp.notes = resp.notes.concat(newResp.notes); - resp.branches = resp.branches.concat(newResp.branches); - resp.attributes = resp.attributes.concat(newResp.attributes); - - await this.loadParents(resp, additiveLoad); - } - } - addResp(resp) { const noteRows = resp.notes; const branchRows = resp.branches; @@ -198,7 +161,6 @@ class TreeCache { const resp = await server.post('tree/load', { noteIds }); - await this.loadParents(resp, true); this.addResp(resp); for (const note of resp.notes) { diff --git a/src/routes/api/tree.js b/src/routes/api/tree.js index e6b810834..02f95ea66 100644 --- a/src/routes/api/tree.js +++ b/src/routes/api/tree.js @@ -5,14 +5,15 @@ const optionService = require('../../services/options'); const treeService = require('../../services/tree'); function getNotesAndBranchesAndAttributes(noteIds) { - noteIds = Array.from(new Set(noteIds)); - const notes = treeService.getNotes(noteIds); + const notes = treeService.getNotesIncludingAscendants(noteIds); - noteIds = notes.map(note => note.noteId); + noteIds = new Set(notes.map(note => note.noteId)); + + sql.fillNoteIdList(noteIds); // joining child note to filter out not completely synchronised notes which would then cause errors later // cannot do that with parent because of root note's 'none' parent - const branches = sql.getManyRows(` + const branches = sql.getRows(` SELECT branches.branchId, branches.noteId, @@ -20,28 +21,45 @@ function getNotesAndBranchesAndAttributes(noteIds) { branches.notePosition, branches.prefix, branches.isExpanded - FROM branches + FROM param_list + JOIN branches ON param_list.paramId = branches.noteId OR param_list.paramId = branches.parentNoteId JOIN notes AS child ON child.noteId = branches.noteId - WHERE branches.isDeleted = 0 - AND (branches.noteId IN (???) OR parentNoteId IN (???))`, noteIds); + WHERE branches.isDeleted = 0`); + + const attributes = sql.getRows(` + SELECT + attributes.attributeId, + attributes.noteId, + attributes.type, + attributes.name, + attributes.value, + attributes.position, + attributes.isInheritable + FROM param_list + JOIN attributes ON attributes.noteId = param_list.paramId + OR (attributes.type = 'relation' AND attributes.value = param_list.paramId) + WHERE attributes.isDeleted = 0`); + + // we don't really care about the direction of the relation + const missingTemplateNoteIds = attributes + .filter(attr => attr.type === 'relation' + && attr.name === 'template' + && !noteIds.has(attr.value)) + .map(attr => attr.value); + + if (missingTemplateNoteIds.length > 0) { + const templateData = getNotesAndBranchesAndAttributes(missingTemplateNoteIds); + + // there are going to be duplicates with simple concatenation, however: + // 1) shouldn't matter for the frontend which will update the entity twice + // 2) there shouldn't be many duplicates. There isn't that many templates + addArrays(notes, templateData.notes); + addArrays(branches, templateData.branches); + addArrays(attributes, templateData.attributes); + } // sorting in memory is faster branches.sort((a, b) => a.notePosition - b.notePosition < 0 ? -1 : 1); - - const attributes = sql.getManyRows(` - SELECT - attributeId, - noteId, - type, - name, - value, - position, - isInheritable - FROM attributes - WHERE isDeleted = 0 - AND (noteId IN (???) OR (type = 'relation' AND value IN (???)))`, noteIds); - - // sorting in memory is faster attributes.sort((a, b) => a.position - b.position < 0 ? -1 : 1); return { @@ -51,6 +69,16 @@ function getNotesAndBranchesAndAttributes(noteIds) { }; } +// should be fast based on https://stackoverflow.com/a/64826145/944162 +// in this case it is assumed that target is potentially much larger than elementsToAdd +function addArrays(target, elementsToAdd) { + while (elementsToAdd.length) { + target.push(elementsToAdd.shift()); + } + + return target; +} + function getTree(req) { const subTreeNoteId = req.query.subTreeNoteId || optionService.getOption('hoistedNoteId'); @@ -63,25 +91,8 @@ function getTree(req) { JOIN treeWithDescendants ON branches.parentNoteId = treeWithDescendants.noteId WHERE treeWithDescendants.isExpanded = 1 AND branches.isDeleted = 0 - ), - treeWithDescendantsAndAscendants AS ( - SELECT noteId FROM treeWithDescendants - UNION - SELECT branches.parentNoteId FROM branches - JOIN treeWithDescendantsAndAscendants ON branches.noteId = treeWithDescendantsAndAscendants.noteId - WHERE branches.isDeleted = 0 - AND branches.parentNoteId != ? - ), - treeWithDescendantsAscendantsAndTemplates AS ( - SELECT noteId FROM treeWithDescendantsAndAscendants - UNION - SELECT attributes.value FROM attributes - JOIN treeWithDescendantsAscendantsAndTemplates ON attributes.noteId = treeWithDescendantsAscendantsAndTemplates.noteId - WHERE attributes.isDeleted = 0 - AND attributes.type = 'relation' - AND attributes.name = 'template' ) - SELECT noteId FROM treeWithDescendantsAscendantsAndTemplates`, [subTreeNoteId, subTreeNoteId]); + SELECT noteId FROM treeWithDescendants`, [subTreeNoteId]); noteIds.push(subTreeNoteId); diff --git a/src/services/sql.js b/src/services/sql.js index fbf0a10dd..1f8ae1be2 100644 --- a/src/services/sql.js +++ b/src/services/sql.js @@ -236,6 +236,29 @@ function transactional(func) { return ret; } +function fillNoteIdList(noteIds, truncate = true) { + if (noteIds.length === 0) { + return; + } + + if (truncate) { + execute("DELETE FROM param_list"); + } + + noteIds = Array.from(new Set(noteIds)); + + if (noteIds.length > 30000) { + fillNoteIdList(noteIds.slice(30000), false); + + noteIds = noteIds.slice(0, 30000); + } + + // doing it manually to avoid this showing up on the sloq query list + const s = stmt(`INSERT INTO param_list VALUES ` + noteIds.map(noteId => `(?)`).join(','), noteIds); + + s.run(noteIds); +} + module.exports = { dbConnection, insert, @@ -253,5 +276,6 @@ module.exports = { executeMany, executeScript, transactional, - upsert + upsert, + fillNoteIdList }; diff --git a/src/services/sql_init.js b/src/services/sql_init.js index c655fcee8..8af9c8f35 100644 --- a/src/services/sql_init.js +++ b/src/services/sql_init.js @@ -40,6 +40,8 @@ async function initDbConnection() { require('./options_init').initStartupOptions(); + sql.execute('CREATE TEMP TABLE "param_list" (`paramId` TEXT NOT NULL PRIMARY KEY)'); + dbReady.resolve(); } diff --git a/src/services/tree.js b/src/services/tree.js index 582517c36..43e6bdc81 100644 --- a/src/services/tree.js +++ b/src/services/tree.js @@ -6,6 +6,41 @@ const Branch = require('../entities/branch'); const entityChangesService = require('./entity_changes.js'); const protectedSessionService = require('./protected_session'); +function getNotesIncludingAscendants(noteIds) { + noteIds = Array.from(new Set(noteIds)); + + sql.fillNoteIdList(noteIds); + + // we return also deleted notes which have been specifically asked for + + const notes = sql.getRows(` + WITH RECURSIVE + treeWithAscendants AS ( + SELECT paramId AS noteId FROM param_list + UNION + SELECT branches.parentNoteId FROM branches + JOIN treeWithAscendants ON branches.noteId = treeWithAscendants.noteId + WHERE branches.isDeleted = 0 + ) + SELECT + noteId, + title, + isProtected, + type, + mime, + isDeleted + FROM notes + JOIN treeWithAscendants USING(noteId)`); + + protectedSessionService.decryptNotes(notes); + + notes.forEach(note => { + note.isProtected = !!note.isProtected + }); + + return notes; +} + function getNotes(noteIds) { // we return also deleted notes which have been specifically asked for const notes = sql.getManyRows(` @@ -190,6 +225,7 @@ function setNoteToParent(noteId, prefix, parentNoteId) { module.exports = { getNotes, + getNotesIncludingAscendants, validateParentChild, sortNotesAlphabetically, setNoteToParent