From 28164da61956a2d7286b3b2871e3b48b7616d670 Mon Sep 17 00:00:00 2001 From: Edward Lee Date: Fri, 5 Mar 2010 14:43:11 -0800 Subject: [PATCH] Bug 549632 - Remove storage cache, which is mostly un/incorrectly used [r=mconnor] Remove incorrectly used cache from some engines and clean up references from SyncEngine. --- services/sync/modules/engines.js | 17 ----- services/sync/modules/engines/bookmarks.js | 9 +-- services/sync/modules/engines/history.js | 2 +- services/sync/modules/stores.js | 79 ---------------------- 4 files changed, 2 insertions(+), 105 deletions(-) diff --git a/services/sync/modules/engines.js b/services/sync/modules/engines.js index 3fa33e299507..48abea913d17 100644 --- a/services/sync/modules/engines.js +++ b/services/sync/modules/engines.js @@ -407,13 +407,6 @@ SyncEngine.prototype = { if (Svc.Prefs.get("client.type") == "mobile") fetchNum = 50; - // enable cache, and keep only the first few items. Otherwise (when - // we have more outgoing items than can fit in the cache), we will - // keep rotating items in and out, perpetually getting cache misses - this._store.cache.enabled = true; - this._store.cache.fifo = false; // filo - this._store.cache.clear(); - let newitems = new Collection(this.engineURL, this._recordObj); newitems.newer = this.lastSync; newitems.full = true; @@ -502,9 +495,6 @@ SyncEngine.prototype = { this._log.info(["Records:", count.applied, "applied,", count.reconciled, "reconciled,", this.toFetch.length, "left to fetch"].join(" ")); - - // try to free some memory - this._store.cache.clear(); }, /** @@ -554,8 +544,6 @@ SyncEngine.prototype = { this._store.changeItemID(dupeId, item.id); this._deleteId(dupeId); } - - this._store.cache.clear(); // because parentid refs will be wrong }, // Reconciliation has three steps: @@ -632,9 +620,6 @@ SyncEngine.prototype = { up.clearRecords(); }); - // don't cache the outgoing items, we won't need them later - this._store.cache.enabled = false; - for (let id in this._tracker.changedIDs) { try { let out = this._createRecord(id); @@ -658,8 +643,6 @@ SyncEngine.prototype = { // Final upload if (count % MAX_UPLOAD_RECORDS > 0) doUpload(count >= MAX_UPLOAD_RECORDS ? "last batch" : "all"); - - this._store.cache.enabled = true; } this._tracker.clearChangedIDs(); }, diff --git a/services/sync/modules/engines/bookmarks.js b/services/sync/modules/engines/bookmarks.js index 20d9f676f3a8..bfb138ddba74 100644 --- a/services/sync/modules/engines/bookmarks.js +++ b/services/sync/modules/engines/bookmarks.js @@ -638,9 +638,8 @@ BookmarksStore.prototype = { }, changeItemID: function BStore_changeItemID(oldID, newID) { - // Remember the GUID change for incoming records and avoid invalid refs + // Remember the GUID change for incoming records this.aliases[oldID] = newID; - this.cache.clear(); // Update any existing annotation references this._findAnnoItems(PARENT_ANNO, oldID).forEach(function(itemId) { @@ -706,16 +705,11 @@ BookmarksStore.prototype = { // Create a record starting from the weave id (places guid) createRecord: function BStore_createRecord(guid, cryptoMetaURL) { - let record = this.cache.get(guid); - if (record) - return record; - let placeId = idForGUID(guid); if (placeId <= 0) { // deleted item record = new PlacesItem(); record.id = guid; record.deleted = true; - this.cache.put(guid, record); return record; } @@ -799,7 +793,6 @@ BookmarksStore.prototype = { record.encryption = cryptoMetaURL; record.sortindex = this._calculateIndex(record); - this.cache.put(guid, record); return record; }, diff --git a/services/sync/modules/engines/history.js b/services/sync/modules/engines/history.js index 4481aab704d2..c1ef22a2073a 100644 --- a/services/sync/modules/engines/history.js +++ b/services/sync/modules/engines/history.js @@ -262,7 +262,7 @@ HistoryStore.prototype = { } else record.deleted = true; - this.cache.put(guid, record); + return record; }, diff --git a/services/sync/modules/stores.js b/services/sync/modules/stores.js index 148ef4728001..da15572f034f 100644 --- a/services/sync/modules/stores.js +++ b/services/sync/modules/stores.js @@ -59,12 +59,6 @@ function Store(name) { this._log.level = Log4Moz.Level[level]; } Store.prototype = { - get cache() { - let cache = new Cache(); - this.__defineGetter__("cache", function() cache); - return cache; - }, - applyIncoming: function Store_applyIncoming(record) { if (record.deleted) this.remove(record); @@ -108,76 +102,3 @@ Store.prototype = { throw "override wipe in a subclass"; } }; - -function Cache() { - this.count = 0; - this.maxItems = 100; - this.fifo = true; - this.enabled = true; - this._head = this._tail = null; - this._items = {}; -} -Cache.prototype = { - remove: function Cache_remove(item) { - if (this.count <= 0 || this.count == 1) { - this.clear(); - return; - } - - item.next.prev = item.prev; - item.prev.next = item.next; - - if (item == this._head) - this._head = item.next; - if (item == this._tail) - this._tail = item.prev; - - item.next = null; - item.prev = null; - - delete this._items[item.id]; - this.count--; - }, - pop: function Cache_pop() { - if (this.fifo) - this.remove(this._tail); - else - this.remove(this._head); - }, - put: function Cache_put(id, item) { - if (!this.enabled) - return; - - let wrapper = {id: id, item: item}; - - if (this._head === null) { - wrapper.next = wrapper; - wrapper.prev = wrapper; - this._head = wrapper; - this._tail = wrapper; - } else { - wrapper.next = this._tail; - wrapper.prev = this._head; - this._head.prev = wrapper; - this._tail.next = wrapper; - this._tail = wrapper; - } - - this._items[wrapper.id] = wrapper; - this.count++; - - if (this.count >= this.maxItems) - this.pop(); - }, - get: function Cache_get(id) { - if (id in this._items) - return this._items[id].item; - return undefined; - }, - clear: function Cache_clear() { - this.count = 0; - this._head = null; - this._tail = null; - this._items = {}; - } -};