From 64e8ce0c07959bf3415fe36d57234527f1e1dea9 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 27 Aug 2015 19:01:34 +0800 Subject: [PATCH] Don't rely on IDWeakMap for bookkeeping remote objects It frees us from using C++ to track JS objects, thus improves the performance of collecting memory. --- atom/browser/lib/objects-registry.coffee | 115 ++++++++++------------- atom/browser/lib/rpc-server.coffee | 6 +- atom/common/api/atom_api_v8_util.cc | 6 ++ atom/renderer/api/lib/remote.coffee | 2 +- 4 files changed, 59 insertions(+), 70 deletions(-) diff --git a/atom/browser/lib/objects-registry.coffee b/atom/browser/lib/objects-registry.coffee index f102cbef894e..ccfe2dbe0ad2 100644 --- a/atom/browser/lib/objects-registry.coffee +++ b/atom/browser/lib/objects-registry.coffee @@ -1,82 +1,65 @@ EventEmitter = require('events').EventEmitter -IDWeakMap = process.atomBinding('id_weak_map').IDWeakMap v8Util = process.atomBinding 'v8_util' -# Class to reference all objects. -class ObjectsStore - @stores = {} - - constructor: -> - @nextId = 0 - @objects = [] - - getNextId: -> - ++@nextId - - add: (obj) -> - id = @getNextId() - @objects[id] = obj - id - - has: (id) -> - @objects[id]? - - remove: (id) -> - throw new Error("Invalid key #{id} for ObjectsStore") unless @has id - delete @objects[id] - - get: (id) -> - throw new Error("Invalid key #{id} for ObjectsStore") unless @has id - @objects[id] - - @forRenderView: (key) -> - @stores[key] = new ObjectsStore unless @stores[key]? - @stores[key] - - @releaseForRenderView: (key) -> - delete @stores[key] - class ObjectsRegistry extends EventEmitter constructor: -> @setMaxListeners Number.MAX_VALUE + @nextId = 0 - # Objects in weak map will be not referenced (so we won't leak memory), and - # every object created in browser will have a unique id in weak map. - @objectsWeakMap = new IDWeakMap - @objectsWeakMap.add = (obj) -> - id = IDWeakMap::add.call this, obj - v8Util.setHiddenValue obj, 'atomId', id - id + # Stores all objects by ref-counting. + # (id) => {object, count} + @storage = {} + + # Stores the IDs of objects referenced by WebContents. + # (webContentsId) => {(id) => (count)} + @owners = {} # Register a new object, the object would be kept referenced until you release # it explicitly. - add: (key, obj) -> - # Some native objects may already been added to objectsWeakMap, be care not - # to add it twice. - @objectsWeakMap.add obj unless v8Util.getHiddenValue obj, 'atomId' - id = v8Util.getHiddenValue obj, 'atomId' + add: (webContentsId, obj) -> + id = @saveToStorage obj + # Remember the owner. + @owners[webContentsId] ?= {} + @owners[webContentsId][id] ?= 0 + @owners[webContentsId][id]++ + # Returns object's id + id - # Store and reference the object, then return the storeId which points to - # where the object is stored. The caller can later dereference the object - # with the storeId. - # We use a difference key because the same object can be referenced for - # multiple times by the same renderer view. - store = ObjectsStore.forRenderView key - storeId = store.add obj - - [id, storeId] - - # Get an object according to its id. + # Get an object according to its ID. get: (id) -> - @objectsWeakMap.get id + @storage[id]?.object - # Remove an object according to its storeId. - remove: (key, storeId) -> - ObjectsStore.forRenderView(key).remove storeId + # Dereference an object according to its ID. + remove: (webContentsId, id) -> + @dereference id, 1 + # Also reduce the count in owner. + pointer = @owners[webContentsId] + --pointer[id] + delete pointer[id] if pointer[id] is 0 - # Clear all references to objects from renderer view. - clear: (key) -> - @emit "clear-#{key}" - ObjectsStore.releaseForRenderView key + # Clear all references to objects refrenced by the WebContents. + clear: (webContentsId) -> + @emit "clear-#{webContentsId}" + return unless @owners[webContentsId]? + @dereference id, count for id, count of @owners[webContentsId] + delete @owners[webContentsId] + + # Private: Saves the object into storage and assigns an ID for it. + saveToStorage: (object) -> + id = v8Util.getHiddenValue object, 'atomId' + unless id + id = ++@nextId + @storage[id] = {count: 0, object} + v8Util.setHiddenValue object, 'atomId', id + ++@storage[id].count + id + + # Private: Dereference the object from store. + dereference: (id, count) -> + pointer = @storage[id] + pointer.count -= count + if pointer.count is 0 + v8Util.deleteHiddenValue pointer.object, 'atomId' + delete @storage[id] module.exports = new ObjectsRegistry diff --git a/atom/browser/lib/rpc-server.coffee b/atom/browser/lib/rpc-server.coffee index d19b5cf0659c..0a28d350e8a9 100644 --- a/atom/browser/lib/rpc-server.coffee +++ b/atom/browser/lib/rpc-server.coffee @@ -28,7 +28,7 @@ valueToMeta = (sender, value, optimizeSimpleObject=false) -> # Reference the original value if it's an object, because when it's # passed to renderer we would assume the renderer keeps a reference of # it. - [meta.id, meta.storeId] = objectsRegistry.add sender.getId(), value + meta.id = objectsRegistry.add sender.getId(), value meta.members = [] meta.members.push {name: prop, type: typeof field} for prop, field of value @@ -174,8 +174,8 @@ ipc.on 'ATOM_BROWSER_MEMBER_GET', (event, id, name) -> catch e event.returnValue = errorToMeta e -ipc.on 'ATOM_BROWSER_DEREFERENCE', (event, storeId) -> - objectsRegistry.remove event.sender.getId(), storeId +ipc.on 'ATOM_BROWSER_DEREFERENCE', (event, id) -> + objectsRegistry.remove event.sender.getId(), id ipc.on 'ATOM_BROWSER_GUEST_WEB_CONTENTS', (event, guestInstanceId) -> try diff --git a/atom/common/api/atom_api_v8_util.cc b/atom/common/api/atom_api_v8_util.cc index ae90fe1f37fc..21f23a97b456 100644 --- a/atom/common/api/atom_api_v8_util.cc +++ b/atom/common/api/atom_api_v8_util.cc @@ -28,6 +28,11 @@ void SetHiddenValue(v8::Local object, object->SetHiddenValue(key, value); } +void DeleteHiddenValue(v8::Local object, + v8::Local key) { + object->DeleteHiddenValue(key); +} + int32_t GetObjectHash(v8::Local object) { return object->GetIdentityHash(); } @@ -48,6 +53,7 @@ void Initialize(v8::Local exports, v8::Local unused, dict.SetMethod("createObjectWithName", &CreateObjectWithName); dict.SetMethod("getHiddenValue", &GetHiddenValue); dict.SetMethod("setHiddenValue", &SetHiddenValue); + dict.SetMethod("deleteHiddenValue", &DeleteHiddenValue); dict.SetMethod("getObjectHash", &GetObjectHash); dict.SetMethod("setDestructor", &SetDestructor); dict.SetMethod("takeHeapSnapshot", &TakeHeapSnapshot); diff --git a/atom/renderer/api/lib/remote.coffee b/atom/renderer/api/lib/remote.coffee index 29a24b1789ef..abd86e7eee0c 100644 --- a/atom/renderer/api/lib/remote.coffee +++ b/atom/renderer/api/lib/remote.coffee @@ -102,7 +102,7 @@ metaToValue = (meta) -> # Track delegate object's life time, and tell the browser to clean up # when the object is GCed. v8Util.setDestructor ret, -> - ipc.send 'ATOM_BROWSER_DEREFERENCE', meta.storeId + ipc.send 'ATOM_BROWSER_DEREFERENCE', meta.id # Remember object's id. v8Util.setHiddenValue ret, 'atomId', meta.id