Limit cpu+mem, read+write only once in checkToClose

Several functions were reading + writing the entirety of `persist:localStorage` and were making `checkToClose` heavy on memory without needing to. Reduce the read + write to only 1 time to cut lots of memory and CPU usage.
tabwrangler · Nov 24, 2023 · 487bf10 · 487bf10
1 parent 46c624e
commit 487bf10
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 48 deletions.
diff --git a/app/background.ts b/app/background.ts
@@ -1,14 +1,18 @@
 import {
-  getOlderThen,
+  StorageLocalPersistState,
+  getStorageLocalPersist,
+  getStorageSyncPersist,
+} from "./js/queries";
+import {
   initTabs,
   onNewTab,
   removeTab,
   replaceTab,
   updateClosedCount,
   updateLastAccessed,
   wrangleTabs,
+  wrangleTabsAndPersist,
 } from "./js/tabUtil";
-import { getStorageLocalPersist, getStorageSyncPersist } from "./js/queries";
 import Menus from "./js/menus";
 import debounce from "lodash.debounce";
 import { removeAllSavedTabs } from "./js/actions/localStorageActions";
@@ -48,7 +52,7 @@ chrome.commands.onCommand.addListener((command) => {
       break;
     case "wrangle-current-tab":
       chrome.tabs.query({ active: true, currentWindow: true }, (tabs) => {
-        wrangleTabs(tabs);
+        wrangleTabsAndPersist(tabs);
       });
       break;
     default:
@@ -89,18 +93,19 @@ chrome.storage.onChanged.addListener((changes, areaName) => {
   }
 });
 
-function closeTabs(tabs: chrome.tabs.Tab[]): Promise<void> {
-  console.debug("[closeTabs] CLOSING TABS", tabs);
-  return wrangleTabs(
-    tabs.filter(
-      (tab) =>
-        !(
-          true === tab.pinned ||
-          (settings.get("filterAudio") && tab.audible) ||
-          (tab.url != null && settings.isWhitelisted(tab.url))
-        )
-    )
-  );
+function getTabsOlderThan(
+  tabTimes: StorageLocalPersistState["tabTimes"],
+  time: number
+): Array<number> {
+  const ret: Array<number> = [];
+  for (const i in tabTimes) {
+    if (Object.prototype.hasOwnProperty.call(tabTimes, i)) {
+      if (!time || tabTimes[i] < time) {
+        ret.push(parseInt(i, 10));
+      }
+    }
+  }
+  return ret;
 }
 
 async function startup() {
@@ -111,14 +116,14 @@ async function startup() {
     try {
       cutOff = cutOff || new Date().getTime() - settings.get<number>("stayOpen");
       const minTabs = settings.get<number>("minTabs");
+      const storageLocalPersist = await getStorageLocalPersist();
+      const storageSyncPersist = await getStorageSyncPersist();
 
       // Tabs which have been locked via the checkbox.
       const lockedIds = settings.get<Array<number>>("lockedIds");
-      const toCut = await getOlderThen(cutOff);
-
+      const toCut = getTabsOlderThan(storageLocalPersist.tabTimes, cutOff);
       console.debug("[checkToClose] toCut", toCut);
-      const storageLocalPersist = await getStorageLocalPersist();
-      const storageSyncPersist = await getStorageSyncPersist();
+
       if (!storageSyncPersist.paused) {
         const updatedAt = Date.now();
 
@@ -185,10 +190,20 @@ async function startup() {
             tabsToClose.push(tabsToCut[i]);
           }
 
+          wrangleTabs(
+            storageLocalPersist,
+            tabsToClose.filter(
+              (tab) =>
+                !(
+                  true === tab.pinned ||
+                  (settings.get("filterAudio") && tab.audible) ||
+                  (tab.url != null && settings.isWhitelisted(tab.url))
+                )
+            )
+          );
           await chrome.storage.local.set({
             "persist:localStorage": storageLocalPersist,
           });
-          await closeTabs(tabsToClose);
         }
       }
     } catch (error) {

diff --git a/app/js/__tests__/tabUtil.test.ts b/app/js/__tests__/tabUtil.test.ts
@@ -2,7 +2,7 @@ import {
   findPositionByHostnameAndTitle,
   findPositionByURL,
   getURLPositionFilterByWrangleOption,
-  wrangleTabs,
+  wrangleTabsAndPersist,
 } from "../tabUtil";
 import { setSavedTabs } from "../actions/localStorageActions";
 import settings from "../settings";
@@ -31,14 +31,14 @@ beforeEach(async () => {
   jest.clearAllMocks();
 });
 
-describe("wrangleTabs", () => {
+describe("wrangleTabsAndPersist", () => {
   test("wrangles new tabs", async () => {
     // eslint-disable-next-line @typescript-eslint/ban-ts-comment
     // @ts-ignore:next-line
     settings.get = jest.fn(() => 5); //maxTabs
 
     const testTabs = [createTab({ id: 2 }), createTab({ id: 3 }), createTab({ id: 4 })];
-    await wrangleTabs(testTabs);
+    await wrangleTabsAndPersist(testTabs);
 
     expect(window.chrome.tabs.remove).toHaveBeenCalledTimes(1);
     expect(window.chrome.tabs.remove).toHaveBeenCalledWith([2, 3, 4]);
@@ -57,7 +57,7 @@ describe("wrangleTabs", () => {
       createTab({ id: 5 }),
     ];
 
-    await wrangleTabs(testTabs);
+    await wrangleTabsAndPersist(testTabs);
 
     expect(window.chrome.tabs.remove).toHaveBeenCalledTimes(1);
     expect(window.chrome.tabs.remove).toHaveBeenCalledWith([2, 3, 4, 5]);
@@ -79,7 +79,7 @@ describe("wrangleTabs", () => {
 
     const testTabs = [createTab({ id: 4, url: "https://www.nytimes.com" })];
 
-    await wrangleTabs(testTabs);
+    await wrangleTabsAndPersist(testTabs);
     expect(window.chrome.tabs.remove).toHaveBeenCalledWith([4]);
     const data = await chrome.storage.local.get("persist:localStorage");
     expect(data["persist:localStorage"].totalTabsWrangled).toEqual(1);
@@ -100,7 +100,7 @@ describe("wrangleTabs", () => {
       createTab({ id: 4, url: "https://www.nytimes.com", title: "New York Times" }),
     ];
 
-    await wrangleTabs(testTabs);
+    await wrangleTabsAndPersist(testTabs);
 
     expect(window.chrome.tabs.remove).toHaveBeenCalledTimes(1);
     expect(window.chrome.tabs.remove).toHaveBeenCalledWith([4]);

diff --git a/app/js/menus.ts b/app/js/menus.ts
@@ -1,5 +1,5 @@
 import settings from "./settings";
-import { wrangleTabs } from "./tabUtil";
+import { wrangleTabsAndPersist } from "./tabUtil";
 
 function getDomain(url: string): string | null {
   const match = url.match(/[^:]+:\/\/([^/]+)\//);
@@ -34,7 +34,7 @@ export default class Menus {
 
   corralTab(_onClickData: unknown, tab?: chrome.tabs.Tab | undefined) {
     if (tab == null) return;
-    wrangleTabs([tab]);
+    wrangleTabsAndPersist([tab]);
   }
 
   lockTab(_onClickData: chrome.contextMenus.OnClickData, tab?: chrome.tabs.Tab | undefined) {

diff --git a/app/js/queries.ts b/app/js/queries.ts
@@ -17,7 +17,7 @@ export async function getStorageSyncPersist(): Promise<StorageSyncPersistState>
   return Object.assign({}, STORAGE_SYNC_PERSIST_DEFAULTS, data["persist:settings"]);
 }
 
-type StorageLocalPersistState = {
+export type StorageLocalPersistState = {
   // Date of installation of Tab Wrangler
   installDate: number;
   // Tabs closed by Tab Wrangler

diff --git a/app/js/tabUtil.ts b/app/js/tabUtil.ts
@@ -1,10 +1,10 @@
+import { StorageLocalPersistState, getStorageLocalPersist } from "./queries";
 import {
   incrementTotalTabsRemoved,
   removeTabTime,
   setTabTime,
   setTabTimes,
 } from "./actions/localStorageActions";
-import { getStorageLocalPersist } from "./queries";
 import settings from "./settings";
 
 type WrangleOption = "exactURLMatch" | "hostnameAndTitleMatch" | "withDuplicates";
@@ -40,13 +40,16 @@ export function getURLPositionFilterByWrangleOption(
   return () => -1;
 }
 
-export async function wrangleTabs(tabs: Array<chrome.tabs.Tab>) {
+// Note: Mutates `storageLocalPersist`!
+export function wrangleTabs(
+  storageLocalPersist: StorageLocalPersistState,
+  tabs: Array<chrome.tabs.Tab>
+) {
   // No tabs, nothing to do
   if (tabs.length === 0) return;
 
   console.debug("[wrangleTabs] WRANGLING TABS", tabs);
 
-  const storageLocalPersist = await getStorageLocalPersist();
   const maxTabs = settings.get<number>("maxTabs");
   const wrangleOption = settings.get<WrangleOption>("wrangleOption");
   const findURLPositionByWrangleOption = getURLPositionFilterByWrangleOption(
@@ -82,7 +85,14 @@ export async function wrangleTabs(tabs: Array<chrome.tabs.Tab>) {
   // thus cannot allow saved tabs to grow indefinitely.
   if (storageLocalPersist.savedTabs.length - maxTabs > 0)
     storageLocalPersist.savedTabs = storageLocalPersist.savedTabs.splice(0, maxTabs);
+}
+
+export async function wrangleTabsAndPersist(tabs: Array<chrome.tabs.Tab>) {
+  // No tabs, nothing to do
+  if (tabs.length === 0) return;
 
+  const storageLocalPersist = await getStorageLocalPersist();
+  wrangleTabs(storageLocalPersist, tabs);
   await chrome.storage.local.set({
     "persist:localStorage": storageLocalPersist,
   });
@@ -96,22 +106,6 @@ export async function initTabs() {
   );
 }
 
-/**
- * @param time - if null, returns all
- */
-export async function getOlderThen(time?: number): Promise<Array<number>> {
-  const ret: Array<number> = [];
-  const { tabTimes } = await getStorageLocalPersist();
-  for (const i in tabTimes) {
-    if (Object.prototype.hasOwnProperty.call(tabTimes, i)) {
-      if (!time || tabTimes[i] < time) {
-        ret.push(parseInt(i, 10));
-      }
-    }
-  }
-  return ret;
-}
-
 export function onNewTab(tab: chrome.tabs.Tab) {
   console.debug("[onNewTab] updating new tab", tab);
   // Track new tab's time to close.