Skip to content

Conversation

@mscdex
Copy link
Contributor

@mscdex mscdex commented Feb 21, 2015

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

is this a very fair benchmark? after all, wouldn't any run after the first run be only to the .on() listeners?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you suggest?

Copy link
Contributor

Choose a reason for hiding this comment

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

just throwing ideas at the wall, but i'm thinking some sort of Object.freeze() (maybe deep-freeze) on the event emitter, which would (in non-strict mode) i believe silently fail when it is changed, leaving us the same object in each loop - thats the only idea that comes to mind that would only benchmark the .emit(), what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

How do const objects behave, would that do the job?

@mscdex mscdex added the benchmark Issues and PRs related to the benchmark subsystem. label Mar 25, 2015
@brendanashworth
Copy link
Contributor

Right now, regarding these new benchmarks, I don't think most of them benchmark much. I don't really have any idea on how to fix that, but I don't think it's fair as-is.

@mscdex
Copy link
Contributor Author

mscdex commented Apr 8, 2015

Well, at this point the benchmarks don't really matter unless #914 is merge-able. However as it stands, we need to know what kind of effect it will have on modules depending on readable-stream (i.e. who is depending on a fixed readable-stream version).

@brendanashworth
Copy link
Contributor

@mscdex perhaps this should just be closed and reopened if it seems fit?

@Qard
Copy link
Member

Qard commented Aug 8, 2015

Let's get @nodejs/benchmarking on this.

@jasnell jasnell added the stalled Issues and PRs that are stalled. label Nov 16, 2015
@mscdex mscdex closed this Feb 7, 2016
@mscdex mscdex deleted the more-ee-benchmarks-part2 branch February 7, 2016 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmark Issues and PRs related to the benchmark subsystem. stalled Issues and PRs that are stalled.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants