Skip to content

Commit

Permalink
Fix unmounting performance regression in V8 (#7232)
Browse files Browse the repository at this point in the history
As reported in #7227, unmounting performance regressed with React 15.
It seems that `delete` has become much slower since we started using numeric keys.
Forcing dictionary keys to start with a dot fixes the issue.
  • Loading branch information
gaearon authored Jul 9, 2016
1 parent b6e1eb2 commit 64e7602
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 13 deletions.
16 changes: 10 additions & 6 deletions src/renderers/dom/client/eventPlugins/SimpleEventPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,10 @@ for (var type in topLevelEventsToDispatchConfig) {
var ON_CLICK_KEY = keyOf({onClick: null});
var onClickListeners = {};

function getDictionaryKey(inst) {
return '.' + inst._rootNodeID;
}

var SimpleEventPlugin = {

eventTypes: eventTypes,
Expand Down Expand Up @@ -620,10 +624,10 @@ var SimpleEventPlugin = {
// fire. The workaround for this bug involves attaching an empty click
// listener on the target node.
if (registrationName === ON_CLICK_KEY) {
var id = inst._rootNodeID;
var key = getDictionaryKey(inst);
var node = ReactDOMComponentTree.getNodeFromInstance(inst);
if (!onClickListeners[id]) {
onClickListeners[id] = EventListener.listen(
if (!onClickListeners[key]) {
onClickListeners[key] = EventListener.listen(
node,
'click',
emptyFunction
Expand All @@ -634,9 +638,9 @@ var SimpleEventPlugin = {

willDeleteListener: function(inst, registrationName) {
if (registrationName === ON_CLICK_KEY) {
var id = inst._rootNodeID;
onClickListeners[id].remove();
delete onClickListeners[id];
var key = getDictionaryKey(inst);
onClickListeners[key].remove();
delete onClickListeners[key];
}
},

Expand Down
22 changes: 15 additions & 7 deletions src/renderers/shared/stack/event/EventPluginHub.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ var executeDispatchesAndReleaseTopLevel = function(e) {
return executeDispatchesAndRelease(e, false);
};

var getDictionaryKey = function(inst) {
return '.' + inst._rootNodeID;
};

/**
* This is a unified interface for event plugins to be installed and configured.
*
Expand Down Expand Up @@ -96,7 +100,7 @@ var EventPluginHub = {
},

/**
* Stores `listener` at `listenerBank[registrationName][id]`. Is idempotent.
* Stores `listener` at `listenerBank[registrationName][key]`. Is idempotent.
*
* @param {object} inst The instance, which is the source of events.
* @param {string} registrationName Name of listener (e.g. `onClick`).
Expand All @@ -109,9 +113,10 @@ var EventPluginHub = {
registrationName, typeof listener
);

var key = getDictionaryKey(inst);
var bankForRegistrationName =
listenerBank[registrationName] || (listenerBank[registrationName] = {});
bankForRegistrationName[inst._rootNodeID] = listener;
bankForRegistrationName[key] = listener;

var PluginModule =
EventPluginRegistry.registrationNameModules[registrationName];
Expand All @@ -127,7 +132,8 @@ var EventPluginHub = {
*/
getListener: function(inst, registrationName) {
var bankForRegistrationName = listenerBank[registrationName];
return bankForRegistrationName && bankForRegistrationName[inst._rootNodeID];
var key = getDictionaryKey(inst);
return bankForRegistrationName && bankForRegistrationName[key];
},

/**
Expand All @@ -146,7 +152,8 @@ var EventPluginHub = {
var bankForRegistrationName = listenerBank[registrationName];
// TODO: This should never be null -- when is it?
if (bankForRegistrationName) {
delete bankForRegistrationName[inst._rootNodeID];
var key = getDictionaryKey(inst);
delete bankForRegistrationName[key];
}
},

Expand All @@ -156,12 +163,13 @@ var EventPluginHub = {
* @param {object} inst The instance, which is the source of events.
*/
deleteAllListeners: function(inst) {
var key = getDictionaryKey(inst);
for (var registrationName in listenerBank) {
if (!listenerBank.hasOwnProperty(registrationName)) {
continue;
}
if (!listenerBank[registrationName][inst._rootNodeID]) {

if (!listenerBank[registrationName][key]) {
continue;
}

Expand All @@ -171,7 +179,7 @@ var EventPluginHub = {
PluginModule.willDeleteListener(inst, registrationName);
}

delete listenerBank[registrationName][inst._rootNodeID];
delete listenerBank[registrationName][key];
}
},

Expand Down

0 comments on commit 64e7602

Please sign in to comment.