Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incremental Refactoring #261

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

thephoenixofthevoid
Copy link

I found out that many methods exist actually in 2 different implementations: for wildcard emitter and classic one, but for no reason got merged into single function. I am going to:

  1. Separate implementations into two sets of functions
  2. Make the module an ES6 module that supports tree-shaking
  3. Improve quality of the code

I am starting this PR to show progress in real time and also discuss these changes. I don't insist on merging, but if you find this useful, it would be great.

@thephoenixofthevoid
Copy link
Author

With given changes the performance of wildcard version doubled, but ordinary emitter's performance decreased. This is because I temporary removed many micro optimizations that made it hard to refactor the code. I plan to add them back at some point.

> eventemitter2-es@6.4.2 benchmark /home/feanor/Documents/ProgrammingResearch/Lvl60/EventEmitter2
> node test/perf/benchmark.js

Platform: linux, x64, 7878MB
Node version: v13.11.0
CPU: 1 x Intel(R) Core(TM) i5-8250U CPU @ 1.60GHz @ 3129MHz
 1 x Intel(R) Core(TM) i5-8250U CPU @ 1.60GHz @ 3342MHz
 1 x Intel(R) Core(TM) i5-8250U CPU @ 1.60GHz @ 3263MHz
 1 x Intel(R) Core(TM) i5-8250U CPU @ 1.60GHz @ 3040MHz
 1 x Intel(R) Core(TM) i5-8250U CPU @ 1.60GHz @ 3168MHz
 1 x Intel(R) Core(TM) i5-8250U CPU @ 1.60GHz @ 3294MHz
 1 x Intel(R) Core(TM) i5-8250U CPU @ 1.60GHz @ 3326MHz
 1 x Intel(R) Core(TM) i5-8250U CPU @ 1.60GHz @ 3198MHz
----------------------------------------------------------------
EventEmitterHeatUp x 8,168,953 ops/sec ±0.35% (92 runs sampled)
EventEmitter x 7,667,663 ops/sec ±0.55% (93 runs sampled)
EventEmitter2 x 21,699,789 ops/sec ±0.41% (95 runs sampled)
EventEmitter2 (wild) x 539,570 ops/sec ±0.28% (98 runs sampled)
EventEmitter2 (wild) using plain events x 1,048,870 ops/sec ±0.21% (95 runs sampled)
EventEmitter2 (wild) emitting ns x 3,271,452 ops/sec ±0.38% (92 runs sampled)
EventEmitter2 (wild) emitting a plain event x 5,379,542 ops/sec ±0.20% (97 runs sampled)
EventEmitter2Es x 11,466,351 ops/sec ±0.46% (93 runs sampled)
EventEmitter2Es (wild) x 934,198 ops/sec ±0.35% (97 runs sampled)
EventEmitter2Es (wild) using plain events x 1,945,518 ops/sec ±0.38% (94 runs sampled)
EventEmitter2Es (wild) emitting ns x 2,935,073 ops/sec ±0.47% (94 runs sampled)
EventEmitter2Es (wild) emitting a plain event x 4,538,065 ops/sec ±0.25% (97 runs sampled)
EventEmitter3 x 19,735,018 ops/sec ±1.09% (91 runs sampled)

Fastest is EventEmitter2

@SukkaW
Copy link

SukkaW commented Mar 16, 2021

Will the PR being continued?

@thephoenixofthevoid
Copy link
Author

Yeah, I am interested in continuing, but It's important to discuss changes. The project is much depended upon, changes I have proposed so far are massive. Long story short: need feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants