Merge pull request #4905 from lukecotter/perf/event-bus

perf(events): avoid arguments/forEach allocation in event buses

Author: not-night-but

src/js/core/tools/ExternalEventBus.js

@@ -72,34 +72,26 @@ export default class ExternalEventBus {
 		}
 	}
 
-	_dispatch(){
-		var args = Array.from(arguments),
-		key = args.shift(),
-		result;
-
-		if(this.events[key]){
-			this.events[key].forEach((callback, i) => {
-				let callResult = callback.apply(this.table, args);
-
-				if(!i){
-					result = callResult;
-				}
-			});
+	_dispatch(key, ...args){
+		const subs = this.events[key];
+		const len = subs?.length;
+		if(len){
+			const result = subs[0].apply(this.table, args);
+			
+			for(let i = 1; i < len; i++){
+				subs[i].apply(this.table, args);
+			}
+			
+			return result;
 		}
-
-		return result;
 	}
 
-	_debugDispatch(){
-		var args = Array.from(arguments),
-		key = args[0];
-
-		args[0] = "ExternalEvent:" + args[0];
-
+	_debugDispatch(key, ...args){
 		if(this.debug === true || this.debug.includes(key)){
-			console.log(...args);
+			const debugArgs = ["ExternalEvent:" + key, ...args];
+			console.log(...debugArgs);
 		}
 
-		return this._dispatch(...arguments);
+		return this._dispatch(key, ...args);
 	}
-}
+}

src/js/core/tools/InternalEventBus.js

@@ -65,21 +65,28 @@ export default class InternalEventBus {
 	}
 
 	_chain(key, args, initialValue, fallback){
-		var value = initialValue;
+		const subs = this.events[key];
 
-		if(!Array.isArray(args)){
-			args = [args];
-		}
+		if(subs && subs.length){
+			if(!Array.isArray(args)){
+				args = [args];
+			}
 
-		if(this.subscribed(key)){
-			this.events[key].forEach((subscriber, i) => {
-				value = subscriber.callback.apply(this, args.concat([value]));
-			});
+			//reuse a single args array across subscribers, mutating only the trailing
+			//accumulator slot, instead of allocating a fresh concat per subscriber
+			const callArgs = args.slice();
+			const valueIndex = callArgs.length;
+			let value = initialValue;
+
+			for(let i = 0; i < subs.length; i++){
+				callArgs[valueIndex] = value;
+				value = subs[i].callback.apply(this, callArgs);
+			}
 
 			return value;
-		}else{
-			return typeof fallback === "function" ? fallback() : fallback;
 		}
+
+		return typeof fallback === "function" ? fallback() : fallback;
 	}
 
 	_confirm(key, args){
@@ -110,53 +117,39 @@ export default class InternalEventBus {
 		}
 	}
 
-	_dispatch(){
-		var args = Array.from(arguments),
-		key = args.shift();
-
-		if(this.events[key]){
-			this.events[key].forEach((subscriber) => {
+	_dispatch(key, ...args){
+		const subs = this.events[key];
+		if(subs){
+			for(const subscriber of subs){
 				subscriber.callback.apply(this, args);
-			});
+			}
 		}
 	}
 
-	_debugDispatch(){
-		var args = Array.from(arguments),
-		key = args[0];
-
-		args[0] = "InternalEvent:" + key;
-
+	_debugDispatch(key, ...args){
 		if(this.debug === true || this.debug.includes(key)){
-			console.log(...args);
+			const debugArgs = ["InternalEvent:" + key, ...args];
+			console.log(...debugArgs);
 		}
 
-		return this._dispatch(...arguments);
+		return this._dispatch(key, ...args);
 	}
 
-	_debugChain(){
-		var args = Array.from(arguments),
-		key = args[0];
-
-		args[0] = "InternalEvent:" + key;
-
+	_debugChain(key, args, initialValue, fallback){
 		if(this.debug === true || this.debug.includes(key)){
-			console.log(...args);
+			const debugArgs = ["InternalEvent:" + key, args, initialValue, fallback];
+			console.log(...debugArgs);
 		}
 
-		return this._chain(...arguments);
+		return this._chain(key, args, initialValue, fallback);
 	}
 
-	_debugConfirm(){
-		var args = Array.from(arguments),
-		key = args[0];
-
-		args[0] = "InternalEvent:" + key;
-
+	_debugConfirm(key, args){
 		if(this.debug === true || this.debug.includes(key)){
-			console.log(...args);
+			const debugArgs = ["InternalEvent:" + key, args];
+			console.log(...debugArgs);
 		}
 
-		return this._confirm(...arguments);
+		return this._confirm(key, args);
 	}
-}
+}

test/unit/core/tools/InternalEventBus.spec.js

@@ -0,0 +1,51 @@
+import InternalEventBus from "../../../../src/js/core/tools/InternalEventBus";
+
+// Locks the _chain semantics the reuse-args fast path must preserve.
+
+describe("InternalEventBus._chain", () => {
+	test("threads the accumulator through subscribers in order", () => {
+		const bus = new InternalEventBus(false);
+		bus.subscribe("k", (_arg, value) => value + 1);
+		bus.subscribe("k", (_arg, value) => value * 10);
+		// priority equal -> insertion order: (0+1)=1 then 1*10=10
+		expect(bus.chain("k", ["x"], 0, false)).toBe(10);
+	});
+
+	test("passes the leading args before the accumulator value", () => {
+		const bus = new InternalEventBus(false);
+		bus.subscribe("k", (a, b, value) => `${a}|${b}|${value}`);
+		expect(bus.chain("k", ["p", "q"], "init", false)).toBe("p|q|init");
+	});
+
+	test("non-array args is wrapped", () => {
+		const bus = new InternalEventBus(false);
+		bus.subscribe("k", (a, value) => `${a}:${value}`);
+		expect(bus.chain("k", "solo", "v", false)).toBe("solo:v");
+	});
+
+	test("an array return value is forwarded as a SINGLE argument", () => {
+		const bus = new InternalEventBus(false);
+		bus.subscribe("k", () => [1, 2, 3]);
+		bus.subscribe("k", (_arg, value) => (Array.isArray(value) ? value.length : "spread"));
+		// must be 3 (received [1,2,3] as one arg), not "spread"/NaN from a spread array
+		expect(bus.chain("k", ["x"], 0, false)).toBe(3);
+	});
+
+	test("no subscribers returns the fallback value", () => {
+		const bus = new InternalEventBus(false);
+		expect(bus.chain("missing", ["x"], "init", "fallbackValue")).toBe("fallbackValue");
+	});
+
+	test("no subscribers invokes a function fallback", () => {
+		const bus = new InternalEventBus(false);
+		expect(bus.chain("missing", ["x"], "init", () => "fromFn")).toBe("fromFn");
+	});
+
+	test("a key whose subscribers were all removed falls back (not the initial value)", () => {
+		const bus = new InternalEventBus(false);
+		const cb = (_arg, value) => value;
+		bus.subscribe("k", cb);
+		bus.unsubscribe("k", cb);
+		expect(bus.chain("k", ["x"], "init", "fallbackValue")).toBe("fallbackValue");
+	});
+});