DRY up tappable code for applyPlugins.

[samccone]

migrate the logic to live in the root function and use a switch
statement to decide when to use call / apply.

Keep the existing functions in place and call though to maintain
backwards compat.

original ref bug:
#33

---

If you are OK with this change we can delete most of the code in this repo for all of the other functions.

[bmeurer]

Without knowing a lot about this code and how it's being used, I'd suspect that this this._plugins[name] is not going to be monomorphic (i.e. name will be different and probably this._plugins too). So repeating this load is going to be the expensive part.

[TheLarkInn]

@bmeurer here is just a few examples of apply plugins being used in two separate classes that extend Tapable.

https://github.com/webpack/webpack/blob/990563f9a2b3fb3e817b7e1b9d6841bcd1eaa51e/lib/Compilation.js#L48-L676

https://github.com/webpack/webpack/blob/990563f9a2b3fb3e817b7e1b9d6841bcd1eaa51e/lib/Parser.js#L1045-L1141

[bmeurer]

Ok, based on this benchmark, I'd strongly suggest to go for F.p.apply and rest parameters instead, as indicated by the results for Chrome Canary:

[thymikee]

So I was curious about Chrome Stable (V8 6.0.286.52, same as Node 8.3, and this code is running on Node) and the results are bit different:

[bmeurer]

Jep, just checked Chrome 61 (which is what Node 8 will eventually be based on in the end), and same picture there:

[bmeurer]

FYI: Ran this gist with latest Node 8.3.0 locally and I do indeed see results similar to what I observed with Canary:

[samccone]

PR updated with optimized version

[sokra]

I see to performance benefit for the method with variable number of arguments.

But I don't see the benefit for the methods with fixed number of arguments (i. e. applyPlugins2). They are manually optimized for a specific number of arguments. Most "hot" hooks use them.

Anyway, I would love to take a more radical approach. This whole package is a bit old. What about a new major version with more performance and more features.

• Never use arguments, compile fixed arguments function.
• Create a function per hooks which makes it monomorphic.
• Maybe something magic like new Function(`function x(${args}) ...`)
• Different apply function depending on number of registered plugins
  • no plugins: noop apply for max performance
  • special version to 1 plugin: optimized
  • general version for more plugin
• You need to register hooks before using them. You need to specify number of arguments and kind (sync/async/parallel/waterfall).
• Instead of compiler.plugin("before-run", ...) compiler.hooks.beforeRun(...) to allow have typings for these hooks.
• Each plugin can have a name compiler.hooks.beforeRun("HotModuleReplacementPlugin", () => ...)
• Insert plugins at specify locations instead of always to the end. hooks.beforeRun({ name: ..., before: "HotModuleReplacementPlugin" }, () => ...)
• Maybe a middleware api, if this doesn't affect performance: hooks.beforeRun.intercept(...)
  • Would be great of the progress display and profiling.
• The plugin(...) method can be provided for backward compatibility for plugins.
  • We may generate plugin name from stack trace (depending on performance cost for this)
• The old applyPluginsXXX methods can also be offered for backward compatibility, but trigger a deprecation.
• Using a unknown hook name in the plugin or applyPluginsXXX automatically register a compat hook which handle them.
• The compatibility methods convert dash-case to camelCase.
• This would be a big breaking change, but I guess acceptable with the compatibility layer.

[sokra]

I created a new issue for this: #35

[bmeurer]

Do you have a benchmark for this? I.e. what are the likely cases?

[samccone]

@TheLarkInn PTAL when you have a moment. thank you!

[sokra]

new major version uses a better system to avoid using arguments and generate a function per hook

[samccone]

Thanks!