I heard you like inferring types but don't like global state... #21677

jrevels · 2017-05-02T19:19:08Z

...so @vtjnash and I removed the work queue by refactoring inference to run recursively.

I'm not sure yet how far this will make it through CI. @vtjnash will probably be able to answer any algorithmic questions better than myself - this is my first foray into the dangerous world of type inference. I mainly sat there like a monkey poking at the keyboard.

Besides reducing dependence on global state, this PR provides the following goodies:

exactly precise cycle detection
more optimal inference state provided to inlining pass
Jameson says it might be faster (still need to time this)

This PR enables the following future work:

a more optimal inlining order
effect-free type inference
more optimal/reliable inference recursion detection rules (will allow automatic effect-free computation and constant propagation)

@vtjnash is sad that he has to write a new blog post now.

JeffBezanson · 2017-05-02T19:23:21Z

Cool!

Does this do anything to avoid using a very large amount of stack space?

JeffBezanson · 2017-05-02T19:25:48Z

base/inference.jl

-const nactive = Array{Int,0}()
-nactive[] = 0
-const workq = Vector{InferenceState}() # set of InferenceState objects that can make immediate progress
+# None! There is none! :)


JeffBezanson · 2017-05-02T19:27:10Z

base/inference.jl

-            end
-        end
-
-        # limit argument type size growth
        if mightlimitlength || mightlimitdepth
            # TODO: FIXME: this heuristic depends on non-local state making type-inference unpredictable


Does this change fix this issue?

Unfortunately not, but this PR does enable a code rewrite to fix the issue. This is also relevant to your other question:

Does this do anything to avoid using a very large amount of stack space?

In short, nope. @vtjnash believes that the future rewrite of the call recursion limit code here will help with stack space usage.

JeffBezanson · 2017-05-02T19:29:11Z

base/inference.jl

+    return frame
+end
+
+function in_egal(item, collection)


Already exists in this file; called contains_is.

martinholters · 2017-05-03T06:45:32Z

Looks like

julia/test/staged.jl

Line 150 in 970a742

    
           @test Core.Inference.isempty(Core.Inference.active) && Core.Inference.isempty(Core.Inference.workq)

is the only problem hit by CI? That would be pretty awesome!

jrevels · 2017-05-03T14:34:08Z

Looks like

julia/test/staged.jl

Line 150 in 970a742

@test Core.Inference.isempty(Core.Inference.active) && Core.Inference.isempty(Core.Inference.workq)

is the only problem hit by CI? That would be pretty awesome!

That would be crazy! I removed the now-invalid reference to the work queue (and kept the rest of that tiny module), so I guess we'll see what happens.

Let's see if this can make it through a whole BaseBenchmarks run:

@nanosoldier runbenchmarks(ALL, vs = ":master")

nanosoldier · 2017-05-03T18:46:29Z

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

martinholters · 2017-05-03T19:02:42Z

This seems to go so smooth, it's almost scary.

jrevels · 2017-05-03T19:08:43Z

Could anybody hazard a guess as to what's causing the Base.Test.TestSetException on 32-bit Windows?

vtjnash · 2017-05-03T19:50:53Z

It seems like something is failing to drop a gc-reference (https://ci.appveyor.com/project/JuliaLang/julia/build/1.0.13610/job/iex595jdaqb6w9og#L2826)

…e caller merging

…the loop)

…queue

JeffBezanson · 2017-05-12T21:03:46Z

base/inference.jl

@@ -296,10 +305,7 @@ end

 #### current global inference state ####


Seriously though, would be nice to just delete this section :)

JeffBezanson · 2017-05-12T21:13:47Z

base/inference.jl

+    end
+end
+
+function resolve_call_cycle!(linfo::MethodInstance, parent::InferenceState)


This function could use a comment. Not obvious to me what resolve means in this context.

tkelman · 2017-05-13T00:46:26Z

base/inference.jl

-                return nothing
+function merge_call_chain!(parent::InferenceState, ancestor::InferenceState, child::InferenceState)
+    # add backedge of parent <- child
+    # then add all backeges of parent <- parent.parent


tkelman · 2017-05-13T00:49:13Z

base/inference.jl

-        src = ccall(:jl_uncompress_ast, Any, (Any, Any), li.def, li.def.source)
+function InferenceState(linfo::MethodInstance,
+                        optimize::Bool, cached::Bool, params::InferenceParams)
+    # prepare an InferenceState object for infering lambda


tkelman · 2017-05-13T00:56:45Z

base/sysimg.jl

@@ -73,6 +72,9 @@ include("refpointer.jl")
 include("checked.jl")
 importall .Checked

+# buggy handling of ispure in type-inference means this should be after re-defining the basic operations that they might try to call


line wrap comment

tkelman · 2017-05-13T06:18:02Z

We could still use devdocs explaining what a backedge is

(general comment and lingering todo from the last rewrite, not necessary for this pr to solve)

jrevels · 2017-05-13T14:42:15Z

Made the requested clean-ups and added a comment describing resolve_call_cycle (@vtjnash might want to check that my explanation is correct/complete).

We could still use devdocs explaining what a backedge is

AFAIK, a backedge is a reference from one frame to another (within a call graph) that facilitates the reverse-propagation of type information. I'll leave the actual documentation to Jameson, though I imagine that could be done in a separate PR.

vtjnash · 2017-05-15T18:16:10Z

We could still use devdocs explaining what a backedge is

There's several different types. An edge is simply a function call. A backedge is that same thing, backwards. At some point, I'll get Jeff to sit down for a couple days with me (and anyone else who's interested) and try to figure out what they are.

vtjnash · 2017-05-15T20:17:06Z

https://juliacomputing.com/blog/2017/05/15/inference-converage2.html

andyferris · 2017-06-11T03:18:04Z

Will this one still make it into v0.6-final? (it fixes some problems I'm having with intermittent (i.e. unit tests failing @inferred while @code_warntype works OK at REPL) inference on rc3).

tkelman · 2017-06-11T17:17:19Z

Can you contribute base tests that were fixed by this?

removes the global work queue, which allows increasing the precision of cycle detection and resolution, and decreases the need for the threading synchronization lock surrounding inference updates `inInference` flag usage to be merely a hint for `jl_type_infer` to not bother trying to infer a method (helps avoid accidental infinite recursion over inferring type inference), enable inferring inference ref #21677 (cherry picked from commit 5847317)

removes the global work queue, which allows increasing the precision of cycle detection and resolution, and decreases the need for the threading synchronization lock surrounding inference updates `inInference` flag usage to be merely a hint for `jl_type_infer` to not bother trying to infer a method (helps avoid accidental infinite recursion over inferring type inference), enable inferring inference (cherry picked from commit 5847317)

JeffBezanson reviewed May 2, 2017

View reviewed changes

kshyatt added the compiler:inference Type inference label May 2, 2017

JeffBezanson reviewed May 2, 2017

View reviewed changes

jrevels force-pushed the jr/types branch 2 times, most recently from ad3534e to c260e1c Compare May 2, 2017 20:10

Sacha0 mentioned this pull request May 3, 2017

Make reshape(::BitArray,...) type stable #21670

Merged

vtjnash force-pushed the jr/types branch from 78c599b to edb2a70 Compare May 8, 2017 21:17

jrevels and others added 15 commits May 9, 2017 12:38

first steps towards recursive type inference

616e8bd

fill in mocked implementation of cyclic frame retrieval + intermediat…

8335504

…e caller merging

remove dead code

41d5d4f

fix add_backedge

d789442

remove unused global_sv

0089f54

reimplement typeinf in the new recursive world

d070885

clean up old variable usage

c597eaf

add back in age convergence logic

eeca667

fix those nasty nasty bugs

60b13fa

correction to merge_call_chain algo (missing initial edge that makes …

0bc7fe1

…the loop)

this is necessary

cd6f29f

callers --> callers_in_cycle

cf8e7d1

restore heuristic limiting

4ef1176

in_egal --> contains_is

b59c578

remove test checking the state of the now-nonexistant inference work …

e02e6be

…queue

JeffBezanson reviewed May 12, 2017

View reviewed changes

JeffBezanson approved these changes May 12, 2017

View reviewed changes

tkelman reviewed May 13, 2017

View reviewed changes

jrevels added 3 commits May 13, 2017 10:13

remove global state section

9363004

add explanation of resolve_call_cycle function

7633148

fix typos and comment length

bc90761

vtjnash merged commit 5847317 into master May 15, 2017

martinholters deleted the jr/types branch May 15, 2017 18:29

vtjnash added the backport pending 0.6 label May 22, 2017

This was referenced Jun 8, 2017

Inference results lost after multiple codegen invocations #22290

Closed

Fix inInference state corruption. #22313

Merged

This was referenced Jun 9, 2017

Unstable vararg inlining #20668

Closed

return_types() confused after running many tests #21740

Closed

tkelman mentioned this pull request Jun 13, 2017

Inference disabled by a world age issue (0.6.0-rc3) #22355

Closed

ararslan removed the backport pending 0.6 label Oct 7, 2017

maleadt mentioned this pull request Oct 27, 2017

Tests fail on 0.6.1 JuliaGPU/CUDAnative.jl#124

Closed

JeffBezanson mentioned this pull request Oct 28, 2017

20+ minute precompilation regression on v0.6.1 #24383

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

I heard you like inferring types but don't like global state... #21677

I heard you like inferring types but don't like global state... #21677

jrevels commented May 2, 2017

JeffBezanson commented May 2, 2017

JeffBezanson May 2, 2017

JeffBezanson May 2, 2017

jrevels May 2, 2017

JeffBezanson May 2, 2017

martinholters commented May 3, 2017

jrevels commented May 3, 2017

nanosoldier commented May 3, 2017

martinholters commented May 3, 2017

jrevels commented May 3, 2017

vtjnash commented May 3, 2017

JeffBezanson May 12, 2017

JeffBezanson May 12, 2017

tkelman May 13, 2017

tkelman May 13, 2017

tkelman May 13, 2017

tkelman commented May 13, 2017 •

edited

Loading

jrevels commented May 13, 2017

vtjnash commented May 15, 2017

vtjnash commented May 15, 2017

andyferris commented Jun 11, 2017

tkelman commented Jun 11, 2017

		@@ -296,10 +305,7 @@ end

		#### current global inference state ####

I heard you like inferring types but don't like global state... #21677

I heard you like inferring types but don't like global state... #21677

Conversation

jrevels commented May 2, 2017

JeffBezanson commented May 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinholters commented May 3, 2017

jrevels commented May 3, 2017

nanosoldier commented May 3, 2017

martinholters commented May 3, 2017

jrevels commented May 3, 2017

vtjnash commented May 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman commented May 13, 2017 • edited Loading

jrevels commented May 13, 2017

vtjnash commented May 15, 2017

vtjnash commented May 15, 2017

andyferris commented Jun 11, 2017

tkelman commented Jun 11, 2017

tkelman commented May 13, 2017 •

edited

Loading