Skip to content
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

Using Bluebird instead of Native Promise in RedisCacheAdapter #3359

Closed
linpf870208 opened this issue Jan 12, 2017 · 5 comments
Closed

Using Bluebird instead of Native Promise in RedisCacheAdapter #3359

linpf870208 opened this issue Jan 12, 2017 · 5 comments

Comments

@linpf870208
Copy link

when query or update data, parse need to query from cacheAdapt frequently, and as benchmarks showing native is more slower when compared to bluebird. So should we use bluebird instead of Native Promise in RedisCacheAdapter?

In my tests via benchmark, use bluebird would highly improve performance.

@Caijiacheng
Copy link

try to post the performance bench of BlueBird or PR

@flovilmart
Copy link
Contributor

flovilmart commented Jan 15, 2017

i ran the test suite with and without bluebird:

$ node --version
v6.2.2

npm test 77,77s user 5,53s system 69% cpu 1:59,65 total

BLUEBIRD=1 npm test 75,94s user 5,82s system 52% cpu 2:36,02 total

$ node --version
v4.3.2

npm test  171,79s user 7,88s system 80% cpu 3:44,20 total

BLUEBIRD=1 npm test # FAILURE!

That's not really convincing me for the High performance expected from BB...

@linpf870208
Copy link
Author

linpf870208 commented Feb 4, 2017

@flovilmart

machine parameters:
cpu cores: 2
model name: Intel(R) Xeon(R) CPU E5-2430 0 @ 2.20GHz
mem 4096MB
1Mbps

first i ran the benchmark for InMemoryCache, cpu ran 100%.
secondly i ran it with RedisCacheAdapte, cpu just ran 60%. i feel confuse about that.
so i use bluebird to wrap redis, then cpu ran 100% again.

code like this:

get(key) {
    debug('get', key);
    return this.client.getAsync(key).then(res => {
      if (!res) {
        return null;
      }
      return JSON.parse(res);
    }).catch(err => {
      return null;
    });

    // debug('get', key);
    // this.p = this.p.then(() => {
    //   return new Promise((resolve) => {
    //     this.client.get(key, function(err, res) {
    //       debug('-> get', key, res);
    //       if(!res) {
    //         return resolve(null);
    //       }
    //       resolve(JSON.parse(res));
    //     });
    //   });
    // });
    // return this.p;
  }

my benchmark result as follow:
ab -n 20000 -c 50 -k -H 'X-Parse-Application-Id: APPID' -H 'X-Parse-Master-Key: MASTERKEY' -T  application/json -p ab.txt

InMemoryCache
CPU 100% MEM 3.4% TPS 168 LTC 300ms
RedisCache
CPU 60% MEM 2.9% TPS 51 LTC 968ms
RedisCache with Bluebird
CPU 99% MEM 3.1% TPS 119 LTC 418ms

According to issue http://softwareengineering.stackexchange.com/questions/278778/why-are-native-es6-promises-slower-and-more-memory-intensive-than-bluebird
new Promise is an extremely slow way of creating a promise, first the executor function allocates a closure, secondly it is passed 2 separate closures as arguments. That's 3 closures allocated per promise but a closure is already a more expensive object than an optimized promise

i think new Promise result in slower permance,what do you think?

@acinader
Copy link
Contributor

acinader commented Mar 31, 2017

I'd like it if we could substitue bluebird in as the promise library.

both better performing and its got a richer api that I am getting used to having!

things like mongo, elastic cache, etc. give one the option to configure in the promise lib of choice.

not sure what's involved...yet

@flovilmart
Copy link
Contributor

Faster is not a given :/ that highly depends on the nodejs version. Using bluebird, you can swap it globally, and that doesn't require any change in parse-server.

As for the new Promise pattern, well, that implies an extensive rewrite

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

No branches or pull requests

4 participants