Moved cache queue before of the store get function. (up to 2x performance boost).

[aletorrado]

Hi! This change prevents the cache store from being bursted. I'm getting up to 200% performance boost when using a local redis store, and should be higher with caches located farther.

[BryanDonovan]

Thanks! Can you please run "make" and fix the lint and test errors?

[coveralls]

Coverage remained the same when pulling 80f2318 on aletorrado:master into 13eb093 on BryanDonovan:master.

[BryanDonovan]

Thanks, I'll check this out in more detail ASAP.

[aletorrado]

Great thanks!

[coveralls]

Coverage decreased (-1.33%) when pulling 7bd7e9e on aletorrado:master into 13eb093 on BryanDonovan:master.

[aletorrado]

I've added domain support to make sure the wrap callback function is always called.

[BryanDonovan]

Sorry, I got busy and didn't get around to checking this out in more detail. I will today.

[BryanDonovan]

@aletorrado - Do you know if there's a way to get test coverage for this domain error?

[aletorrado]

You can just throw an exception in the work function and check that the
callback function is called only once and that it receives that error.
El dic 18, 2014 8:19 PM, "Bryan Donovan" notifications@github.com
escribió:

In lib/caching.js
#18 (diff)
:

         } else {

- self.queues[key] = [cb];

•            work(function () {
  

•                var work_args = Array.prototype.slice.call(arguments, 0);
  

•                if (work_args[0]) { // assume first arg is an error
  

•                    self.queues[key].forEach(function (done) {
  

•                        done.call(null, work_args[0]);
  

•                    });
  

•                    delete self.queues[key];
  

•            domain
  

•            .create()
  

•            .on('error', function(err) {
  

@aletorrado https://github.com/aletorrado - Do you know if there's a
way to get test coverage for this domain error?

—
Reply to this email directly or view it on GitHub
https://github.com/BryanDonovan/node-cache-manager/pull/18/files#r22079610
.

[BryanDonovan]

Thanks. Can you please add tests for this? E.g.,

            context("when an error is thrown in the work function", function () {
                var fake_error;

                beforeEach(function() {
                    fake_error = new Error(support.random.string());
                });

                it("bubbles up that error", function (done) {
                    cache.wrap(key, function () {
                        throw fake_error;
                    }, ttl, function (err) {
                        assert.equal(err, fake_error);
                        done();
                    });
                });
            });

around line 360 in caching.unit.js and something similar in multi_caching.unit.js?

[coveralls]

Coverage remained the same when pulling a6cb298 on aletorrado:master into 13eb093 on BryanDonovan:master.

[BryanDonovan]

Thanks, I'll merge now.

[BryanDonovan]

This is in 0.15.0 now.

[aletorrado]

Great! Thank you!
El dic 18, 2014 9:06 PM, "Bryan Donovan" notifications@github.com
escribió:

This is in 0.15.0 now.

—
Reply to this email directly or view it on GitHub
#18 (comment)
.