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

Net benchmarks fail to build #6405

Closed
matthewloring opened this issue Apr 27, 2016 · 6 comments
Closed

Net benchmarks fail to build #6405

matthewloring opened this issue Apr 27, 2016 · 6 comments
Assignees
Labels
benchmark Issues and PRs related to the benchmark subsystem. events Issues and PRs related to the events subsystem / EventEmitter.

Comments

@matthewloring
Copy link

  • Version: v7.0.0-pre (@master)
  • Platform: OS X 10.11.4
  • Subsystem: Benchmark

Running make bench results in the output:

net/net-c2s-cork.js
_stream_readable.js:19
    return emitter.prependListener(event, fn);
                   ^

TypeError: emitter.prependListener is not a function
    at prependListener (_stream_readable.js:19:20)
    at Socket.Readable.pipe (_stream_readable.js:582:3)
    at Server.<anonymous> (<node_dir>/benchmark/net/net-c2s-cork.js:71:12)
    at emitOne (events.js:96:13)
    at Server.emit (events.js:188:7)
    at TCP.onconnection (net.js:1452:8)
child process exited with code 1
@matthewloring matthewloring added confirmed-bug Issues with confirmed bugs. benchmark Issues and PRs related to the benchmark subsystem. labels Apr 27, 2016
@matthewloring matthewloring self-assigned this Apr 27, 2016
@matthewloring matthewloring added events Issues and PRs related to the events subsystem / EventEmitter. and removed confirmed-bug Issues with confirmed bugs. labels Apr 27, 2016
@jasnell
Copy link
Member

jasnell commented Apr 27, 2016

Ugh... ok, yeah, it looks like those benchmarks are doing some funny things with the event emitter interface. Good catch, will have to spend some time to figure that one out.

@matthewloring
Copy link
Author

@jasnell Putting together a potential fix now. I'm looking for more input but I think prependEmitter can be mocked out just like emit, on, etc for these benchmarks.

@Trott
Copy link
Member

Trott commented Apr 27, 2016

/cc @mcollina

@mcollina
Copy link
Member

The structure of these benchmark was already there. They probably need an overhaul anyway.
BTW, we should make sure that the benchmarks are part of the CI thing, not to take actual measurement, but on the fact that they "work".

@jasnell
Copy link
Member

jasnell commented Apr 27, 2016

Yeah I was thinking the same thing. The challenge is that some of the
benchmarks are fairly intensive. However, during release prep, there should
be an option to include benchmarks in a CI run. /cc @nodejs/build
On Apr 27, 2016 2:07 AM, "Matteo Collina" notifications@github.com wrote:

The structure of these benchmark was already there. They probably need an
overhaul anyway.
BTW, we should make sure that the benchmarks are part of the CI thing, not
to take actual measurement, but on the fact that they "work".


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#6405 (comment)

@jasnell
Copy link
Member

jasnell commented Apr 30, 2016

Fix landed in #6407

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. events Issues and PRs related to the events subsystem / EventEmitter.
Projects
None yet
Development

No branches or pull requests

4 participants