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

Performance regression in pipe after v0.11.3 on OS X #3475

Closed
mhart opened this issue Oct 21, 2015 · 10 comments
Closed

Performance regression in pipe after v0.11.3 on OS X #3475

mhart opened this issue Oct 21, 2015 · 10 comments
Assignees
Labels
benchmark Issues and PRs related to the benchmark subsystem. macos Issues and PRs related to the macOS platform / OSX. stream Issues and PRs related to the stream subsystem.

Comments

@mhart
Copy link
Contributor

mhart commented Oct 21, 2015

(extracted from #3429 because that's specifically about the disparity between stdin and spawn)

This graph shows the performance of cat file | node readStdin.js on different node.js versions on OS X 10.11:

stdin

This was generated from 11 runs of the following program after cat/piping a 1GB file generated with head -c 1000000000 /dev/zero > file:

stdinPipe(function(err, time) {
  if (err) return console.error(err.stack || err)
  console.log([process.version, time].join(','))
})

function stdinPipe(cb) {
  var bytesRead = 0
  var time = process.hrtime()

  process.stdin.resume()
  process.stdin.on('data', function(data) {
    bytesRead += data.length
  })
  process.stdin.on('error', cb)
  process.stdin.on('end', function() {
    if (bytesRead != 1000000000) return cb(new Error('Incorrect bytes read: ' + bytesRead))
    var diff = process.hrtime(time)
    cb(null, 1000 / (diff[0] + diff[1] / 1e9))
  })
}

Now interestingly, as far as I can tell this seems to be a specifically pipe-related problem, because if the same program is run with node readStdin.js < file, no such difference exists:

stdinread

@bnoordhuis bnoordhuis added benchmark Issues and PRs related to the benchmark subsystem. macos Issues and PRs related to the macOS platform / OSX. labels Oct 21, 2015
@mhart mhart changed the title Performance regression in pipe from v0.11.3 to v0.11.4 on OS X Performance regression in pipe from v0.11.4 onwards on OS X Oct 21, 2015
@mhart mhart changed the title Performance regression in pipe from v0.11.4 onwards on OS X Performance regression in pipe after v0.11.4 on OS X Oct 21, 2015
@mhart mhart changed the title Performance regression in pipe after v0.11.4 on OS X Performance regression in pipe after v0.11.3 on OS X Oct 21, 2015
@silverwind
Copy link
Contributor

@silverwind silverwind added the stream Issues and PRs related to the stream subsystem. label Oct 22, 2015
@jasnell
Copy link
Member

jasnell commented Apr 2, 2016

is this still an issue in v4, v5 or master?

@mhart
Copy link
Contributor Author

mhart commented Apr 2, 2016

You can see v4.2.1 there in the graph – would there have been anything that would've fixed it?

Can you reproduce?

@MylesBorins
Copy link
Contributor

I'm going to investigate this

@GlenTiki GlenTiki assigned MylesBorins and unassigned GlenTiki Jul 12, 2016
@Fishrock123
Copy link
Contributor

Sounds like a duplicate of #3477 (comment) -- @addaleax would you mind checking?

@mhart
Copy link
Contributor Author

mhart commented Jul 19, 2016

@Fishrock123 just to be clear – these two issues were spun out because this one is Mac OS X specific.

@addaleax
Copy link
Member

Yeah, the symptoms here don’t match that either. This one also looks more fixable, I guess?

@Trott
Copy link
Member

Trott commented Jul 9, 2017

I don't have any reason to believe this was fixed in Node.js 6.x or 8.x, but does anyone want to run a comparison along with the versions already in the graph above so we know for sure?

@mhart
Copy link
Contributor Author

mhart commented Aug 17, 2017

This is definitely still an issue btw, the needle's hardly moved since v0.12. The performance of v8.4.0 is only 35% of what it is in v0.10.48.

chart 1

See #3429 for how this chart was generated

@mhart
Copy link
Contributor Author

mhart commented Aug 17, 2017

Actually, scratch that – pretty sure this is another case of #1671

If I add gc(true) calls every 100 data events, this is how the chart looks:

chart 3

So the gap between spawn and stdin is still there (which is what #3429 is about) – but there's no drop in performance between versions if GC is fixed (as I guess is proposed)

Closing as a duplicate of #1671

@mhart mhart closed this as completed Aug 17, 2017
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. macos Issues and PRs related to the macOS platform / OSX. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

9 participants