Does `MutableDiffResult` have to exist at all?
Preliminary: DiffResult objects must always be realiased when updated, as mentioned in the docs
https://juliadiff.org/DiffResults.jl/stable/#DiffResults.value!
In most cases it is unnecessary, because the result object itself gets mutated. However, it causes a heap allocation to create the mutable struct defined here:
https://github.com/JuliaDiff/DiffResults.jl/blob/724a23d326353353aa78ae9b9e770685119492dc/src/DiffResults.jl#L19-L25
Here's a proof of concept: instead of the current code
https://github.com/JuliaDiff/DiffResults.jl/blob/724a23d326353353aa78ae9b9e770685119492dc/src/DiffResults.jl#L156-L157
we could instead return a new object every time
value!(r::MutableDiffResult, x::Number) = MutableDiffResult(x, r.derivs)
value!(r::MutableDiffResult, x::AbstractArray) = (copyto!(value(r), x); return r)
- Would it decrease or increase the performance of the update? Don't know
- Would it be considered a breaking change? Perhaps (see below)
- Would it break stuff in practice? Definitely (see below)
In terms of SemVer, I'm not sure whether the mutable/immutable distinction is part of the API. It is mentioned in a few docstrings but the concrete types themselves have no docstrings
https://juliadiff.org/DiffResults.jl/stable/#DiffResults.DiffResult
I know that some dependencies (like ForwardDiff and ReverseDiff) break if we decide to return a new struct in more cases, because they don't follow the documentation's advice on realiasing. But then again they misuse the interface so it's pretty much their fault, right?
Here's an issue to keep track of these miscreants:
- https://github.com/JuliaDiff/DiffResults.jl/issues/27
Would it decrease or increase the performance of the update? Don't know
Yes, there is an ImmutableDiffResult and if you update the package to use that you see resulting heap allocations.
Would it be considered a breaking change? Perhaps (see below)
Yes
Would it break stuff in practice? Definitely (see below)
Yes, there are many SciML examples use it for mutation and ignore the return.
Not sure i understand your answer to the performance aspect, I asked whether it would be good or bad and you said yes ^^
The SciML packages that use results without realiasing are already incorrect for StaticArrays, even without changes in DiffResults. Example: https://github.com/JuliaDiff/ReverseDiff.jl/issues/251#issuecomment-2000036388 In this regard, wouldnt it make sense to fix those used anyway?
No, those would use the immutable diff results and a different code path.
Not sure i understand your answer to the performance aspect, I asked whether it would be good or bad and you said yes ^^
It causes allocations last time I checked. In certain scenarios it can elide but often it does not.