Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

Performance issue #29

Open
weizhu opened this issue Nov 22, 2013 · 2 comments
Open

Performance issue #29

weizhu opened this issue Nov 22, 2013 · 2 comments

Comments

@weizhu
Copy link

weizhu commented Nov 22, 2013

Hi,

I've using node-sync for a while and love it.
However, when my code starts to doing a large number of async call with Sync.future, I am running into serious performance issue. I narrowed it down to node-sync module.

Here is my test code that does't nothing but doing 2000 empty async full on AWS m1.small instance and it took 847ms.

At this level of performance, it's almost unusable. Help!

// Versions
// ├─┬ [email protected]
//│ └── [email protected]

"use strict";
var Sync = require('sync');

var getByIdCb= function(id, cb) {
  cb(null, 'foo');
}

var getByIdAsync = function(id, cb) {
  return 'foo';
}.async()

var testSync = function() {
  Sync(function() {
    var places=[];

    console.time('getByIdAsync');
    // Load places
    for (var i=0; i < 2000; i++) {
      places.push(getByIdAsync.future(null, i));
    }
    console.timeEnd('getByIdAsync');

    console.time('getByIdCb');
    // Load places
    for (var i=0; i < 2000; i++) {
      places.push(getByIdCb.future(null, i));
    }
    console.timeEnd('getByIdCb');
  }, function(e) {
    if (e) {
      console.error(e.stack);
    }
  });
}

testSync(); 

This is the output:
/server/node$ node profileSync.js
getByIdAsync: 847ms
getByIdCb: 575ms

@ybogdanov
Copy link
Owner

Hi,

Yep, sync.future could be slow. Please run this benchmark in your environment https://github.com/0ctave/node-sync/blob/master/benchmarks/index.js

Also, in a real life, you will never be calling 2000 empty functions in a row. Imagine those functions are doing some i/o etc with average response time 10ms. So, your test will take 10*2000 + 847 = 20847 - sync takes only 4% of this time. In most cases it's allowable trade-off in favor of comfortable development and readable code.

In other cases (some async intensive stuff with very fast responses) to achieve the best performance nodejs could give, you should not use sync.

@weizhu
Copy link
Author

weizhu commented Nov 23, 2013

Hi 0ctave, the sample code I used is using empty function only because I am doing pure perf measurement of of sync.future.

For my real world usage, I need to load 2000 objects and filter them.
So I am writing a loop to load the objects in future first, before using them.
With future use, my code can make batch loading of data from caching layer.
The IO costs only 50-100ms, but the rest of the loading code took 5 seconds!

I believe my usage is a reasonable scenario where sync.future should be used to in order to archive maximum IO performance. Also, I believe that there should be a way to optimize node-sync to dramatically improve its performance. For example, I tried to re-write the sample code in Fibers future (see https://gist.github.com/weizhu/7613409) and it took only 44 ms. That's 10-20x faster than sync.future.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants