-
-
Notifications
You must be signed in to change notification settings - Fork 92
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: add ObliviousLRUCache with delete functionality #162
Conversation
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.
Hello @trivikr, thanks for this new PR. I have added some comments on this review. Have a good day.
*/ | ||
import {IArrayLikeConstructor} from './utils/types'; | ||
|
||
export default class ObliviousLRUCache<K, V> implements Iterable<[K, V]> { |
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.
I may be wrong but I think you can import the LRUCache
class in this definition file and make this class definition extends
it, no?
setpop(key: K, value: V): {evicted: boolean, key: K, value: V}; | ||
get(key: K): V | undefined; | ||
peek(key: K): V | undefined; | ||
remove(key: K): this; |
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.
This should be delete
:)
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.
And probably return boolean
, based on next comment.
} | ||
|
||
if (this.head === pointer && this.tail === pointer) { | ||
this.clear(); |
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.
Using #.clear
here will reallocate the #.items
object. Resetting head, tail and size to 0
should do the trick? Your condition can be simplified to this.size === 1
to be more performant if I understand the problem correctly.
* @param {any} key - Key. | ||
* @return {undefined} | ||
*/ | ||
ObliviousLRUCache.prototype.delete = function(key) { |
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.
This method could a boolean
indicating whether an item was delete, like the ES6 Map
does, no?
@@ -215,8 +216,52 @@ function makeTests(Cache, name) { | |||
|
|||
assert.deepStrictEqual(entries, Array.from(cache.entries())); | |||
}); | |||
|
|||
if (name === 'ObliviousLRUCache') { | |||
it('should be possible to delete keys from a LRU cache.', function() { |
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.
I am wondering whether a rolling test should be done? Like add 2 remove 1, in a loop of ~10 items for a cache of ~4-5 capacity.
Any update on this @trivikr? |
Superceded by #173 |
Fixes: #143
This PR creates ObliviousLRUCache with
delete
functionality.Changes done:
delete
functionality using a PointerArray to store deleted pointers.