Check for :meta expressions more thoroughly, and improve indexing performance

[timholy]

This addresses #11595, and fixes most of our current problems with indexing performance. Sadly, it still doesn't recover the full performance compared to #5117 (comment). With this PR (running on a different machine, but the times relative to the first should be pretty comparable):

julia> include("/tmp/mykern.jl")
follow up: mykern( 2:(siz - 3), s, a, x, y )
  16.097 milliseconds (6 allocations: 192 bytes)
follow up: mykern( 2:(siz - 3), s1, a, x1, y1 )
  99.089 milliseconds (29670 allocations: 1216 KB)
follow up: mykern( 3:(siz - 2), s2, a, x2, y2 )
  44.005 milliseconds (6 allocations: 192 bytes)
  41.978 milliseconds (6 allocations: 192 bytes)
  41.984 milliseconds (6 allocations: 192 bytes)

which is a big improvement over the nearly thousand-fold penalty observed in #5117 (comment).

If we want SubArrays to have indexing performance nearly equivalent to Arrays, we probably have to go back to not doing bounds checking 😦.

[mbauman]

Awesome, thank you @timholy! And thank you to @tkelman and @yuyichao for digging into this with me over the weekend.

Theoretically, we're not doing more work than we were before (we still were checking bounds upon accessing the parent array), but there's a lot more indirection and we're definitely making the compiler work harder. I'll try to figure out if it's possible to regain some ground here.

Interesting (and wonderful) that this hasn't run into the win32 bug. I just pushed a branch to run on Appveyor to test this branch + the factor of 1000 inline_worthy patch. Even if that fails, we'll have a nice lead on a workaround for the bug without wrecking performance: some function may have to marked @noinline temporarily.

[mbauman]

Success! The only question I have about this implementation is if we really should look anywhere within a function for a meta annotation. When I sat down to do this this morning (before seeing your solution), I just made popmeta! try a little harder:

diff --git a/base/expr.jl b/base/expr.jl
index 49850aa..a8ebade 100644
--- a/base/expr.jl
+++ b/base/expr.jl
@@ -139,8 +139,13 @@ function pushmeta!(ex::Expr, sym::Symbol, args::Any...)
 end

 function popmeta!(body::Expr, sym::Symbol)
-    if isa(body.args[1],Expr) && (body.args[1]::Expr).head === :meta
-        metaargs = (body.args[1]::Expr).args
+    # Skip unrelated top-of-function annotations
+    i = 1;
+    while i < length(body.args) && isa(body.args[i],NewvarNode)
+        i+=1
+    end
+    if isa(body.args[i],Expr) && (body.args[i]::Expr).head === :meta
+        metaargs = (body.args[i]::Expr).args
         for i = 1:length(metaargs)
             if (isa(metaargs[i], Symbol) && metaargs[i] == sym) ||
                (isa(metaargs[i], Expr) && metaargs[i].head == sym)

My concern here is that we could start using :meta to annotate things inside the function, e.g., call-site inlining. But otherwise this looks great to me.

[timholy]

For building julia, I got this:

master (as of 6b3520628753443afbe32f57a25f90243bac2601, prior to ebfebfdc48919b767124a62958b9b6455552f3c9):
real    6m25.935s
user    6m22.982s
sys     0m4.278s

this diff on top of 6b3520628753443afbe32f57a25f90243bac2601:
real    6m15.928s
user    6m12.513s
sys     0m4.802s

Since I couldn't measure a performance hit, it seemed best to just check the entire expression (recursively, no less).

[timholy]

Regarding local annotation, I think we'd certainly need a different symbol, e.g., :inline_calls or something. Those wouldn't be stripped by popmeta!(ex, :inline).

[mbauman]

Ah, yes, of course. Makes sense. LGTM!

[timholy]

Thanks for reviewing.

Obviously the inline_worthy patch should also be merged.

[timholy]

I guess if we start inserting metadata throughout a function, the biggest concern is that we currently assume that there is at most one :meta expression.