Skip to content

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Feb 5, 2015

No description provided.

@3rd-Eden
Copy link
Contributor

3rd-Eden commented Feb 5, 2015

You might also want to look at https://github.com/primus/eventemitter3/tree/master/benchmarks/run for additional benchmarks.


bench.start();
for (var i = 0; i < n; i += 1) {
ee.emit('dummy', 5, true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any chance we could do a varying range of argument length (i.e. 1-4) for this one? I'd say we could use .apply() since the benchmarks are only compared against eachother, really

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't care one way or the other. I just picked some number of arguments. I'm not sure if it'd be best to have a separate benchmark for one with tons of arguments or to lump them all into one....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just thinking on the basis of your PR comment, and that it would be best to express that in a benchmark -- of course, this comment is pretty much only based on that PR, so thats up to you

bnoordhuis pushed a commit that referenced this pull request Feb 6, 2015
PR-URL: #730
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brendan Ashworth <brendan.ashworth@me.com>
@bnoordhuis
Copy link
Member

Thanks Brian, landed in 847b9d2 with Brendan as co-reviewer.

@bnoordhuis bnoordhuis closed this Feb 6, 2015
@mscdex mscdex deleted the more-ee-benchmarks branch February 7, 2016 19:05
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants