Has anyone faced (and solved or worked around) performance issues when dealing with large numbers of bookmarks?

I have a bookmark context menu command that is enabled only when right-clicking on a bookmark or folder that is inside (a descendant of), say, Folder X.

My original implementation goes: My Command starts out disabled. When a node (bookmark or folder) is right-clicked, get its parentId and check if it matches FOLDER_X_ID, otherwise get that parent node’s parentId and check, and so on recursively. If a match is found, enable My Command.

Snippet to illustrate:

browser.menus.create({
    contexts: ['bookmarks'],
    id: 'myCommand',
    title: 'My Command',
    enabled: false,
});
browser.menus.onShown.addListener  (onMenuShow);
browser.menus.onHidden.addListener (onMenuHide);

async function onMenuShow({ bookmarkId }) {
    if (bookmarkId && await isInsideFolderX(bookmarkId)) {
        browser.menus.update('myCommand', { enabled: true });
        browser.menus.refresh();
    }
}

async function isInsideFolderX(bookmarkId) {
    if (bookmarkId === 'FOLDER_X_ID' || isRootId(bookmarkId)) return false;
    const parentId = (await getNode(bookmarkId)).parentId;
    return parentId === 'FOLDER_X_ID' ? true : await isInsideFolderX(parentId);
}

const ROOT_IDS = ['toolbar_____', 'menu________', 'unfiled_____'];
const isRootId = bookmarkId => ROOT_IDS.includes(bookmarkId);
const getNode = async bookmarkId => (await browser.bookmarks.get(bookmarkId))[0];

function onMenuHide() {
    browser.menus.update('myCommand', { enabled: false }); // Restore disabled status
}    

When there are none or a few bookmarks in the tree (like in a fresh browser instance), upon right-clicking a node inside Folder X, My Command appears instantly enabled.

Meanwhile when there are about 7k+ bookmarks (like in my browser), upon doing the same as above, My Command takes about a second to visibly change from disabled to enabled.

This is even when right-clicking a direct child of Folder X, which rules out multiple serial calls to browser.bookmarks.get() as the culprit.

What this tells me is when the bookmarks tree is large enough, simply accessing a single node once can take a significant amount of time.

This noticeable lag is poor UX; you might just assume the command is disabled if you didn’t wait long enough. Any suggestions to reduce the lag on browsers with lots and lots of bookmarks?

Keep a list of all bookmark IDs where you want the context menu item shown.
Using bookmarks.onCreated, bookmarks.onRemoved, etc.

I don’t know if storage.local alone is fast enough for this.
So maybe you need a copy of the list in memory.

Well, if you are calling your await browser.bookmarks.get 7 000 times than for sure it’s gonna take some time :slight_smile:. It’s asynchronous operation with who knows how efficient implementation behind…
So either cache it or get the whole tree with browser.bookmarks.getTree and process the tree synchronously.

Is that what happens in his code?

If I understand the code correctly, he’s only going up the folder hierarchy, not looking at every single bookmark node.

1 Like

Ah, yes, good point :slight_smile:.
7k would for sure took more than one second, I knew something doesn’t add up :smiley:.

Anyway, the only thing that can be slow about this code are those few async calls, and the only one that can be avoided (or “optimized”) is browser.bookmarks.get.
It would be nice to measure how long it takes to execute it (using performance.now()).

I would guess that the underlying implementation is somehow iterating all nodes which then gets slower and slower with more and more bookmarks.

1 Like

And also bookmarks.onMoved (in and out of Folder X). Yes, I did think about this, a Map or Set of bookmark IDs. I guess I was hoping to hear more ideas, because I can’t think of any other way.

I hadn’t thought of using storage. Probably should since keeping the map/set indefinitely in memory seems like overkill for an occasional-use command. I’ll have to test it out in terms of speed. Thanks.

Is that faster and/or more memory-efficient than using an object to store the bookmark IDs?

When you said “list” I assumed an Array. A more efficient alternative to an Array of IDs is a Set of IDs especially for a very long list, because Set.prototype.has() performs an O(1) lookup versus Array.prototype.includes() performing at O(n) – it traverses through the array from its beginning to find the target.

Map is equivalent to Object with a few improvements such as built-in entry counting viaMap.prototype.size and optimised for frequent additions and deletions. I might opt for Map instead of Set because I’ll likely want extra data like bookmark titles to go along with IDs too.