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

Rejection of a Promise that depends on another promise is extremely slow in V8/Chrome. #501

Closed
dtinth opened this Issue Feb 18, 2015 · 9 comments

Comments

Projects
None yet
4 participants
@dtinth

dtinth commented Feb 18, 2015

The Story

First, I created a Promise, called x.

var x = new Promise(function(resolve) {
  setTimeout(resolve, 1000)
})

Then I created a function that processes a task, but it depends on x, but they all fail:

function y(task) {
  return x.then(function() { throw new Error('it failed') })
}

I have 1000 tasks, and I process them all.

Promise.all(tasks.map(y)).then(...).catch(...)

The Problem

At the moment x is resolved, on Google Chrome with a minified build, the browser freezes for ~10 seconds.

The Test Case

https://rawgit.com/dtinth/39520b129f143c66bd40/raw/index.html

The Weirdness

  • From my testing, this only happens in Google Chrome, including Chrome Canary.
    • It runs smoothly on Firefox and Safari.
  • When using native promises, this problem doesn't happen.
  • It only happens in minified builds.
    • When I use bluebird.js instead of bluebird.min.js, the problem disappears.
    • Nevertheless, when I use bluebird in webpack environment, the problem still appears.

The Profile

On my MacBook, it takes 9 seconds to settle the Promise.

screen shot 2015-02-18 at 21 02 47

Upon zooming, most time is spent on _rejectCallback.

screen shot 2015-02-18 at 21 04 13

The Variations

If y returns a promise that's eventually fulfilled (instead of rejected), this problem doesn't happen.

If each y depends on different x, this problem doesn't happen. What I mean is that x and y looks like this instead:

function x() {
  return new Promise(function(resolve) {
    setTimeout(resolve, 1000)
  })
}

function y(task) {
  return x().then(function() { throw new Error('it failed') })
}
@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr Feb 18, 2015

Collaborator

Can you reproduce this in node/io
?

Collaborator

benjamingr commented Feb 18, 2015

Can you reproduce this in node/io
?

@dtinth

This comment has been minimized.

Show comment
Hide comment
@dtinth

dtinth Feb 18, 2015

Yes, this is reproducable in Node.js (v0.10.32) using the following test code:

var Promise = require('bluebird')

var x = new Promise(function(resolve) {
  setTimeout(function() {
    console.log('will now resolve!')
    resolve()
  }, 1000)
})

function y(task) {
  return x.then(function() { throw new Error('it failed') })
}

var tasks = []
for (var i = 0; i < 1000; i ++) tasks.push('Task ' + i)

Promise.all(tasks.map(y))
.then(function() { console.log('done') })
.catch(function() { console.log('caught') })

After will now resolve! is printed, it takes ~8 seconds until the word caught is displayed.

If the function y is modified to return a fulfilling promise, the 1000 tasks are resolved instantly.

dtinth commented Feb 18, 2015

Yes, this is reproducable in Node.js (v0.10.32) using the following test code:

var Promise = require('bluebird')

var x = new Promise(function(resolve) {
  setTimeout(function() {
    console.log('will now resolve!')
    resolve()
  }, 1000)
})

function y(task) {
  return x.then(function() { throw new Error('it failed') })
}

var tasks = []
for (var i = 0; i < 1000; i ++) tasks.push('Task ' + i)

Promise.all(tasks.map(y))
.then(function() { console.log('done') })
.catch(function() { console.log('caught') })

After will now resolve! is printed, it takes ~8 seconds until the word caught is displayed.

If the function y is modified to return a fulfilling promise, the 1000 tasks are resolved instantly.

@phpnode

This comment has been minimized.

Show comment
Hide comment
@phpnode

phpnode Feb 18, 2015

@dtinth that particular test case did not reproduce the bug for me on iojs / node 0.11.x / node 0.10.x

phpnode commented Feb 18, 2015

@dtinth that particular test case did not reproduce the bug for me on iojs / node 0.11.x / node 0.10.x

@dtinth

This comment has been minimized.

Show comment
Hide comment
@dtinth

dtinth Feb 18, 2015

@phpnode

It also happens on iojs 1.0.3. I reproduced this on Mac OS X 10.10.1 with Bluebird 2.9.9.

This is also reproducible on Ubuntu 13.10 running Node v0.10.28 and Bluebird 2.9.9.

dtinth commented Feb 18, 2015

@phpnode

It also happens on iojs 1.0.3. I reproduced this on Mac OS X 10.10.1 with Bluebird 2.9.9.

This is also reproducible on Ubuntu 13.10 running Node v0.10.28 and Bluebird 2.9.9.

@dtinth dtinth changed the title from Rejection of a Promise that depends on another promise is extremely slow in Chrome. to Rejection of a Promise that depends on another promise is extremely slow in V8/Chrome. Feb 18, 2015

@phpnode

This comment has been minimized.

Show comment
Hide comment
@phpnode

phpnode Feb 18, 2015

@dtinth ah, sorry, you're right, I was testing on 2.4.x, was able to reproduce with 2.9.9

phpnode commented Feb 18, 2015

@dtinth ah, sorry, you're right, I was testing on 2.4.x, was able to reproduce with 2.9.9

@dtinth

This comment has been minimized.

Show comment
Hide comment
@dtinth

dtinth Feb 18, 2015

By trying to install various versions, this problem started appearing since 2.8.2.

▹ npm install bluebird@2.8.1
bluebird@2.8.1 node_modules/bluebird

▹ time node test.js
will now resolve!
caught
node test.js  0.12s user 0.03s system 12% cpu 1.143 total

▹ npm install bluebird@2.8.2
bluebird@2.8.2 node_modules/bluebird

▹ time node test.js
will now resolve!
caught
node test.js  8.98s user 0.15s system 89% cpu 10.224 total

dtinth commented Feb 18, 2015

By trying to install various versions, this problem started appearing since 2.8.2.

▹ npm install bluebird@2.8.1
bluebird@2.8.1 node_modules/bluebird

▹ time node test.js
will now resolve!
caught
node test.js  0.12s user 0.03s system 12% cpu 1.143 total

▹ npm install bluebird@2.8.2
bluebird@2.8.2 node_modules/bluebird

▹ time node test.js
will now resolve!
caught
node test.js  8.98s user 0.15s system 89% cpu 10.224 total

dtinth added a commit to bemusic/bemuse that referenced this issue Feb 18, 2015

@petkaantonov

This comment has been minimized.

Show comment
Hide comment
@petkaantonov

petkaantonov Feb 18, 2015

Owner

@dtinth I will push a 2.9.10 release as soon as the browser tests come back out, no need to revert to 2.8.1 :)

Owner

petkaantonov commented Feb 18, 2015

@dtinth I will push a 2.9.10 release as soon as the browser tests come back out, no need to revert to 2.8.1 :)

@petkaantonov

This comment has been minimized.

Show comment
Hide comment
@petkaantonov

petkaantonov Feb 18, 2015

Owner

@dtinth Fixed in 2.9.10

Owner

petkaantonov commented Feb 18, 2015

@dtinth Fixed in 2.9.10

dtinth added a commit to bemusic/bemuse that referenced this issue Feb 19, 2015

chore(package): use latest Bluebird (2.9.10)
Because petkaantonov/bluebird#501 has been fixed very quickly. Kudos!
@dtinth

This comment has been minimized.

Show comment
Hide comment
@dtinth

dtinth Feb 19, 2015

Thank you for fixing this very quickly. :)

dtinth commented Feb 19, 2015

Thank you for fixing this very quickly. :)

dtinth added a commit to bemusic/bemuse that referenced this issue Feb 19, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment