Open Artturin opened 1 year ago
The function returned by memoise
as implemented in 0395b9b is strict in its argument. I suppose this may be expected. However...
Potentially worse is that it does not consistently perform a deep traversal, but rather evaluates attributes (etc) based on which other arguments the function has seen in the past. This breaks strong referential transparency. The strictness behavior of a function is observable, so it must not depend on unrelated past arguments.
This may be resolved by doing a proper deepSeq
on the argument. This avoids the head scratching but does make memoise less useful and probably a little slower.
An alternative is to make the evaluation of the argument optional. If the comparison and lookup encounter an exception, we may catch the exception and behave accordingly. It's always possible to just assume that the argument is distinct from anything else, and therefore apply no memoization, but perhaps we could continue the comparison, maybe compare exceptions. If both the current argument and a cached argument behave the same, we may consider them equivalent. We might consider exceptions to be values throwing an exception to be smaller than everything for the purpose of MemoArgComparator
. However, this assumes that the value produces an exception consistently and cheaply. This may not be the case! Some exceptions may be retried after tryEval
.
I've opened a draft PR for it, #10280.
just a issue to not have the memoise primop hidden and forgotten in a commit on a non merged branch https://github.com/NixOS/nix/commit/0395b9b94af56bb814810a32d680c606614b29e0
'builtins.memoise f x' is equal to 'f x', but uses a cache to speed up repeated invocations of 'f' with the same 'x'. A typical application is memoising evaluations of Nixpkgs in NixOps network specifications. For example, with the patch below, the time to evaluate a 10-machine NixOps network is reduced from 17.1s to 9.6s, while memory consumption goes from 4486 MiB to 3089 MiB. (This is with GC_INITIAL_HEAP_SIZE=16G.)
Priorities
Add :+1: to issues you find important.