JuliaArrays / ArrayViews.jl

A Julia package to explore a new system of array views
MIT License
19 stars 18 forks source link

`unsafe_aview` is allocating #54

Closed goretkin closed 6 years ago

goretkin commented 6 years ago

Or I'm measuring allocations incorrectly...

(On Julia 0.6.0 and after https://github.com/JuliaArrays/ArrayViews.jl/pull/53 and https://github.com/JuliaArrays/ArrayViews.jl/pull/51)

julia> using ArrayViews

julia> function doit(P)
                @allocated sum(unsafe_aview(P, 1, :))
              end
doit (generic function with 1 method)

julia> P = eye(10);

julia> doit(P)
548582

julia> doit(P)
224

julia> doit(P)
224

julia> function doit(P)
                @allocated sum(unsafe_aview(P, 1, :))
              end
doit (generic function with 1 method)

julia> doit(P)
160
goretkin commented 6 years ago

I was hoping that getting rid of the deprecations on 0.6.0 (https://github.com/JuliaArrays/ArrayViews.jl/pull/59) would fix this, but I'm getting the same results. @yuyichao am I measuring allocations correctly?

yuyichao commented 6 years ago

AFAICT these are expected to allocate.

goretkin commented 6 years ago

Really?

immutable UnsafeStridedView{T,N,M} <: UnsafeArrayView{T,N,M}
    ptr::Ptr{T}
    len::Int
    shp::NTuple{N,Int}
    strides::NTuple{N,Int}
end

and

julia> isbits(typeof(unsafe_aview(eye(1), 1, :)))
true

So I'm really hoping they can not allocate. Otherwise I don't see the point of having an Unsafe_ versions:

julia> function doit()
                       P = eye(10);
                       # safe ArrayView, expected to allocate
                       @allocated sum(aview(P, 1, :))
                     end
doit (generic function with 1 method)

julia> doit()
160
yuyichao commented 6 years ago

I did a simple searched and found one method that is properly defined and missed the actual unsafe one. It seems that the unsafe one should just be removed.

goretkin commented 6 years ago

No, I'm looking for an unsafe, non-allocating view. That's the whole point of having the unsafe option, so it should not be removed.

Is there some method that is improperly defined somewhere?

yuyichao commented 6 years ago

The whole definition and use of UnsafeStridedView.

goretkin commented 6 years ago

What is wrong the definition? If you can, I'd appreciate some more details in your responses.

yuyichao commented 6 years ago

That it doesn't keep a reference to the actual array.

goretkin commented 6 years ago

That is the design. That is the intention of the unsafe views. That is what should allow it to definitely not be heap-allocated.

yuyichao commented 6 years ago

And the design is completely wrong, should be removed and shouldn't be called unsafe but rather unsupported.

goretkin commented 6 years ago

From the perspective that non-allocating views are needed, the design isn't wrong, since the design aims to provide that functionality, which is otherwise missing in Julia so far.

The issue I opened is "why does unsafe_aviews" allocate, if you have an answer to that question, it would be extremely helpful. Why is there a memory allocation for constructing an immutable isbits type?

yuyichao commented 6 years ago

From the perspective that non-allocating views are needed

From the perspective of valid julia code, it's invalid.

yuyichao commented 6 years ago

if you have an answer to that question

So as I said, my only advice is to stop using it. I'm not going to provide any help for how to write invalid code.

goretkin commented 6 years ago

I guess I am not understanding your meaning of "valid julia code".

Is this invalid? :

access_a_pointer(p) = unsafe_load(p)
something(a) = access_a_pointer(pointer(a))
A = eye(2)
something(A)

I don't understand why "unsafeaviews" isn't equivalent to any of the other `unsafe` methods in Julia: use at your own risk.

Thanks for the help so far, even if it's to elucidate what your definition of "invalid code" is, so that I can definitely not write it.

yuyichao commented 6 years ago

Is this invalid? :

I mean it's not valid.

goretkin commented 6 years ago

Well that makes no sense to me! But I understand you might not be able to explain it to me.

I see that 0.6.0 Base does plenty of pointer accesses that look like the "invalid" Julia code above. More recently, it looks like @gc_preserve is used. Does that make it valid?

Oh, I see: https://github.com/JuliaLang/julia/commit/15ca594696b5823a9f7b7c32dad6b75b0699e320. So unsafe_views can be valid within a gc_preserve context must be valid, right?

yuyichao commented 6 years ago

it looks like @gc_preserve is used. Does that make it valid?

Yes.

Do unsafe_views can be valid within a gc_preserve context must be valid, right?

On 0.7, kind of.

JaredCrean2 commented 6 years ago

I don't see any allocations on Julia 0.6.2:

using ArrayViews

function doit(P)
  @allocated sum(unsafe_aview(P, 1, :))

end

P = rand(10, 10)

println("warm up = ", doit(P))
println("final  = ", doit(P))

Outputs:

warm up = 0
final  = 0