Optimize copy(::Generic.MPoly)

[fingolfin]

Before, copy did the same as deepcopy. Now we only copy the coefficient and exponent vectors.

Before:

julia> R,(x,y) = polynomial_ring(QQ, [:x, :y]);

julia> f = y^2 + 1;

julia> @b copy(y)
377.016 ns (17 allocs: 1.078 KiB)

julia> @b copy(f)
561.188 ns (26 allocs: 1.656 KiB)

After:

julia> R,(x,y) = polynomial_ring(QQ, [:x, :y]);

julia> f = y^2 + 1;

julia> @b copy(y)
120.892 ns (5 allocs: 224 bytes)

julia> @b copy(f)
117.348 ns (5 allocs: 256 bytes)

However, the downside are potential aliasing issues... For f.exp I think there is no problem as it has type Matrix{UInt64}. But perhaps f.coeffs should still be deepcopied?

Should we decide to merge it, it might be best to treat this as "breaking" and mention it explicitly in the release notes, as someone somewhere might have code that relies on the current behavior.

Thoughts @joschmitt @thofma @fieker ?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.92%. Comparing base (f69ac97) to head (53c6cbe).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2180   +/-   ##
=======================================
  Coverage   87.92%   87.92%           
=======================================
  Files         127      127           
  Lines       31780    31782    +2     
=======================================
+ Hits        27943    27945    +2     
  Misses       3837     3837           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[thofma]

Hm, to be honest, not a big of a fan of this. This will be a longer comment, sorry.

Let me preface that I think it was a mistake that we use copy and deepcopy.

What we really need is mutable_copy thing, where the names implies what it is supposed to do. Give me y = mutable_copy(x) with the promise that mul!(y, ...) won't change x. So far, we have abused deepcopy for this.

Since copy(x) is supposed to be "shallow copy" and no one knows what a definition of this is, in Nemo/AbstractAlgebra, copy(x) is either defined (implicitly) as

copy(x) = x

or

copy(x) = deepcopy(x)

I am a bit hesitant to add yet another definition of copy.

If we want copy to be fast and different from deepcopy, I would suggest to go with copy(x::Generic.MPoly) = x for "consistency".

Edit: Same comment applies to #2178. I don't understand what this copy is suppose to be used for.