improvement: memoize Comparable's type-pair dispatch in a release - #2819
Conversation
| defp dispatch(left_type, right_type) do | ||
| key = {__MODULE__, :dispatch, left_type, right_type} | ||
|
|
||
| case Process.get(key) do |
There was a problem hiding this comment.
Maybe we should use persistent term here actually to memoizenit forever?
There was a problem hiding this comment.
That was my first thought too. I'm trying to avoid subtle bugs if/when comparators are changed during development or runtime synthesis. At least a process is restarted in development and can be easily restarted on hot code swap, whereas persistent_term lodges these in the VM where they will survive tests, etc. I've not had lots of experience with either approach so I'm happy to be guided by your decision.
With persistent_term I briefly considered using :erase in defcomparable, but it is a convenience macro so no guarantee that it would be used, another footgun.
There was a problem hiding this comment.
Hmm...thats a fair point, but I feel like we could work around it. We could use a sentinel like build_embedded or something like that to not do this in development environments 🤔
7128900 to
105376c
Compare
|
I think there may be a simpler way to do this. I think what we can do is use something like |
matt-beanland
changed the title
|
I was drafting an detailed explanation, but you've already read the code. In my view we should provide an option to control the memoization irrespective of the environment. So can memoize or not (benchmarks, etc), and can reset if necessary at runtime rather than restart a VM. I don't think we can rely on a Comparator implementing anything other than the protocol. |
|
I've tried the And being compile-time, it wouldn't reach the case I'm most concerned about anyway: a comparator So I think the two functions on Comp are necessary. |
|
#2840 should be reviewed/merged first, it deals with a fairly unlikely hazard regarding Comp.compare dispatch which is compounded with memoization. |
|
will merge once conflicts are resolved 👍 |
Interleave and repeat several distinct operand type pairs through In.evaluate/1 so a later change memoising the type-pair -> comparator resolution cannot let one pair's cached comparator decide another. Passes against the current per-comparison resolution.
`Comp.new/2` rebuilds a `Comparable.Type.<L>.To.<R>` module name on every comparison. For any pair with no specific comparator — the common case — that is a `Module.safe_concat/1` raising `ArgumentError`, caught, and a second name built for the `Any.To.Any` fallback. Resolving a pair is a pure function of the two type atoms, so it can be memoized. Where it is safe to keep the answer depends on whether code can still change. `Module.safe_concat/1` resolves on atom existence, and atoms only ever appear, so the one way a cached answer goes stale is a pair resolving to the fallback before a specific comparator's atom exists. In `:embedded` mode — a release — every module is loaded at boot and none arrives later, so the resolution is fixed for the life of the VM and `:persistent_term` can hold it once for every process. Under `mix`, `iex` and tests the mode is `:interactive`, code is loaded lazily and recompiled in place, and nothing is cached. `memoize_dispatch/1` overrides the mode at runtime and clears on the way through, so a switch cannot leave a stale answer behind; `reset_dispatch/0` discards what is held. Both are needed by anything that defines a comparator at runtime, and turning it on is how a benchmark or a test reaches the cached path at all, since both run interactively. Being off means nothing is written, so a read is a `:persistent_term` miss and the answer is resolved as before. The flag is therefore only consulted when a pair is first resolved, never on a cached read, and the switch costs nothing on the hot path. `In.evaluate/1` over a 421-element `MapSet` miss, the per-record shape of a runtime `in`-filter: | build | off | on | | | --- | ---: | ---: | ---: | | dev, unconsolidated | 343.9 µs | 107.2 µs | 3.2x | | prod, consolidated | 236.4 µs | 24.8 µs | 9.5x |
105376c to
8ac3831
Compare
|
🚀 Thank you for your contribution! 🚀 |
2 participants
Contributor checklist
Leave anything that you believe does not apply unchecked.
Summary
Comp.new/2rebuilds the dispatch module on every comparison. For any pair with no specific comparator (the common case) that is a raised and caughtArgumentError. This memoises the resolution, a pure function of the two type atoms, in the process dictionary. No semantics change.It shows up in runtime
in-filters. AMapSetmiss falls back toEnum.any?(set, &Comp.equal?(&1, left)), so every non-matching record scans the set and each element pays that dispatch twice over. The change is to the vendoredComp. The O(n) miss scan is left as is.Evidence
In.evaluate/1over a miss (binary left,MapSetof binaries) is the per-record shape of a runtimein-filter. Elixir 1.20 / OTP 27.Module.safe_concat/1for one 421-element miss: 842 to 0.