Use `@inbounds` annotations for `Dict` implementation #23843

andyferris · 2017-09-23T13:30:02Z

It seems to me that Dicts can be used in performance sensitive code, and ought to be as fast as possible.

I made liberty of @propagate_inbounds in order to factor out the "chain of custody" (so to speak) of the correctness of the indices.

EDIT: See microbenchmark below (I didn't see anything in BaseBenchmarks.jl...)

It seems to me that `Dict`s can be used in performance sensitive code, and ought to be as fast as possible. I made liberty of `@propagate_inbounds` in order to factor out the "chain of custody" (so to speak) of the indices.

KristofferC · 2017-09-23T13:46:11Z

Any microbenchmarks to show?

andyferris · 2017-09-23T13:47:40Z

Give me one minute to recompile master. I did one on v0.6 that took 1.3 seconds and this branch takes 0.95 seconds... I'll post an update soon.

andyferris · 2017-09-23T13:53:14Z

Here's a function that does some general dict-y operations. I heard BenchmakTools isn't working (didn't try) so I rolled my own:

julia> function f(n)
                  keys = 1:n
                  vals1 = 2*(1:n)
                  vals2 = 3*(1:n)
                  
                  dict = Dict{Int,Int}()
                  for k in keys
                      @inbounds dict[k] = vals1[k]
                  end
                  for k in keys
                      @inbounds dict[k] = dict[k] + vals2[k]
                  end
                  out = 0
                  for k in keys
                      out += dict[k]
                  end
                  return out
              end
f (generic function with 1 method)

julia> @noinline g(n) = f(n)
g (generic function with 1 method)

julia> h(n) = for i in 1:10000; g(n); end
h (generic function with 1 method)

On this branch (after warmup):

julia> @time h(1000)
  0.944691 seconds (180.00 k allocations: 901.795 MiB, 2.59% gc time)

julia> @time h(1000)
  0.951921 seconds (180.00 k allocations: 901.795 MiB, 2.60% gc time)

julia> @time h(1000)
  0.949090 seconds (180.00 k allocations: 901.795 MiB, 2.58% gc time)

On master

julia> @time h(1000)
  1.052432 seconds (180.00 k allocations: 901.795 MiB, 2.16% gc time)

julia> @time h(1000)
  1.051774 seconds (180.00 k allocations: 901.795 MiB, 1.85% gc time)

julia> @time h(1000)
  1.065635 seconds (180.00 k allocations: 901.795 MiB, 1.84% gc time)

On v0.6

julia> @time h(1000)
  1.296142 seconds (180.00 k allocations: 901.795 MiB, 1.35% gc time)

julia> @time h(1000)
  1.288516 seconds (180.00 k allocations: 901.795 MiB, 1.33% gc time)

julia> @time h(1000)
  1.298345 seconds (180.00 k allocations: 901.795 MiB, 1.36% gc time)

So this PR represents a 10% improvement on this benchmark (with another 25% since v0.6... nice).

JeffBezanson · 2017-09-24T16:10:21Z

base/dict.jl

@@ -713,13 +713,19 @@ function start(t::Dict)
    return i
 end
 done(t::Dict, i) = i > length(t.vals)
-next(t::Dict{K,V}, i) where {K,V} = (Pair{K,V}(t.keys[i],t.vals[i]), skip_deleted(t,i+1))
+function next(t::Dict{K,V}, i) where {K,V}
+    @inbounds return (Pair{K,V}(t.keys[i],t.vals[i]), skip_deleted(t,i+1))


I don't think this is safe --- it's possible some user code could pass a bad value of i by mistake.

These could be marked @propagate_inbounds, though; that seems to be what arrays do.

Good point!

I'm assuming that what is now here makes sense? And that for loops expand/lower to include @inbounds next(...) for fast iteration over e.g. Array?

No, for loop cannot be lowered to @inbounds. It only guarantee that the iterator protocol is properly executed, but not inbounds access in next.

Umm... so are you telling me that for x in vector isn't optimally fast? It really spends time checking bounds, immediately after it has verified done?

Oh dear, I see #15291 wasn't merged... (I figured it was merged and we've moved onto something simpler, but alas)

Thanks @yuyichao for clearing that up. At least with @propagate_inbounds here, users have the option of @inbounds for kv in dict for a speedup, right?

Also, I'll have to update my benchmark.

JeffBezanson · 2017-09-24T16:11:47Z

base/dict.jl

-next(v::KeyIterator{<:Dict}, i) = (v.dict.keys[i], skip_deleted(v.dict,i+1))
-next(v::ValueIterator{<:Dict}, i) = (v.dict.vals[i], skip_deleted(v.dict,i+1))
+function next(v::KeyIterator{<:Dict}, i)
+    @inbounds return (v.dict.keys[i], skip_deleted(v.dict,i+1))


andyferris · 2017-10-02T12:33:15Z

Is it normal for CircleCI 64 bit to give ProcessExitedExceptions?

fredrikekre · 2017-10-02T12:41:15Z

Is it normal for CircleCI 64 bit to give ProcessExitedExceptions?

Happens somewhat regularly.

rfourquet · 2017-10-02T12:42:07Z

I didn't see anything in BaseBenchmarks.jl

cf. JuliaCI/BaseBenchmarks.jl#109, which benchmarks some operations with Dict, but I need some help for fixing one test before it can be merged.

andyferris · 2017-10-02T13:28:53Z

OK, I'm sad we've regressed to stochastic testing... but I guess I should merge this then.

Use @inbounds annotations for Dict implementation

cc33a40

It seems to me that `Dict`s can be used in performance sensitive code, and ought to be as fast as possible. I made liberty of `@propagate_inbounds` in order to factor out the "chain of custody" (so to speak) of the indices.

JeffBezanson reviewed Sep 24, 2017

View reviewed changes

Use @propagate_inbounds instead of @inbounds for next

7e9aca3

andyferris merged commit f2fd1f8 into master Oct 2, 2017

fredrikekre deleted the ajf/faster-dict branch October 2, 2017 13:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `@inbounds` annotations for `Dict` implementation #23843

Use `@inbounds` annotations for `Dict` implementation #23843

andyferris commented Sep 23, 2017 •

edited

Loading

KristofferC commented Sep 23, 2017

andyferris commented Sep 23, 2017

andyferris commented Sep 23, 2017

JeffBezanson Sep 24, 2017

JeffBezanson Sep 24, 2017

andyferris Sep 25, 2017

yuyichao Sep 25, 2017

andyferris Sep 25, 2017

yuyichao Sep 25, 2017

andyferris Sep 25, 2017 •

edited

Loading

yuyichao Sep 25, 2017

andyferris Sep 25, 2017

andyferris Sep 25, 2017 •

edited

Loading

JeffBezanson Sep 24, 2017

andyferris commented Oct 2, 2017

fredrikekre commented Oct 2, 2017

rfourquet commented Oct 2, 2017

andyferris commented Oct 2, 2017

Use @inbounds annotations for Dict implementation #23843

Use @inbounds annotations for Dict implementation #23843

Conversation

andyferris commented Sep 23, 2017 • edited Loading

KristofferC commented Sep 23, 2017

andyferris commented Sep 23, 2017

andyferris commented Sep 23, 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

Choose a reason for hiding this comment

andyferris Sep 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyferris Sep 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyferris commented Oct 2, 2017

fredrikekre commented Oct 2, 2017

rfourquet commented Oct 2, 2017

andyferris commented Oct 2, 2017

Use `@inbounds` annotations for `Dict` implementation #23843

Use `@inbounds` annotations for `Dict` implementation #23843

andyferris commented Sep 23, 2017 •

edited

Loading

andyferris Sep 25, 2017 •

edited

Loading

andyferris Sep 25, 2017 •

edited

Loading