-
Notifications
You must be signed in to change notification settings - Fork 19
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
Use WeakMap and FinalizationRegistry when available. #132
Conversation
8715d17
to
b6e5f8b
Compare
if (useWeakKeys && key && typeof key === "object") { | ||
let ref = weakRefs!.get(key)!; | ||
if (!ref) { | ||
weakRefs!.set(key, ref = {}); | ||
registry!.register(key, ref); |
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.
The !
coercions here are safe because useWeakKeys
implies weakRefs
and registry
must be defined.
// Optional WeakMap mapping object keys returned by makeCacheKey to | ||
// empty object references that will be stored in the cache instead of | ||
// the original key object. Undefined/unused if useWeakKeys is false. | ||
// It's tempting to use WeakRef objects instead of empty objects, but | ||
// we never actually need to call .deref(), and using WeakRef here | ||
// noticeably slows down cache performance. | ||
const weakRefs = useWeakKeys | ||
? new WeakMap<object, {}>() |
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.
Realizing that WeakRef
is not necessary was the biggest (pleasant) surprise of this work.
"lib": ["es2015"], | ||
"lib": ["esnext"], |
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 change brings in the TypeScript typings for WeakRef
and FinalizationRegistry
.
b6e5f8b
to
e1de530
Compare
e1de530
to
995cff9
Compare
Although
WeakRef
andFinalizationRegistry
are not yet supported by all browsers, I was pleasantly surprised to learn that more than 68% of browsers already have support for these new APIs.Using a
FinalizationRegistry
is appealing for this library, because it allows deleting keys from the internal LRU cache when they become unreachable, which makes sense because an unreachable key can never be looked up in the cache again. Without this functionality, a naive LRU cache will keep keys alive that could otherwise be garbage collected.This PR uses a
FinalizationRegistry
when possible, but also allows the behavior to be configured with theuseWeakKeys
option for thewrap
function, since the additional overhead of the registration and cleanup has a noticeable performance cost (the performance stress test run bynpm test
now takes ~1325ms instead of ~950ms, on my machine).