Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getindex for BitArrays does not get inlined. #9077

Closed
lendle opened this issue Nov 20, 2014 · 6 comments
Closed

getindex for BitArrays does not get inlined. #9077

lendle opened this issue Nov 20, 2014 · 6 comments
Labels
performance Must go faster

Comments

@lendle
Copy link
Contributor

lendle commented Nov 20, 2014

julia> testgetindex(v) = (@inbounds x = v[1]; x)
testgetindex (generic function with 1 method)

julia> code_llvm(testgetindex, (BitVector,))

define i1 @"julia_testgetindex;41783"(%jl_value_t*) {
top:
  %1 = call i1 @julia_getindex2385(%jl_value_t* %0, i64 1), !dbg !763
  ret i1 %1, !dbg !763
}

I'm surprised that I wasn't able to find an open issue about this, so hopefully this isn't a dupe.
I'm guessing this is due to the bounds check? I don't fully understand how @inbounds works but I would guess this check does not go away when @inbounds is used.

@ivarne
Copy link
Member

ivarne commented Nov 20, 2014

Currently @inbounds only work with native Arrays, and there is an open issue about that in #7799. That issue doesn't say anything about using it for any of the array wrapping types, so it is not exactly a dupe, but very close.

@ivarne ivarne added the performance Must go faster label Nov 20, 2014
@JeffBezanson
Copy link
Member

If you need a quick workaround, there seems to be a Base.unsafe_bitgetindex. You could also try adding @inline to getindex at bitarray.jl:348.

@vtjnash
Copy link
Member

vtjnash commented Nov 21, 2014

perhaps what we really want is an @unlikely annotation that can be put onto if statements.

if @unlikely i > length(a)
  throw(error())
else
  return a[i]
end

inline_worthy could put less weight onto statements inside an @unlikely metadata annotation, and we can also pass this information directly to llvm (iirc)

@jiahao
Copy link
Member

jiahao commented Nov 21, 2014

I would use @unlikely.

@StefanKarpinski
Copy link
Member

The corresponding @likely would also be useful, of course.

@carlobaldassi
Copy link
Member

I just added an @inline annotation to the linear-indexing getindex method and the difference is quite noticeable: consider a BitArray(10^8) and a function

function testgetindex(v)
    s = 0
    for i = 1:length(v)
        s += v[i]
    end
    s
end

then the before/after timing on my laptop is 0.37s vs 0.12s, a factor of 3.
For the record, using Array{Bool} the timing is 0.055s, another factor of 2.

If you need a quick workaround, there seems to be a Base.unsafe_bitgetindex.

Actually Base.unsafe_getindex is more similar to the usual getindex interface, and can be used with Arrays too, so I'd suggest that until #7799 is fixed.

In the example above, the timings become 0.91s for BitArrays vs 0.35s for Arrays.

This is not to say that a @likely/@unlikely macro pair wouldn't be useful of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster
Projects
None yet
Development

No branches or pull requests

7 participants