JuliaAlgebra / DynamicPolynomials.jl

Multivariate polynomials implementation of commutative and non-commutative variables
Other
60 stars 20 forks source link

`MA.operate!(+, pol1, pol2)`: prevent captured variable perf issue #145

Closed nsajko closed 7 months ago

nsajko commented 7 months ago

The local function was spuriously recursive, making it not recursive fixes the performance problem. That is, this removes some run time dispatch and decreases the amount of allocation, but there's still some run time dispatch left in other places, according to JET.jl.

Before:

julia> import MutableArithmetics; const MA = MutableArithmetics;

julia> using DynamicPolynomials

julia> @polyvar x y;

julia> p = 2x + y;

julia> q = x + 2y;

julia> @allocated MA.operate!(+, p, q)
34413936

After:

julia> import MutableArithmetics; const MA = MutableArithmetics;

julia> using DynamicPolynomials

julia> @polyvar x y;

julia> p = 2x + y;

julia> q = x + 2y;

julia> @allocated MA.operate!(+, p, q)
30835632

Both REPL runs was with nightly Julia v1.11.

blegat commented 7 months ago

It seems the issue is that get1 does not infer correctly ? Maybe fixing this underlying issue will save even more. What about annotating it with _NoVarTerm ?

nsajko commented 7 months ago

Not sure why you think that? Have you checked with Cthulhu or something? The issue that I fixed in this PR is a standard performance of captured variables issue, where the closure is itself the captured variable, so making it nonrecursive prevents boxing, as far as I understand.