Slowdown due to non-parallel waiting for promises in asynchronous generators

Question

I am 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, it is rather slow, I feel that my application is waiting too much for I / O, and it is not parallel. How to improve the performance of my application?

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

Answer 1

You spend too much time waiting for input / output from different sources.

In normal promise code, you would use Promise.all to do this, however - people tend to write code that expects 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, he should do:

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

Simply put, all of your I / O must be done in parallel.

To fix this, I would use Promise.props . Promise.props accepts objects and waits until all its properties are resolved (if they are promises).

Remember - generators and promises fit well and match, you just get 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 is shamelessly taken from Q-Connection by Chris Kowal}

Answer 2

As mentioned in the Bluebird Promise.coroutine for Promise.coroutine , you need to make sure that there is no yield in the 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 which stops execution until a specific promise is defined. The code will create and execute each of the async tasks sequentially.

To wait for multiple tasks in parallel, you must yield array of promises . This will wait until they are all installed, and then return an array of result values. Using assignments to restructure ES6 leads to a short code for this:

 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 }; }); 

Answer 3

Benjamin Gruenbaum's answer is correct, but he completely loses the completely generator aspect, which usually happens when you try to run several things in parallel. However, you can do this job perfectly with the yield keyword. I also use some additional ES6 features, such as destructuring assignments and object initializer reduction :

 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 do not want to use these additional ES6 features:

 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] }; });