javascript - Slowdown due to non-parallel awaiting of promises in async generators - Stack Overflow

admin管理员组

文章数量:1178528

I'm writing code using generators and Bluebird and I have the following:

var async = Promise.coroutine;
function Client(request){
    this.request = request;
}


Client.prototype.fetchCommentData = async(function* (user){
    var country = yield countryService.countryFor(user.ip);
    var data = yield api.getCommentDataFor(user.id);
    var notBanned = yield authServer.authenticate(user.id);
    if (!notBanned) throw new AuthenticationError(user.id);
    return {
        country: country,
        comments: data,
        notBanned: true
    };
});

However, this is kind of slow, I feel like my application is waiting too much for I/O and it's not in parallel. How can I improve the performance of my application?

The total response time is 800 for countryFor + 400 for getCommentDataFor + 600 for authenticate so in total 1800ms which is a lot.

I'm writing code using generators and Bluebird and I have the following:

var async = Promise.coroutine;
function Client(request){
    this.request = request;
}


Client.prototype.fetchCommentData = async(function* (user){
    var country = yield countryService.countryFor(user.ip);
    var data = yield api.getCommentDataFor(user.id);
    var notBanned = yield authServer.authenticate(user.id);
    if (!notBanned) throw new AuthenticationError(user.id);
    return {
        country: country,
        comments: data,
        notBanned: true
    };
});

However, this is kind of slow, I feel like my application is waiting too much for I/O and it's not in parallel. How can I improve the performance of my application?

The total response time is 800 for countryFor + 400 for getCommentDataFor + 600 for authenticate so in total 1800ms which is a lot.

• javascript
• async-await
• promise
• generator
• bluebird

Share Improve this question Follow edited Jul 10, 2015 at 15:36 Martijn Pieters 1.1m320320 gold badges4.2k4.2k silver badges3.4k3.4k bronze badges asked Jun 12, 2014 at 20:44 Benjamin GruenbaumBenjamin Gruenbaum 276k8989 gold badges518518 silver badges514514 bronze badges 5

• Can we come up with a better title, something along the lines of "Run promises in parallel in async generators"? – Bergi Commented Mar 6, 2015 at 3:02
• @Bergi by all means - go for it. – Benjamin Gruenbaum Commented Mar 6, 2015 at 12:30
• It's just that I don't like the phrase "run promises", and I'd also like to incorporate the performance thing. Can you think of some better? – Bergi Commented Mar 6, 2015 at 13:34
• Yeah, promises are not "run" by any measure but the longer I've been teaching people code and answering things on StackOverflow the less I care about exact terminology in favor of usefulness. The goal here was to let people know that generators can be slow in these scenarios and to let people know of a common performance bug, anything that better reaches people or accomplishes that goal is positive IMO @Bergi – Benjamin Gruenbaum Commented Mar 6, 2015 at 14:17
• !notBanned means the user is banned? B/c you then return notBanned: true which would be the opposite, no? – Penguin9 Commented Aug 23, 2018 at 12:09

Add a comment  | 

3 Answers 3

Sorted by: Reset to default 20

You are spending too much time waiting for I/O from different sources.

In normal promise code, you'd use Promise.all for this, however - people have a tendency to write code that waits for requests with generators. Your code does the following:

<-client     service->
countryFor..
           ''--..
              ''--..
                 ''--.. country server sends response
               ..--''
          ..--''
     ..--''
getCommentDataFor
     ''--..
           ''--..
               ''--..
                     ''--.. comment service returns response
                ..--''
          ..--''
      ..--''
authenticate
       ''--..
            ''--..
                  ''--.. authentication service returns
             ..--''
       ..--''
 ..--''
 Generator done.

Instead, it should be doing:

<-client     service->
countryFor..
commentsFor..''--..
authenticate..''--..''--..
                 ''--..''--..''--.. country server sends response
                        ''--..--''..  comment service returns response
                   ..--''..--''..     authentication service returns response
          ..--''..--''..
 ..--''..--''..--''
 ..--''..--''
 ..--''
 Generator done

Simply put, all your I/O should be done in parallel here.

To fix this, I'd use Promise.props. Promise.props takes an objects and waits for all its properties to resolve (if they are promises).

Remember - generators and promises mix and match really well, you simply yield promises:

Client.prototype.fetchCommentData = async(function* (user){
    var country = countryService.countryFor(user.ip);
    var data = api.getCommentDataFor(user.id);
    var notBanned = authServer.authenticate(user.id).then(function(val){
          if(!val) throw new AuthenticationError(user.id);
    });
    return Promise.props({ // wait for all promises to resolve
        country : country,
        comments : data,
        notBanned: notBanned
    });
});

This is a very common mistake people make when using generators for the first time.

^{ascii art shamelessly taken from Q-Connection by Kris Kowal}

As it is mentioned in the Bluebird docs for Promise.coroutine, you need to watch out not to yield in a series.

var county = yield countryService.countryFor(user.ip);
var data = yield api.getCommentDataFor(user.id);
var notBanned = yield authServer.authenticate(user.id);

This code has 3 yield expressions, each of them stopping execution until the particular promise is settled. The code will create and execute each of the async tasks consecutively.

To wait for multiple tasks in parallel, you should yield an array of promises. This will wait until all of them are settled, and then return an array of result values. Using ES6 destructuring assignments leads to concise code for that:

Client.prototype.fetchCommentData = async(function* (user){
    var [county, data, notBanned] = yield [
//             a single yield only: ^^^^^
        countryService.countryFor(user.ip),
        api.getCommentDataFor(user.id),
        authServer.authenticate(user.id)
    ];
    if (!notBanned)
        throw new AuthenticationError(user.id);
    return {
        country: country,
        comments: data,
        notBanned: true
    };
});

The answer by Benjamin Gruenbaum is correct, but it loses the generators aspect completely, which tends to happen a bit when you're trying to run multiple things in parallel. You can, however, make this work just fine with the yield keyword. I'm also using some extra ES6 features like destructuring assignments and object initializer shorthand:

Client.prototype.fetchCommentData = async(function* (user){
    var country = countryService.countryFor(user.ip);
    var data = api.getCommentDataFor(user.id);
    var notBanned = authServer.authenticate(user.id).then(function(val){
        if(!val) throw new AuthenticationError(user.id);
    });

    // after each async operation finishes, reassign the actual values to the variables
    [country, data, notBanned] = yield Promise.all([country, data, notBanned]);

    return { country, data, notBanned };
});

If you don't want those extra ES6 features being used:

Client.prototype.fetchCommentData = async(function* (user){
    var country = countryService.countryFor(user.ip);
    var data = api.getCommentDataFor(user.id);
    var notBanned = authServer.authenticate(user.id).then(function(val){
        if(!val) throw new AuthenticationError(user.id);
    });

    var values = yield Promise.all([country, data, notBanned]);

    return { 
        country: values[0], 
        data: values[1], 
        notBanned: values[2]
    };
});

本文标签： javascriptSlowdown due to nonparallel awaiting of promises in async generatorsStack Overflow