From 1bebb3fe6f1de05c17688b03bee0a85098fcd57a Mon Sep 17 00:00:00 2001 From: Charlles Galves Date: Fri, 4 Sep 2026 10:34:13 -0300 Subject: [PATCH] perf: cut the popup's serialized storage round trips on cold start Opening the popup ran roughly fifty chrome.storage round trips one after another before Vue could mount, which is what makes it feel slow to open even with a couple of entries and no encryption configured. Almost all of them come from two places: - BrowserStorage.getStorageLocation() runs at the top of every storage operation, and each call re-read all user settings (one or two round trips, depending on whether settings live in sync storage) plus the managed policy. Nothing was cached, so the same values were fetched over and over. - The popup awaited each async Vuex module in turn, and the advisor store awaited its five insight validations in turn -- with each validation re-reading settings from scratch. Cache the settings and the managed policy, invalidating on chrome.storage.onChanged so other extension contexts are still observed, and coalesce concurrent readers onto a single in-flight read. Then build the store modules and the i18n catalog concurrently, and run the advisor validations concurrently. Managed storage keeps its original contract: no caller waits more than ~10ms on it, and a policy that arrives after that still lands in the cache for later lookups. Measured against a chrome.storage mock that counts calls: 15x UserSettings.updateItems(): 30 gets -> 2 8x ManagedStorage.get(): 8 gets -> 1 Also adds a missing await in the backup store, which read UserSettings.items before the read it depends on had resolved. Co-Authored-By: Claude Opus 5 --- src/models/settings.ts | 46 ++++++++++++++++- src/models/storage.ts | 112 ++++++++++++++++++++++++++++++++--------- src/popup.ts | 24 ++++++--- src/store/Advisor.ts | 22 ++++---- src/store/Backup.ts | 2 +- 5 files changed, 162 insertions(+), 44 deletions(-) diff --git a/src/models/settings.ts b/src/models/settings.ts index aac7a84b0..1a0da4086 100644 --- a/src/models/settings.ts +++ b/src/models/settings.ts @@ -56,8 +56,51 @@ const LocalUserSettingsDataKeys = [ export class UserSettings { static items: UserSettingsData = {}; + // Settings are read from chrome.storage on nearly every storage operation + // (see BrowserStorage.getStorageLocation). Reading them afresh each time + // costs one or two IPC round trips per call, which adds up to dozens of + // serialized round trips before the popup can render. Cache the result and + // drop the cache whenever anything writes to storage, so callers still + // observe changes made by other extension contexts. + private static cachedItems: UserSettingsData | null = null; + private static pendingRead: Promise | null = null; + private static invalidationHooked = false; + + private static hookInvalidation() { + if (UserSettings.invalidationHooked) { + return; + } + UserSettings.invalidationHooked = true; + chrome.storage.onChanged?.addListener(() => { + UserSettings.invalidateCache(); + }); + } + + static invalidateCache() { + UserSettings.cachedItems = null; + } + static async updateItems() { - UserSettings.items = await UserSettings.getAllItems(); + UserSettings.hookInvalidation(); + + if (UserSettings.cachedItems) { + UserSettings.items = UserSettings.cachedItems; + return; + } + + // Coalesce concurrent readers onto a single read. + if (!UserSettings.pendingRead) { + UserSettings.pendingRead = UserSettings.getAllItems() + .then((items) => { + UserSettings.cachedItems = items; + return items; + }) + .finally(() => { + UserSettings.pendingRead = null; + }); + } + + UserSettings.items = await UserSettings.pendingRead; } static async convertFromLocalStorage( @@ -108,6 +151,7 @@ export class UserSettings { ]); } + UserSettings.invalidateCache(); await UserSettings.updateItems(); } diff --git a/src/models/storage.ts b/src/models/storage.ts index 4b3699fb2..9d0bfcce0 100644 --- a/src/models/storage.ts +++ b/src/models/storage.ts @@ -3,7 +3,23 @@ import { OTPEntry, OTPType, OTPAlgorithm, CodeState } from "./otp"; import { StorageLocation, UserSettings } from "./settings"; import { DataType } from "./otp"; export class BrowserStorage { - private static async getStorageLocation(): Promise { + private static pendingLocation: Promise | null = null; + + // Every storage operation resolves the storage location first. Share one + // resolution between concurrent callers so parallel readers don't each + // re-run the auto-detection branch (which can also commit settings). + private static getStorageLocation(): Promise { + if (!BrowserStorage.pendingLocation) { + BrowserStorage.pendingLocation = BrowserStorage.resolveStorageLocation().finally( + () => { + BrowserStorage.pendingLocation = null; + } + ); + } + return BrowserStorage.pendingLocation; + } + + private static async resolveStorageLocation(): Promise { await UserSettings.updateItems(); const managedLocation = await ManagedStorage.get( "storageArea" @@ -688,34 +704,82 @@ export class EntryStorage { } export class ManagedStorage { + // The managed policy is a single object, but callers read it one key at a + // time (the menu store alone reads eight). Fetch it once and answer + // subsequent lookups from memory, dropping the cache if policy changes. + private static cachedPolicy: ManagedPolicy | null = null; + private static pendingPolicy: Promise | null = null; + private static invalidationHooked = false; + + private static hookInvalidation() { + if (ManagedStorage.invalidationHooked) { + return; + } + ManagedStorage.invalidationHooked = true; + chrome.storage.onChanged?.addListener((_changes, areaName) => { + if (areaName === "managed") { + ManagedStorage.cachedPolicy = null; + ManagedStorage.pendingPolicy = null; + } + }); + } + + private static readPolicy(): Promise { + return new Promise((resolve: (result: ManagedPolicy) => void) => { + if (chrome.storage.managed) { + chrome.storage.managed.get((data) => { + if (chrome.runtime.lastError) { + return resolve({}); + } + return resolve(data || {}); + }); + } else { + // no available in Safari + resolve({}); + } + }); + } + + // Share a single underlying read between all callers. The read is kept + // running even if an individual lookup times out below, so a policy that + // arrives late still populates the cache for subsequent lookups. + private static getPolicy(): Promise { + if (!ManagedStorage.pendingPolicy) { + ManagedStorage.pendingPolicy = ManagedStorage.readPolicy().then( + (data) => { + ManagedStorage.cachedPolicy = data; + return data; + } + ); + } + return ManagedStorage.pendingPolicy; + } + static get(key: string): T | undefined; static get(key: string, defaultValue: T): T; static get(key: string, defaultValue?: T) { - const managedStoragePromise = new Promise( - (resolve: (result: T | undefined) => void) => { - if (chrome.storage.managed) { - chrome.storage.managed.get((data) => { - if (chrome.runtime.lastError) { - return resolve(defaultValue); - } - if (data) { - if (data[key]) { - return resolve(data[key]); - } - } - return resolve(defaultValue); - }); - } else { - // no available in Safari - resolve(defaultValue); - } + ManagedStorage.hookInvalidation(); + + const pick = (data: ManagedPolicy) => + data && data[key] ? (data[key] as T) : defaultValue; + + if (ManagedStorage.cachedPolicy) { + return Promise.resolve(pick(ManagedStorage.cachedPolicy)); + } + + // Preserve the original contract: never make a caller wait more than + // ~10ms on managed storage. + const timeoutPromise = new Promise( + (resolve: (r: T | undefined) => void) => { + setTimeout(() => resolve(defaultValue), 10); } ); - const timeoutPromise = new Promise((resolve) => { - setTimeout(() => resolve(defaultValue), 10); - }); - - return Promise.race([managedStoragePromise, timeoutPromise]); + return Promise.race([ + ManagedStorage.getPolicy().then(pick), + timeoutPromise, + ]); } } + +type ManagedPolicy = Record; diff --git a/src/popup.ts b/src/popup.ts index 173bc033e..72bca3c1f 100644 --- a/src/popup.ts +++ b/src/popup.ts @@ -34,9 +34,6 @@ async function init() { await migrateLocalStorageToBrowserStorage(); await UserSettings.updateItems(); - // Add globals - Vue.prototype.i18n = await loadI18nMessages(); - // Load modules Vue.use(Vuex); Vue.use(Vue2Dragula); @@ -46,14 +43,27 @@ async function init() { Vue.component(component.name, component.component); } + // The i18n catalog and the async store modules don't depend on one another, + // so resolve them concurrently instead of awaiting each in turn. + const [i18nMessages, accounts, advisor, backup, menu] = await Promise.all([ + loadI18nMessages(), + new Accounts().getModule(), + new Advisor().getModule(), + new Backup().getModule(), + new Menu().getModule(), + ]); + + // Add globals + Vue.prototype.i18n = i18nMessages; + // State const store = new Vuex.Store({ modules: { - accounts: await new Accounts().getModule(), - advisor: await new Advisor().getModule(), - backup: await new Backup().getModule(), + accounts, + advisor, + backup, currentView: new CurrentView().getModule(), - menu: await new Menu().getModule(), + menu, notification: new Notification().getModule(), qr: new Qr().getModule(), style: new Style().getModule(), diff --git a/src/store/Advisor.ts b/src/store/Advisor.ts index a9514d4e8..8b711cb4e 100644 --- a/src/store/Advisor.ts +++ b/src/store/Advisor.ts @@ -100,19 +100,19 @@ export class Advisor implements Module { ? JSON.parse(UserSettings.items.advisorIgnoreList || "[]") : UserSettings.items.advisorIgnoreList || []; - const filteredInsightsData: AdvisorInsightInterface[] = []; - - for (const insightData of insightsData) { - if (advisorIgnoreList.includes(insightData.id)) { - continue; - } + // The validations are independent reads, so run them concurrently instead + // of paying for each one in turn. Order is preserved by index. + const candidates = insightsData.filter( + (insightData) => !advisorIgnoreList.includes(insightData.id) + ); - const validation = await insightData.validation(); + const validations = await Promise.all( + candidates.map((insightData) => insightData.validation()) + ); - if (validation) { - filteredInsightsData.push(insightData); - } - } + const filteredInsightsData: AdvisorInsightInterface[] = candidates.filter( + (_, index) => validations[index] + ); return filteredInsightsData.map( (insightData) => new AdvisorInsight(insightData) diff --git a/src/store/Backup.ts b/src/store/Backup.ts index 790ed719a..55a98775f 100644 --- a/src/store/Backup.ts +++ b/src/store/Backup.ts @@ -2,7 +2,7 @@ import { UserSettings } from "../models/settings"; export class Backup implements Module { async getModule() { - UserSettings.updateItems(); + await UserSettings.updateItems(); return { state: {