-
-
Notifications
You must be signed in to change notification settings - Fork 59
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
feat(cache): plain object cache #162
Conversation
Benchmark result:
|
I notice repetitive codes, perhaps can refactor into a |
@curbengh I see no necessary. I have bring up something like this: // cache.js
'use strict';
module.exports = class Cache {
constructor() {
this.cache = {};
}
set(id, value) {
this.cache[id] = value;
}
has(id) {
return this.cache[id] != null;
}
get(id) {
return this.cache[id];
}
flush() {
this.cache = {};
}
}; But the usage will end in something like: const Cache = require('./cache');
const cache = new Cache();
if (cache.has(cacheId)) return cache.get(cacheId);
...
cache.set(cacheId, value); Nothing changed... |
const Cache = require('./cache');
const cache = new Cache();
if (cache.has(cacheId)) return cache.get(cacheId);
...
cache.set(cacheId, value); while it doesn't reduce the line, it does look more readable. |
@curbengh I have bring up |
Performance with
|
7a5ded5
to
afdc7f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
I tasted this patch in a large blog (dummy 3000 posts). The result shows this patch is 13% faster than Details:Measure elapsed time of ResultElapsed time:
Source code:
Environment:
|
Memory usage is about 2.3GB as the worst case in both conditions. |
Context: #158
Cache with plain object.
cc @dailyrandomphoto @seaoak