Skip to content

define equivalence on argument lists #5

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
michaelficarra opened this issue Mar 30, 2022 · 13 comments
Open

define equivalence on argument lists #5

michaelficarra opened this issue Mar 30, 2022 · 13 comments
Labels
question Further information is requested

Comments

@michaelficarra
Copy link
Member

The most important thing for this proposal to figure out is how it determines that a subsequent call is "equivalent" to a previous call so that it can return the stored result. This will involve at least

  1. Determining what comparison algorithm to apply to each positional argument.
  2. Determining whether extra arguments beyond the declared parameters contribute.
  3. Determining whether the this and new.target values contribute.

and probably more.

@zloirock
Copy link

zloirock commented Mar 30, 2022

I think that it should define a Function.prototype.memo callback, by default the key should be something like compositeKey(fn, this, ...arguments), I'm not sure about new.target in the default logic.

@js-choi
Copy link
Collaborator

js-choi commented Mar 30, 2022

My inclination is to let the developer supply any Map-like object as a cache, and to supply tuple keys to cache.has and cache.get (see also #3 and #4). That way the cache object can handle equivalence itself as part of its lookup. Map would be used by default.

The tuple keys would have the structure #[thisValue, newTargetValue, arg0, …].

@js-choi js-choi added the question Further information is requested label Mar 30, 2022
@zloirock
Copy link

Map would be used by default.

A strange option.

@js-choi
Copy link
Collaborator

js-choi commented Mar 30, 2022

@zloirock: new Map() would basically be the memory-unbounded version of new LRUMap(maxNumberOfEntries).

@zloirock
Copy link

@js-choi anyway - I think it's good for custom cach strategy, but I think that the user should be sure that with the default cach strategy the method will not be called again with the same arguments and for arguments that GCed and no longer available the result will not pollute memore.

@js-choi
Copy link
Collaborator

js-choi commented Mar 30, 2022

Yes, the question of what caching policy we should use by default (unbounded memory versus garbage-collectable weak references if possible) is complex, but it probably should go more in #3.

As far as this particular issue goes (what function calls are considered equivalent), I definitely think that this and new.target should be distinguished in the cache. And we should probably use === semantics to compare values (like Map does), and we could use argument tuples #[thisVal, newTargetVal, arg0, arg1, …], for comparing argument lengths and values in order.

@michaelficarra
Copy link
Member Author

@js-choi Map keys don't use strict equality comparison, they use SameValueZero (and replace -0 with 0). So NaNs are equivalent, for example, unlike in strict equality comparison.

@js-choi
Copy link
Collaborator

js-choi commented Mar 30, 2022

@michaelficarra: Ah, yes, that is true…I wonder if this disqualifies Map-like caches. Or would we be fine with using Map-likes and caching f(+0) and f(-0) the same…

@michaelficarra
Copy link
Member Author

@js-choi If only Maps and Sets had used SameValue...

@js-choi
Copy link
Collaborator

js-choi commented Mar 30, 2022

@michaelficarra: Well, if we allow developers to supply custom user-supplied map-like caches, then the developer can supply a map-like object that treats -0 specially:

f.memo(new MapLikeCacheThatDistinguishesNegativeZero)

So the problem would be more about what default behavior we want. And, given that the Committee already decided that ordinary Maps should use SameValueZero, it likely will resolve on SameValueZero as the default memoization argument-matching behavior, too.

@zloirock
Copy link

The tuple keys would have the structure #[thisValue, newTargetValue, arg0, …]

I'm not sure, what do you mean? Tuples can't store objects - only primitives, Records and other Tuples.

@js-choi
Copy link
Collaborator

js-choi commented Mar 30, 2022

@zloirock: Ah, yes, I had forgotten that. Let’s continue in #4 (comment).

@js-choi js-choi mentioned this issue Mar 30, 2022
js-choi added a commit that referenced this issue Mar 31, 2022
@NickGard
Copy link

Regardless of how the cache is built, the comparator function only really cares if the arguments for two different calls produce the same result. Or to get a little more in-depth, it cares if the cache key produced by the arguments of two different calls are the same. It may be helpful to split up the idea of creating keys and comparing keys into different functions, say hash and compare. Then we could encode extra parameters outside of the argument list into the key like so:

let extraParam = 2;

const memoFn = Function.memoize(fn, {
  hash: function (...args) {
    return [
      this,  // <- at call time, 'this' is captured as part of the cache key
      extraParam,  // <- at call time, extraParam is captured as part of the cache key
      ...args
    ];
  },
  compare: (keyA, keyB) => 
    keyA.length === keyB.length && 
    keyA.every((key, index) => key === keyB[index])
  )
});

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants