Storage clean-up interface, prune preferences (#5070)

* Run clean-up less frequently * Common interface for purge old entries * Clean-up preferences regularly The settings console does not remove options that are the default value (or no longer exists), making the option object heavier than necessary. It is important to have it as lean as possible, as any load delays slow down the init process.
honestbleeps · Feb 7, 2019 · ccf7437 · ccf7437
1 parent 2720754
commit ccf7437
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 60 deletions.
diff --git a/lib/core/migrate/migrate.js b/lib/core/migrate/migrate.js
@@ -809,6 +809,14 @@ const migrations = [
 				await Storage.delete('RESmodules.styleTweaks.ignoredSubredditStyles');
 			}
 		},
+	}, {
+		versionNumber: '5.14.4-prune-interface',
+		async go() {
+			await Migrators.updateOption('newCommentCount', 'cleanComments', '7', '30');
+			await Storage.delete('RESmodules.readComments.lastClean');
+			await Storage.delete('RESmodules.newCommentCount.lastClean');
+			await Storage.delete('RESmodules.commentHidePersistor.lastClean');
+		},
 	},
 
 ]; // ^^ Add new migrations ^^

diff --git a/lib/core/options/options.js b/lib/core/options/options.js
@@ -5,7 +5,7 @@ import { flow } from 'lodash/fp';
 import * as Modules from '../modules';
 import { Storage } from '../../environment';
 import type { OpaqueModuleId } from '../module';
-import { filterMap } from '../../utils';
+import { filterMap, shouldPrune } from '../../utils';
 
 const moduleOptionsStorage = Storage.wrapPrefix('RESoptions.', (): { [string]: any } => ({}));
 
@@ -15,6 +15,8 @@ export async function _loadModuleOptions() {
 	for (const mod of Modules.all()) {
 		_initOptions(mod, allOptions[mod.moduleID]);
 	}
+
+	shouldPrune('RESoptions').then(should => { if (should) prune(); });
 }
 
 // copy in stored options and assign default values
@@ -86,6 +88,31 @@ export function set(opaqueId: OpaqueModuleId, optionKey: string, value: mixed) {
 	return true;
 }
 
+export function isDefault(opaqueId: OpaqueModuleId, optionKey: string, optionValue: mixed) {
+	const module = Modules.get(opaqueId);
+
+	if (!module.options[optionKey]) {
+		console.warn('Could not find option', module.moduleID, optionKey);
+		return true; // No option -> any value is default
+	}
+
+	return _.isEqual(module.options[optionKey].default, optionValue);
+}
+
+export async function prune() {
+	for (const [moduleID, storedOptions] of Object.entries(await moduleOptionsStorage.getAll())) {
+		if (!storedOptions) continue;
+		const _def = Object.entries(storedOptions).filter(([key, value]) => isDefault(moduleID, key, value));
+		if (_def.length === storedOptions.length) {
+			await moduleOptionsStorage.delete(moduleID); // eslint-disable-line no-await-in-loop
+		} else {
+			for (const [key] of _def) {
+				await moduleOptionsStorage.deletePath(moduleID, key); // eslint-disable-line no-await-in-loop
+			}
+		}
+	}
+}
+
 function getModified() {
 	return filterMap(Modules.all(), module => {
 		const { moduleID, options } = module;

diff --git a/lib/modules/commentHidePersistor.js b/lib/modules/commentHidePersistor.js
@@ -3,7 +3,7 @@
 import { $ } from '../vendor';
 import { Module } from '../core/module';
 import { Storage } from '../environment';
-import { DAY, execRegexes, watchForThings, Thing } from '../utils';
+import { execRegexes, maybePruneOldEntries, watchForThings, Thing } from '../utils';
 
 export const module: Module<*> = new Module('commentHidePersistor');
 
@@ -17,7 +17,6 @@ module.include = [
 ];
 
 const currentId: string = (execRegexes.comments(location.pathname) || [])[2];
-const lastCleanStorage = Storage.wrap('RESmodules.commentHidePersistor.lastClean', 0);
 const entryStorage = Storage.wrapPrefix('commentHidePersistor.', (): {
 	updateTime: number,
 	collapsedThings?: { [fullName: string]: boolean },
@@ -33,25 +32,10 @@ module.beforeLoad = async () => {
 
 module.go = () => {
 	listenToCommentCollapse();
-};
 
-module.afterLoad = () => {
-	maybePruneOldEntries();
+	maybePruneOldEntries('commentHidePersistor', entryStorage);
 };
 
-async function maybePruneOldEntries() {
-	// Clean counts every six hours
-	const now = Date.now();
-	if ((now - await lastCleanStorage.get()) < 0.25 * DAY) return;
-	lastCleanStorage.set(now);
-
-	for (const [id, data] of Object.entries(await entryStorage.getAll())) {
-		if ((now - data.updateTime) > 7 * DAY) {
-			entryStorage.delete(id);
-		}
-	}
-}
-
 const COLLAPSE_REASON = 'commentHidePersistor';
 
 async function restoreCommentCollapse() {

diff --git a/lib/modules/newCommentCount.js b/lib/modules/newCommentCount.js
@@ -15,6 +15,7 @@ import {
 	getPostMetadata,
 	isCurrentSubreddit,
 	isPageType,
+	maybePruneOldEntries,
 	loggedInUser,
 	regexes,
 	string,
@@ -41,9 +42,10 @@ module.options = {
 	},
 	cleanComments: {
 		type: 'text',
-		value: '7',
+		value: '30',
 		description: 'newCommentCountCleanCommentsDesc',
 		title: 'newCommentCountCleanCommentsTitle',
+		advanced: true,
 	},
 	subscriptionLength: {
 		type: 'text',
@@ -82,7 +84,6 @@ module.options = {
 	},
 };
 
-const lastCleanStorage = Storage.wrap('RESmodules.newCommentCount.lastClean', 0);
 const entryStorage = Storage.wrapPrefix('newCommentCount.', (): {|
 	count: number,
 	updateTime: number,
@@ -119,6 +120,8 @@ module.go = () => {
 	if (isCurrentSubreddit('dashboard')) {
 		addDashboardFunctionality();
 	}
+
+	maybePruneOldEntries('newCommentCount', entryStorage, parseInt(module.options.cleanComments.value, 10));
 };
 
 let listingThing: ?Thing;
@@ -143,8 +146,6 @@ module.afterLoad = () => {
 			}
 		}
 	}
-
-	maybePruneOldEntries();
 };
 
 const getId = thing => thing.getFullname().split('_').slice(-1)[0];
@@ -199,20 +200,6 @@ function updateCurrentCommentCountFromMyComment(thing) {
 	}
 }
 
-async function maybePruneOldEntries() {
-	// Clean counts every six hours
-	const now = Date.now();
-	if ((now - await lastCleanStorage.get()) < 0.25 * DAY) return;
-	lastCleanStorage.set(now);
-
-	const keepTrackPeriod = DAY * parseInt(module.options.cleanComments.value, 10);
-	for (const [id, { updateTime }] of Object.entries(await entryStorage.getAll())) {
-		if ((now - updateTime) > keepTrackPeriod) {
-			stopTracking(id);
-		}
-	}
-}
-
 const refreshSubscriptionButton = mutex(async () => {
 	const id = (execRegexes.comments(location.pathname) || [])[2];
 	const button = subscriptionButton;
@@ -274,10 +261,6 @@ function unsubscribe(id) {
 	return subscriptionStorage.delete(id);
 }
 
-function stopTracking(id) {
-	return entryStorage.delete(id);
-}
-
 async function checkSubscriptions() {
 	const now = Date.now();
 

diff --git a/lib/modules/readComments.js b/lib/modules/readComments.js
@@ -3,11 +3,10 @@
 import { Module } from '../core/module';
 import { Storage, isPrivateBrowsing } from '../environment';
 import {
-	DAY,
-	WEEK,
 	execRegexes,
 	reifyPromise,
 	fastAsync,
+	maybePruneOldEntries,
 } from '../utils';
 import * as SelectedEntry from './selectedEntry';
 
@@ -22,6 +21,7 @@ module.options = {
 		value: '30',
 		description: 'readCommentsCleanCommentsDesc',
 		title: 'readCommentsCleanCommentsTitle',
+		advanced: true,
 	},
 	monitorWhenIncognito: {
 		type: 'boolean',
@@ -35,7 +35,6 @@ module.options = {
 module.include = ['comments', 'commentsLinklist'];
 
 const currentId = (execRegexes.comments(location.pathname) || [])[2] || location.pathname;
-const lastCleanStorage = Storage.wrap('RESmodules.readComments.lastClean', 0);
 const entryStorage = Storage.wrapPrefix('readComments.', (): {|
 	updateTime: number,
 	ids: { [string]: true },
@@ -59,25 +58,10 @@ module.beforeLoad = async () => {
 			entryStorage.patch(currentId, { ids: { [id]: true }, updateTime: Date.now() });
 		}
 	}, 'beforePaint');
-};
 
-module.afterLoad = () => {
-	maybePruneOldEntries();
+	maybePruneOldEntries('readComments', entryStorage, parseInt(module.options.cleanComments.value, 10));
 };
 
-async function maybePruneOldEntries() {
-	const now = Date.now();
-	if ((now - await lastCleanStorage.get()) < WEEK) return;
-	lastCleanStorage.set(now);
-
-	const keepTrackPeriod = DAY * parseInt(module.options.cleanComments.value, 10);
-	for (const [id, { updateTime }] of Object.entries(await entryStorage.getAll())) {
-		if ((now - updateTime) > keepTrackPeriod) {
-			entryStorage.delete(id);
-		}
-	}
-}
-
 export const isRead = fastAsync(function*(thing: *): * {
 	if (!initialStorage) return false;
 	const ids = yield initialStorage.get();

diff --git a/lib/utils/index.js b/lib/utils/index.js
@@ -144,6 +144,11 @@ export {
 	markEnd,
 } from './profiling';
 
+export {
+	maybePruneOldEntries,
+	shouldPrune,
+} from './storage';
+
 export {
 	DAY,
 	HOUR,

diff --git a/lib/utils/storage.js b/lib/utils/storage.js
@@ -0,0 +1,25 @@
+/* @flow */
+
+import { Storage } from '../environment';
+import { DAY, WEEK } from './time';
+
+export async function maybePruneOldEntries(id: string, entryStorage: *, keepTrackDays: number = 30) {
+	if (!await shouldPrune(id)) return;
+
+	const keepTrackPeriod = DAY * keepTrackDays;
+	const now = Date.now();
+	for (const [id, { updateTime } = {}] of Object.entries(await entryStorage.getAll())) {
+		if (!updateTime || (now - updateTime) > keepTrackPeriod) {
+			entryStorage.delete(id);
+		}
+	}
+}
+
+export async function shouldPrune(id: string, interval: number = WEEK) {
+	await new Promise(res => requestIdleCallback(res));
+	const lastStorage = Storage.wrap(`last_prune.${id}`, 0);
+	const now = Date.now();
+	if ((now - await lastStorage.get()) < interval) return false;
+	lastStorage.set(now);
+	return true;
+}