[RFC] Use `accumulateInto` to save even more allocation
Trying to make the event a bit more performant for events. Feel free to reject this because the API inevitably isn't great. It's good for perf though, and since we're only using `accumulate` in very restrained places, I think we're fine. `accumulateInto` is `accumulate` that mutates more and allocates less. I kept `accumulate` in case we want that in the future.
Cheng Lou committed
Aug 15, 2014 at 20:56 UTC
48e901f8ae8c428351684736ecaabcb12d926df5
6 files changed
+152
-16
src/browser/eventPlugins/ResponderEventPlugin.js
+6
-6
@@ -23,7 +23,7 @@ var EventPluginUtils = require('EventPluginUtils');
23
var EventPropagators = require('EventPropagators');
24
var SyntheticEvent = require('SyntheticEvent');
25
26
-var accumulate = require('accumulate');
26
+var accumulateInto = require('accumulateInto');
27
var keyOf = require('keyOf');
28
29
var isStartish = EventPluginUtils.isStartish;
@@ -212,7 +212,7 @@ function setResponderAndExtractTransfer(
212
nativeEvent
213
);
214
EventPropagators.accumulateDirectDispatches(terminateEvent);
215
- extracted = accumulate(extracted, [grantEvent, terminateEvent]);
215
+ extracted = accumulateInto(extracted, [grantEvent, terminateEvent]);
216
responderID = wantsResponderID;
217
} else {
218
var rejectEvent = SyntheticEvent.getPooled(
@@ -221,10 +221,10 @@ function setResponderAndExtractTransfer(
221
nativeEvent
222
);
223
EventPropagators.accumulateDirectDispatches(rejectEvent);
224
- extracted = accumulate(extracted, rejectEvent);
224
+ extracted = accumulateInto(extracted, rejectEvent);
225
}
226
} else {
227
- extracted = accumulate(extracted, grantEvent);
227
+ extracted = accumulateInto(extracted, grantEvent);
228
responderID = wantsResponderID;
229
}
230
return extracted;
@@ -288,7 +288,7 @@ var ResponderEventPlugin = {
288
nativeEvent
289
);
290
if (transfer) {
291
- extracted = accumulate(extracted, transfer);
291
+ extracted = accumulateInto(extracted, transfer);
292
}
293
}
294
// Now that we know the responder is set correctly, we can dispatch
@@ -303,7 +303,7 @@ var ResponderEventPlugin = {
303
nativeEvent
304
);
305
EventPropagators.accumulateDirectDispatches(gesture);
306
- extracted = accumulate(extracted, gesture);
306
+ extracted = accumulateInto(extracted, gesture);
307
}
308
if (type === eventTypes.responderRelease) {
309
responderID = null;
src/browser/ui/dom/components/LocalEventTrapMixin.js
+3
-2
@@ -20,7 +20,7 @@
20
21
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
22
23
-var accumulate = require('accumulate');
23
+var accumulateInto = require('accumulateInto');
24
var forEachAccumulated = require('forEachAccumulated');
25
var invariant = require('invariant');
26
@@ -36,7 +36,8 @@ var LocalEventTrapMixin = {
36
handlerBaseName,
37
this.getDOMNode()
38
);
39
- this._localEventListeners = accumulate(this._localEventListeners, listener);
39
+ this._localEventListeners =
40
+ accumulateInto(this._localEventListeners, listener);
41
},
42
43
// trapCapturedEvent would look nearly identical. We don't implement that
src/event/EventPluginHub.js
+3
-3
@@ -21,7 +21,7 @@
21
var EventPluginRegistry = require('EventPluginRegistry');
22
var EventPluginUtils = require('EventPluginUtils');
23
24
-var accumulate = require('accumulate');
24
+var accumulateInto = require('accumulateInto');
25
var forEachAccumulated = require('forEachAccumulated');
26
var invariant = require('invariant');
27
var isEventSupported = require('isEventSupported');
@@ -236,7 +236,7 @@ var EventPluginHub = {
236
nativeEvent
237
);
238
if (extractedEvents) {
239
- events = accumulate(events, extractedEvents);
239
+ events = accumulateInto(events, extractedEvents);
240
}
241
}
242
}
@@ -252,7 +252,7 @@ var EventPluginHub = {
252
*/
253
enqueueEvents: function(events) {
254
if (events) {
255
- eventQueue = accumulate(eventQueue, events);
255
+ eventQueue = accumulateInto(eventQueue, events);
256
}
257
},
258
src/event/EventPropagators.js
+7
-5
@@ -21,7 +21,7 @@
21
var EventConstants = require('EventConstants');
22
var EventPluginHub = require('EventPluginHub');
23
24
-var accumulate = require('accumulate');
24
+var accumulateInto = require('accumulateInto');
25
var forEachAccumulated = require('forEachAccumulated');
26
27
var PropagationPhases = EventConstants.PropagationPhases;
@@ -52,8 +52,9 @@ function accumulateDirectionalDispatches(domID, upwards, event) {
52
var phase = upwards ? PropagationPhases.bubbled : PropagationPhases.captured;
53
var listener = listenerAtPhase(domID, event, phase);
54
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);
58
}
59
}
60
@@ -85,8 +86,9 @@ function accumulateDispatches(id, ignoredDirection, event) {
86
var registrationName = event.dispatchConfig.registrationName;
87
var listener = getListener(id, registrationName);
88
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);
92
}
93
}
94
}
src/utils/__tests__/accumulateInto-test.js
new
+64
@@ -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
new
+69
@@ -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;