Data: Call resolver isFulfilled once per argument set#6360
Conversation
|
A downside here is that memory use could grow out of control, particularly for selectors which receive objects as arguments, where those arguments may be new references each time. e.g. getFoo( {} );...would create a memoize entry each time it's called. While on the surface it seems like a sensible optimization, I'm going to investigate further whether this is actually required for my use-case, or if repeated calls to |
|
There's still the same issue here as in #5826, where if we don't memoize Re: The issue raised in #6360 (comment): I have a utility library I've been working on in my spare time which could fit well here, for tracking "equivalent" values via complex keys (deeply). I still think it's a reasonable expectation that fulfillment should only ever occur zero or one time per "same" argument set. |
|
The latest commit a902662 drops |
youknowriad
left a comment
There was a problem hiding this comment.
This looks good.
What do you think about the performance impact? Is it fine?
data/index.js
Outdated
There was a problem hiding this comment.
How many of those packages you have :)
Speaking to computational performance, I think it should be fine enough. Related: https://github.com/aduth/equivalent-key-map#performance-considerations In future maintenance, we might consider to avoid A more practical concern might be memory consumption of tracking each set of arguments for every resolver. Arguably this is a worse concern with the previous implementation, where we were tracking arguments, but not considering deep equality. The internal structure of e.g. These calls:
...would be tracked roughly as: {
"_ekm_index_0": {
"1": {
"_ekm_index_1": {
"2": {
"_ekm_index_2": {
"page": {
"1": {
"_ekm_value": true
},
"2": {
"_ekm_value": true
},
"3": {
"_ekm_value": true
}
}
}
}
}
}
}
} |
a902662 to
7b67a6b
Compare
Related: #6084
This pull request seeks to revise the behavior of a resolver's explicit
isFulfilledcallback to occur a maximum of one time per argument set.isFulfilledis only necessary to call once, as it is intended to support cases where a fulfillment resolver may not be needed. It is assumed that after the initial case, fulfillment condition would never revert back to needing to be satisfied.Testing instructions:
Ensure unit tests pass: