Skip to content

Commit b8ab95a

Browse files
chenglouchenglou
committed
Merge pull request facebook#2054 from chenglou/accum
[RFC] Use `accumulateInto` to save even more allocation
2 parents ef47bb7 + 48e901f commit b8ab95a

6 files changed

Lines changed: 152 additions & 16 deletions

File tree

‎src/browser/eventPlugins/ResponderEventPlugin.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ var EventPluginUtils = require('EventPluginUtils');
2323
var EventPropagators = require('EventPropagators');
2424
var SyntheticEvent = require('SyntheticEvent');
2525

26-
var accumulate = require('accumulate');
26+
var accumulateInto = require('accumulateInto');
2727
var keyOf = require('keyOf');
2828

2929
var isStartish = EventPluginUtils.isStartish;
@@ -212,7 +212,7 @@ function setResponderAndExtractTransfer(
212212
nativeEvent
213213
);
214214
EventPropagators.accumulateDirectDispatches(terminateEvent);
215-
extracted = accumulate(extracted, [grantEvent, terminateEvent]);
215+
extracted = accumulateInto(extracted, [grantEvent, terminateEvent]);
216216
responderID = wantsResponderID;
217217
} else {
218218
var rejectEvent = SyntheticEvent.getPooled(
@@ -221,10 +221,10 @@ function setResponderAndExtractTransfer(
221221
nativeEvent
222222
);
223223
EventPropagators.accumulateDirectDispatches(rejectEvent);
224-
extracted = accumulate(extracted, rejectEvent);
224+
extracted = accumulateInto(extracted, rejectEvent);
225225
}
226226
} else {
227-
extracted = accumulate(extracted, grantEvent);
227+
extracted = accumulateInto(extracted, grantEvent);
228228
responderID = wantsResponderID;
229229
}
230230
return extracted;
@@ -288,7 +288,7 @@ var ResponderEventPlugin = {
288288
nativeEvent
289289
);
290290
if (transfer) {
291-
extracted = accumulate(extracted, transfer);
291+
extracted = accumulateInto(extracted, transfer);
292292
}
293293
}
294294
// Now that we know the responder is set correctly, we can dispatch
@@ -303,7 +303,7 @@ var ResponderEventPlugin = {
303303
nativeEvent
304304
);
305305
EventPropagators.accumulateDirectDispatches(gesture);
306-
extracted = accumulate(extracted, gesture);
306+
extracted = accumulateInto(extracted, gesture);
307307
}
308308
if (type === eventTypes.responderRelease) {
309309
responderID = null;

‎src/browser/ui/dom/components/LocalEventTrapMixin.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
2222

23-
var accumulate = require('accumulate');
23+
var accumulateInto = require('accumulateInto');
2424
var forEachAccumulated = require('forEachAccumulated');
2525
var invariant = require('invariant');
2626

@@ -36,7 +36,8 @@ var LocalEventTrapMixin = {
3636
handlerBaseName,
3737
this.getDOMNode()
3838
);
39-
this._localEventListeners = accumulate(this._localEventListeners, listener);
39+
this._localEventListeners =
40+
accumulateInto(this._localEventListeners, listener);
4041
},
4142

4243
// trapCapturedEvent would look nearly identical. We don't implement that

‎src/event/EventPluginHub.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
var EventPluginRegistry = require('EventPluginRegistry');
2222
var EventPluginUtils = require('EventPluginUtils');
2323

24-
var accumulate = require('accumulate');
24+
var accumulateInto = require('accumulateInto');
2525
var forEachAccumulated = require('forEachAccumulated');
2626
var invariant = require('invariant');
2727
var isEventSupported = require('isEventSupported');
@@ -236,7 +236,7 @@ var EventPluginHub = {
236236
nativeEvent
237237
);
238238
if (extractedEvents) {
239-
events = accumulate(events, extractedEvents);
239+
events = accumulateInto(events, extractedEvents);
240240
}
241241
}
242242
}
@@ -252,7 +252,7 @@ var EventPluginHub = {
252252
*/
253253
enqueueEvents: function(events) {
254254
if (events) {
255-
eventQueue = accumulate(eventQueue, events);
255+
eventQueue = accumulateInto(eventQueue, events);
256256
}
257257
},
258258

‎src/event/EventPropagators.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
var EventConstants = require('EventConstants');
2222
var EventPluginHub = require('EventPluginHub');
2323

24-
var accumulate = require('accumulate');
24+
var accumulateInto = require('accumulateInto');
2525
var forEachAccumulated = require('forEachAccumulated');
2626

2727
var PropagationPhases = EventConstants.PropagationPhases;
@@ -52,8 +52,9 @@ function accumulateDirectionalDispatches(domID, upwards, event) {
5252
var phase = upwards ? PropagationPhases.bubbled : PropagationPhases.captured;
5353
var listener = listenerAtPhase(domID, event, phase);
5454
if (listener) {
55-
event._dispatchListeners = accumulate(event._dispatchListeners, listener);
56-
event._dispatchIDs = accumulate(event._dispatchIDs, domID);
55+
event._dispatchListeners =
56+
accumulateInto(event._dispatchListeners, listener);
57+
event._dispatchIDs = accumulateInto(event._dispatchIDs, domID);
5758
}
5859
}
5960

@@ -85,8 +86,9 @@ function accumulateDispatches(id, ignoredDirection, event) {
8586
var registrationName = event.dispatchConfig.registrationName;
8687
var listener = getListener(id, registrationName);
8788
if (listener) {
88-
event._dispatchListeners = accumulate(event._dispatchListeners, listener);
89-
event._dispatchIDs = accumulate(event._dispatchIDs, id);
89+
event._dispatchListeners =
90+
accumulateInto(event._dispatchListeners, listener);
91+
event._dispatchIDs = accumulateInto(event._dispatchIDs, id);
9092
}
9193
}
9294
}

‎src/utils/__tests__/accumulateInto-test.js‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/**
2+
* Copyright 2014 Facebook, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* @emails react-core
17+
* @jsx React.DOM
18+
*/
19+
20+
"use strict";
21+
22+
require('mock-modules')
23+
.dontMock('accumulateInto');
24+
25+
var accumulateInto;
26+
27+
describe('accumulateInto', function() {
28+
29+
beforeEach(function() {
30+
accumulateInto = require('accumulateInto');
31+
});
32+
33+
it('throws if the second item is null', function() {
34+
expect(function() {
35+
accumulateInto([], null);
36+
}).toThrow(
37+
'Invariant Violation: accumulateInto(...): Accumulated items must not ' +
38+
'be null or undefined.'
39+
);
40+
});
41+
42+
it('returns the second item if first is null', function() {
43+
var a = [];
44+
expect(accumulateInto(null, a)).toBe(a);
45+
});
46+
47+
it('merges the second into the first if first item is an array', function() {
48+
var a = [1, 2];
49+
var b = [3, 4];
50+
accumulateInto(a, b);
51+
expect(a).toEqual([1, 2, 3, 4]);
52+
expect(b).toEqual([3, 4]);
53+
var c = [1];
54+
accumulateInto(c, 2);
55+
expect(c).toEqual([1, 2]);
56+
});
57+
58+
it('returns a new array if first or both items are scalar', function() {
59+
var a = [2];
60+
expect(accumulateInto(1, a)).toEqual([1, 2]);
61+
expect(a).toEqual([2]);
62+
expect(accumulateInto(1, 2)).toEqual([1, 2]);
63+
});
64+
});

‎src/utils/accumulateInto.js‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/**
2+
* Copyright 2014 Facebook, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* @providesModule accumulateInto
17+
*/
18+
19+
"use strict";
20+
21+
var invariant = require('invariant');
22+
23+
/**
24+
*
25+
* Accumulates items that must not be null or undefined into the first one. This
26+
* is used to conserve memory by avoiding array allocations, and thus sacrifices
27+
* API cleanness. Since `current` can be null before being passed in and not
28+
* null after this function, make sure to assign it back to `current`:
29+
*
30+
* `a = accumulateInto(a, b);`
31+
*
32+
* This API should be sparingly used. Try `accumulate` for something cleaner.
33+
*
34+
* @return {*|array<*>} An accumulation of items.
35+
*/
36+
37+
function accumulateInto(current, next) {
38+
invariant(
39+
next != null,
40+
'accumulateInto(...): Accumulated items must not be null or undefined.'
41+
);
42+
if (current == null) {
43+
return next;
44+
}
45+
46+
// Both are not empty. Warning: Never call x.concat(y) when you are not
47+
// certain that x is an Array (x could be a string with concat method).
48+
var currentIsArray = Array.isArray(current);
49+
var nextIsArray = Array.isArray(next);
50+
51+
if (currentIsArray && nextIsArray) {
52+
current.push.apply(current, next);
53+
return current;
54+
}
55+
56+
if (currentIsArray) {
57+
current.push(next);
58+
return current;
59+
}
60+
61+
if (nextIsArray) {
62+
// A bit too dangerous to mutate `next`.
63+
return [current].concat(next);
64+
}
65+
66+
return [current, next];
67+
}
68+
69+
module.exports = accumulateInto;

0 commit comments

Comments
 (0)