How can I speed up this sequence of promises?

Question

I have the following HTTP request handled in my node server. I have to send a list of disks back as a response.

The code is:

DiskPromise.getDiskCount(client).then(function (diskCount) {
    DiskPromise.getDisks(client, diskCount).then(function (disks) {
        RaidPromise.getRaidCount(client).then(function (raidCount) {
            RaidPromise.getRaidArrays(client, raidCount).then(function (raidArrays) {
                for (i in disks) {
                    disks[i].setRaidInfo(raidArrays);
                }
                RaidPromise.getGlobalSpareList(client).then (function(spareNames) {
                    for (i in disks) {
                        disks[i].setSpareNess(spareNames);
                    }
                    res.json(disks);
                }, function (err) {
                    console.log("something (either getDiskCount, or one of the getDisk calls) blew up", err);
                    res.send(403, { error: err.toString() });

                });
            });
        });
    });
});

the promises are SOAP calls. It takes anywhere from 4.5 to 7.0 seconds for the client to get a response back.

Am I doing something structurally wrong laying out the code?

when I break the promises to determine which is the slowest or slow, I don't find one that even takes close to a second... So I was thinking may be it is the way I have structured my code might be the issue. I am making 5 promise calls... — reza, May 30 '14 at 22:33
What you really need to do is to combine the above logic into a single API call to your server (obviously requires changes on the server-side of things). The number of roundtrip requests/responses you are waiting for is large and that makes it slow because each request has a fixed overhead to it. What you really want is one master call to your server that gets you the final results in one call (or a lot less calls than here). If you're going to keep all these serialized calls, then the only thing you can do is make your server respond quicker. — jfriend00, May 30 '14 at 23:03
FYI, this won't affect performance, but will affect code readability. You can chain your `.then()` calls rather than nest them. — jfriend00, May 30 '14 at 23:04
@jfriend00, I could not figure out how to chain them when one promise has a result that the next one uses. Can you help me with that? — reza, May 30 '14 at 23:13
The return result of a `.then()` will be the argument to the next `.then()` or if you return a promise, then the resolved arguments will be passed to the next `.then()` handler when the promise is resolved (for async operations). See here: http://www.html5rocks.com/en/tutorials/es6/promises/#toc-chaining — jfriend00, May 30 '14 at 23:15
See http://stackoverflow.com/questions/22539815/arent-promises-just-callbacks Also, make the server calls in parallel, not sequentially. — Benjamin Gruenbaum, May 31 '14 at 03:01

score 2 · Accepted Answer · answered May 31 '14 at 14:45

2

Analyzing your code implies trivial parallelization opportunities for fetching disks, raidArrays and sparenames. Performance greatly increased

var disks = DiskPromise.getDiskCount(client).then(function (diskCount) {
    return DiskPromise.getDisks(client, diskCount);
});
var raidArrays = RaidPromise.getRaidCount(client).then(function (raidCount) {
    return RaidPromise.getRaidArrays(client, raidCount);
});
var spareNames = RaidPromise.getGlobalSpareList(client);

Promise.all([disks, raidArays, spareNames]).spread(function(disks, raidArrays, spareNames) {
    for(var i in disks) {
        disks[i].setRaidInfo(raidArrays);
        disks[i].setSpareNess(spareNames);
    }
    res.json(disks);
}).catch(function(err) {
    console.log("something (either getDiskCount, or one of the getDisk calls) blew up", err);
    res.send(403, { error: err.toString() });
});

answered May 31 '14 at 14:45

Esailija

138,174
23
272
326

I will try your solution as soon as I can. I am curious how you say: "Performance greatly increased". How do you know that fact? – reza Jun 02 '14 at 03:50
@reza because it starts the process of fetching disks, raidArrays and spareNames right away. Let's say each takes 2 seconds - if you do them sequentially like in your original code, it will take 6 seconds because `2 + 2 + 2 = 6`. If you do them in parallel like I do, it will take 2 seconds because `max(2, 2, 2) = 2` – Esailija Jun 02 '14 at 08:12
I tried your code. I have to say there is really no performance improvement. Before and after using Chrome developer tools to see the network times, the times are less than 5% different. Whoever I do see your valid points and I do like your code layout much better. – reza Jun 02 '14 at 16:08
@reza That is possible if only one of the calls is very slow: `max(0.2, 0.2, 6) = 6` vs `0.2 + 0.2 + 6 = 6.4`. – Esailija Jun 02 '14 at 16:20

How can I speed up this sequence of promises?

1 Answers1