Merge pull request #85 from WordPress/update/run-hooks-perf

Hooks: Micro-optimize runHooks
WordPress · Feb 27, 2018 · d0c807d · d0c807d
2 parents e116f2a + 8d5854b
commit d0c807d
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 41 deletions.
diff --git a/package.json b/package.json
@@ -6,6 +6,7 @@
 	},
 	"devDependencies": {
 		"babel-core": "^6.26.0",
+		"benchmark": "^2.1.4",
 		"chalk": "^2.0.1",
 		"check-node-version": "^3.1.1",
 		"codecov": "^2.2.0",

diff --git a/packages/hooks/benchmark/index.js b/packages/hooks/benchmark/index.js
@@ -0,0 +1,18 @@
+const Benchmark = require( 'benchmark' );
+const hooks = require( '../' );
+
+const suite = new Benchmark.Suite;
+
+function myCallback() {}
+
+hooks.addFilter( 'handled', 'myCallback', myCallback );
+
+suite
+	.add( 'handled', () => {
+		hooks.applyFilters( 'handled' );
+	} )
+	.add( 'unhandled', () => {
+		hooks.applyFilters( 'unhandled' );
+	} )
+	.on( 'cycle', ( event ) => console.log( event.target.toString() ) )
+	.run( { async: true } );
diff --git a/packages/hooks/src/createAddHook.js b/packages/hooks/src/createAddHook.js
@@ -41,7 +41,7 @@ function createAddHook( hooks ) {
 
 		const handler = { callback, priority, namespace };
 
-		if ( hooks.hasOwnProperty( hookName ) ) {
+		if ( hooks[ hookName ] ) {
 			// Find the correct insert index of the new hook.
 			const handlers = hooks[ hookName ].handlers;
 			let i = 0;

diff --git a/packages/hooks/src/createDidHook.js b/packages/hooks/src/createDidHook.js
@@ -22,7 +22,7 @@ function createDidHook( hooks ) {
 			return;
 		}
 
-		return hooks.hasOwnProperty( hookName ) && hooks[ hookName ].runs
+		return hooks[ hookName ] && hooks[ hookName ].runs
 			? hooks[ hookName ].runs
 			: 0;
 	};

diff --git a/packages/hooks/src/createHasHook.js b/packages/hooks/src/createHasHook.js
@@ -17,7 +17,7 @@ function createHasHook( hooks ) {
 	 *                            the given hook.
 	 */
 	return function hasHook( hookName ) {
-		return hooks.hasOwnProperty( hookName )
+		return hooks[ hookName ]
 			? hooks[ hookName ].handlers.length
 			: 0;
 	};

diff --git a/packages/hooks/src/createHooks.js b/packages/hooks/src/createHooks.js
@@ -12,8 +12,10 @@ import createDidHook from './createDidHook';
  * @return {Object} Object that contains all hooks.
  */
 function createHooks() {
-	const actions = {};
-	const filters = {};
+	const actions = Object.create( null );
+	const filters = Object.create( null );
+	actions.__current = [];
+	filters.__current = [];
 
 	return {
 		addAction:        createAddHook( actions ),

diff --git a/packages/hooks/src/createRemoveHook.js b/packages/hooks/src/createRemoveHook.js
@@ -32,7 +32,7 @@ function createRemoveHook( hooks, removeAll ) {
 		}
 
 		// Bail if no hooks exist by this name
-		if ( ! hooks.hasOwnProperty( hookName ) ) {
+		if ( ! hooks[ hookName ] ) {
 			return 0;
 		}
 

diff --git a/packages/hooks/src/createRunHook.js b/packages/hooks/src/createRunHook.js
@@ -21,21 +21,12 @@ function createRunHook( hooks, returnFirstArg ) {
 	 * @return {*}               Return value of runner, if applicable.
 	 */
 	return function runHooks( hookName, ...args ) {
-
-		if ( ! validateHookName( hookName ) ) {
-			return;
-		}
-
-		if ( ! hooks.hasOwnProperty( hookName ) ) {
-			hooks[ hookName ] = {
-				runs: 0,
-				handlers: [],
-			};
+		let handlers;
+		if ( hooks[ hookName ] ) {
+			handlers = hooks[ hookName ].handlers;
 		}
 
-		const handlers = hooks[ hookName ].handlers;
-
-		if ( ! handlers.length ) {
+		if ( ! handlers || ! handlers.length ) {
 			return returnFirstArg
 				? args[ 0 ]
 				: undefined;
@@ -46,25 +37,26 @@ function createRunHook( hooks, returnFirstArg ) {
 			currentIndex: 0,
 		};
 
-		hooks.__current = hooks.__current || [];
 		hooks.__current.push( hookInfo );
-		hooks[ hookName ].runs++;
 
-		let maybeReturnValue = args[ 0 ];
+		if ( ! hooks[ hookName ] ) {
+			hooks[ hookName ] = {
+				runs: 0,
+				handlers: [],
+			};
+		}
+		hooks[ hookName ].runs++;
 
 		while ( hookInfo.currentIndex < handlers.length ) {
 			const handler = handlers[ hookInfo.currentIndex ];
-			maybeReturnValue = handler.callback.apply( null, args );
-			if ( returnFirstArg ) {
-				args[ 0 ] = maybeReturnValue;
-			}
+			args[ 0 ] = handler.callback.apply( null, args );
 			hookInfo.currentIndex++;
 		}
 
 		hooks.__current.pop();
 
 		if ( returnFirstArg ) {
-			return maybeReturnValue;
+			return args[ 0 ];
 		}
 	};
 }

diff --git a/packages/hooks/src/test/index.test.js b/packages/hooks/src/test/index.test.js
@@ -71,6 +71,10 @@ beforeEach( () => {
 	// because the internal functions have references to the original objects.
 	[ actions, filters ].forEach( hooks => {
 		for ( const k in hooks ) {
+			if ( '__current' === k ) {
+				continue;
+			}
+
 			delete hooks[ k ];
 		}
 	} );
@@ -214,20 +218,6 @@ test( 'cannot add filters with non-numeric priorities', () => {
 	);
 } );
 
-test( 'cannot run filters with non-string names', () => {
-	expect( applyFilters( () => {}, 42 ) ).toBe( undefined );
-	expect( console ).toHaveErroredWith(
-		'The hook name must be a non-empty string.'
-	);
-} );
-
-test( 'cannot run filters named with __ prefix', () => {
-	expect( applyFilters( '__test', 42 ) ).toBe( undefined );
-	expect( console ).toHaveErroredWith(
-		'The hook name cannot begin with `__`.'
-	);
-} );
-
 test( 'add 3 filters with different priorities and run them', () => {
 	addFilter( 'test.filter', 'my_callback_filter_a',  filter_a );
 	addFilter( 'test.filter', 'my_callback_filter_b',  filter_b, 2 );