Add a macro to opt into aggressive constprop #38080

Keno · 2020-10-17T22:07:07Z

Right now aggressive constprop is essentially tied to the inlining
threshold (or to function's name being getproperty or setproperty!
respectively), which can be both somewhat brittle if the inlining cost
changes and insufficient when you do really know that const prop
would be beneficial even if the function is not inlineable. This
adds a simple macro that can be used to manually annotate methods
to force aggressive constprop on them.

timholy

Like it!

base/expr.jl

test/core.jl

JeffBezanson · 2020-10-20T20:43:26Z

test/core.jl

+# The first test just makes sure that improvements to the compiler don't
+# render the annotation effectless.
+@test Base.return_types(f_nonaggressive, Tuple{Int})[1] == Val
+@test Base.return_types(f_aggressive, Tuple{Int})[1] == Val{1}


I'd move these to test/compiler/inference

JeffBezanson · 2020-10-20T20:49:49Z

stdlib/Serialization/src/Serialization.jl

@@ -22,7 +22,8 @@ mutable struct Serializer{I<:IO} <: AbstractSerializer
    table::IdDict{Any,Any}
    pending_refs::Vector{Int}
    known_object_data::Dict{UInt64,Any}
-    Serializer{I}(io::I) where I<:IO = new(io, 0, IdDict(), Int[], Dict{UInt64,Any}())
+    version::Int


This is probably a good idea, but I still usually try to handle these by making the data fully self-describing, i.e. store the new field in such a way that the type of value you deserialize tells you which version of the object you have.

That's what I tried at first, but it just becomes super messy, for what seems to be little benefit. I don't see a realistic way to support non-versioned serialization blobs stably, so we might as well require the version.

Keno · 2020-12-17T08:49:08Z

I've rebased this. @JeffBezanson could I bother you to change the Serialization code to your satisfaction. I'm still not entirely convinced I understand why we can't just do the simple version-based thing - the current tricks in there just seem extremely brittle and non-scalable, but if you'd prefer the other thing, I guess that's ok - I just the feature ;).

vtjnash · 2020-12-17T15:54:37Z

Why is this useful, when it's not inlined, so this code will typically not be what ends up getting run? Do we need instead some degrees of @inline strength?

Keno · 2020-12-17T16:13:48Z

Why wouldn't it be useful? Aggressive constprop affects the return type we infer, so even if it's not inlined, sharper type information in the caller can make for a much more efficient function.

JeffBezanson · 2021-01-05T02:26:16Z

Ok, I think we might as well merge this as-is.

vtjnash · 2021-01-05T03:07:18Z

stdlib/Serialization/src/Serialization.jl

@@ -716,6 +721,7 @@ function readheader(s::AbstractSerializer)
        error("""Cannot read stream serialized with a newer version of Julia.
                 Got data version $version > current version $ser_version""")
    end
+    s.version = version
 end


Suggested change

end

nothing

end

vtjnash · 2021-01-05T03:07:57Z

test/core.jl

@@ -7527,4 +7527,4 @@ const RedefineVarargN{N} = Tuple{Vararg{RedefineVararg, N}}
 const RedefineVarargN{N} = Tuple{Vararg{RedefineVararg, N}}

 # NTuples with non-types
-@test NTuple{3, 2} == Tuple{2, 2, 2}
+@test NTuple{3, 2} == Tuple{2, 2, 2}


unrelated (and slightly worse)

timholy · 2021-01-05T12:11:28Z

Does it make any sense to also have a way of blocking constant-prop? I'm thinking #38983. It seems pretty cheap to reinfer for specific constants in many cases, though maybe not all. So 🤷‍♂️

JeffBezanson · 2021-01-05T19:26:12Z

Yes that would make sense. We've seen cases (though it's fairly rare) where it's useful to manually block constant prop.

vtjnash · 2021-01-05T19:50:40Z

We have @noinline already that will do that, but I think the observation there is that we want to consider @nospecialize and avoid inspecting specific typed specializations.

timholy · 2021-01-06T12:24:19Z

We have @noinline already that will do that, but I think the observation there is that we want to consider @nospecialize and avoid inspecting specific typed specializations.

Good point although I'm pretty sure @noinline doesn't prevent constant-prop on Core.kwfunc and possibly (haven't checked) Base.bodyfunction methods. The first is probably cheap, the second (if it happens) maybe not so much. xref tlienart/Franklin.jl#752 (comment).

timholy · 2021-01-09T12:37:17Z

I don't want to derail this from its original purpose, so please see #38983 (comment).

Right now aggressive constprop is essentially tied to the inlining threshold (or to their name being `getproperty` or `setproperty!` respectively, which can be both somewhat brittle if the inlining cost changes and insufficient when you do really know that const prop would be beneficial even if the function is not inlineable. This adds a simple macro that can be used to manually annotate methods to force aggressive constprop on them.

Keno requested review from vtjnash and JeffBezanson October 17, 2020 22:07

timholy reviewed Oct 18, 2020

View reviewed changes

base/expr.jl Outdated Show resolved Hide resolved

test/core.jl Outdated Show resolved Hide resolved

timholy mentioned this pull request Oct 18, 2020

cleanups and inference improvements in makielayout JuliaPlots/AbstractPlotting.jl#520

Merged

Keno force-pushed the kf/aggressive_constprop branch from 11fcf8b to 794883c Compare October 19, 2020 00:23

JeffBezanson reviewed Oct 20, 2020

View reviewed changes

Keno force-pushed the kf/aggressive_constprop branch from 794883c to ffcbab1 Compare December 17, 2020 08:47

Keno assigned JeffBezanson Dec 25, 2020

JeffBezanson approved these changes Jan 5, 2021

View reviewed changes

vtjnash reviewed Jan 5, 2021

View reviewed changes

timholy mentioned this pull request Jan 5, 2021

precompile interacts badly with Const-specialization #38983

Closed

Keno force-pushed the kf/aggressive_constprop branch 3 times, most recently from f62c2ef to e077e86 Compare January 22, 2021 17:14

Keno force-pushed the kf/aggressive_constprop branch from e077e86 to 20ad41c Compare January 22, 2021 17:43

Keno closed this Jan 23, 2021

Keno reopened this Jan 23, 2021

Keno closed this Jan 25, 2021

Keno reopened this Jan 25, 2021

Keno merged commit 3ea80fb into master Jan 26, 2021

Keno deleted the kf/aggressive_constprop branch January 26, 2021 00:46

chriselrod mentioned this pull request Jan 26, 2021

Reducing need for unique generated methods JuliaArrays/ArrayInterface.jl#111

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a macro to opt into aggressive constprop #38080

Add a macro to opt into aggressive constprop #38080

Keno commented Oct 17, 2020 •

edited

Loading

timholy left a comment

JeffBezanson Oct 20, 2020

JeffBezanson Oct 20, 2020

Keno Oct 20, 2020

Keno commented Dec 17, 2020

vtjnash commented Dec 17, 2020

Keno commented Dec 17, 2020

JeffBezanson commented Jan 5, 2021

vtjnash Jan 5, 2021

vtjnash Jan 5, 2021

timholy commented Jan 5, 2021 •

edited

Loading

JeffBezanson commented Jan 5, 2021

vtjnash commented Jan 5, 2021

timholy commented Jan 6, 2021 •

edited

Loading

timholy commented Jan 9, 2021

Add a macro to opt into aggressive constprop #38080

Add a macro to opt into aggressive constprop #38080

Conversation

Keno commented Oct 17, 2020 • edited Loading

timholy left a comment

Choose a reason for hiding this comment

JeffBezanson Oct 20, 2020

Choose a reason for hiding this comment

JeffBezanson Oct 20, 2020

Choose a reason for hiding this comment

Keno Oct 20, 2020

Choose a reason for hiding this comment

Keno commented Dec 17, 2020

vtjnash commented Dec 17, 2020

Keno commented Dec 17, 2020

JeffBezanson commented Jan 5, 2021

vtjnash Jan 5, 2021

Choose a reason for hiding this comment

vtjnash Jan 5, 2021

Choose a reason for hiding this comment

timholy commented Jan 5, 2021 • edited Loading

JeffBezanson commented Jan 5, 2021

vtjnash commented Jan 5, 2021

timholy commented Jan 6, 2021 • edited Loading

timholy commented Jan 9, 2021

Keno commented Oct 17, 2020 •

edited

Loading

timholy commented Jan 5, 2021 •

edited

Loading

timholy commented Jan 6, 2021 •

edited

Loading