AdNauseam icon indicating copy to clipboard operation
AdNauseam copied to clipboard

Performance improvement on local storage updates

Open cqx931 opened this issue 4 years ago • 8 comments

Background: every time storeUserData() is called, µb.userSettings.admap will be updated with the latest admap in core.js, then it will set the whole µb.userSettings into local storage. For large data set(Ex: > 20k ads) this means something over 60MB.

This issue shall be addressed in two stages:

Stage 1: Temporary solution without changing the admap

  1. Remove storeUserData() from functions associated with ad clicks and ad deletions (deleteAd, PostRegister, updateAdOnSccess)
  2. Remove potential duplicate calls of storeUserData(): importads() and repairHashes()
  3. Remove admap from µb.userSettings and store it seperately
  4. Once step 3 is done, make corresponding changes in storeUserData() and initUserSettings().Also need to check messaging for other potential changes for this

Stage 2: Incrementally update the local storage as we go, without re-storing all the data ...perhaps we have 3 data structures in localstorage: 1) userSettings, 2) admap, 3) admapUpdates

1 and 3 are updated regularly whenever they change (they are tiny), but 2 admap is ONLY ever updated on extensionInit, when it is merged with the adMapUpdates. Then these adMapUpdates are deleted from localStorage and we start with an empty set, to be added to over the session. What goes into adMapUpdates is simply an array of all the ads whose data has changed during the session

Questions:

  1. When the hash would need to be created? perhaps just when overwriting the old in the admap with the newer one in adMapUpdates

cqx931 avatar Aug 18 '20 23:08 cqx931

looks good - can we get stage one into this release ?

dhowe avatar Aug 19 '20 13:08 dhowe

A summary on ublock's latest api options on local storage:

vAPI.storage

  • This is only available for background

  • It's the same as webext.storage.local

vAPI.localStorage

  • A localStorage-like object which should be accessible from the background page or auxiliary pages.
  • Mainly used for consistent UI settings

µBlock.cacheStorage

  • Ublock's implementation of indexedDB
  • Mainly used for filters asset -> filters cache

cqx931 avatar Aug 19 '20 16:08 cqx931

A new attempt for stage 1: https://github.com/dhowe/AdNauseam/pull/1716

Changes:

  • Separate admap from µb.userSettings, changed the function name storeUserData() to storeAdData()
  • For initialize, import and clear ads, storeAdData(true) is used to update storage immediately for these situations
  • Added storeAdData(true) if private ads are removed
  • For ad collected, ad clicks, delete ad, storeAdData() is used so that an storage update will only occur if total ad count is less than 1000 or it's already more than 30 minutes from the last storage update.
  • Still consider reading admap from settings during initialization for version migration

cqx931 avatar Aug 19 '20 21:08 cqx931

Need to verify that:

  1. storeAdData (and probably storeUserData) are called on extension shutdown (quit-browser)
  2. storeAdData is still called regularly (at least once per hour with current values)

dhowe avatar Aug 20 '20 05:08 dhowe

I don't think we can detect browser quit as for now.

There is an experimental API chrome.process but that's only available for dev channel.

See also: https://stackoverflow.com/questions/3390470/event-onbrowserclose-for-google-chrome-extension

cqx931 avatar Aug 20 '20 21:08 cqx931

alternatively we could do it on startup (though this isn't as nice)

dhowe avatar Aug 21 '20 10:08 dhowe

@dhowe I'm not sure how you could backup the data on startup... aren't the data already lost from the last session?

cqx931 avatar Aug 21 '20 13:08 cqx931

right, I guess I was thinking of deletes. anyhow, this is problematic as users will lose ads and there total/cost will potentially go down after quit/restart

which argues for implementing the incremental approach

dhowe avatar Aug 22 '20 05:08 dhowe