move bounds checks to constructor

[robertbuessow]

I moved the bounds check from getindex to the Blobs constructors. We now guarantee that the constructed blob is valid (has enough memory for the data), so we don't need boundschecks whenever we deref. This gets rid of all the boundscheck code.

RAICode needs some fixes to work with this, i.e. where we created blobs with offset > limit. See: https://github.com/RelationalAI/raicode/pull/21062

[kuszmaul]

This is an interesting idea. I'm not sure whether I'm sold on it.

[kuszmaul]

This assert against the null pointer seems out of place. There's no way to protect against bad pointers, and may be the user wants to make such a blob.

[rgankema]

Maybe we should allow null pointers if their size is also set to 0?

[robertbuessow]

sounds good to me (I also remove the @Assert since it caused in alloc)

[kuszmaul]

This @propagate_bounds seems unneeded and possibly wrong. We know that there are no bounds errors here, right? Also, this is going to be a problem if self_size != sizeof so maybe it should be self_size rather than sizeof. Or maybe there should be an error.

Suggested change

	Base.@propagate_inbounds function Blob(ref::Base.RefValue{T}) where T
	Blob{T}(pointer_from_objref(ref), 0, sizeof(T))
	function Blob(ref::Base.RefValue{T}) where T
	@assert self_size(T) == sizeof(T)
	@inbounds Blob{T}(pointer_from_objref(ref), 0, self_size(T))

[robertbuessow]

That @Assert actually breaks the test. Basically this operation only works if the memory layout of Julia structs and Blobs are the same. That's not always the case because of alignment. The best would be to get rid of the constructor altogether. Otherwise, the @Assert is probably good enough, assuming that Julia only takes more memory due to alignment.

[kuszmaul]

It's traditional in the C world to allow people to construct a pointer that goes just off the end of an array. (But not to access that pointer)

This code prohibits the construction of the pointer. Is that where we want to go?

[robertbuessow]

You can created a Blob{Nothing}(<someptr>, 0, 0). That actually happening. I don't think a Blob is an array so don't see a need for something else.

[kuszmaul]

What's the rationale for making this @noinline? Julia doesn't do it for checkbounds(::AbstractArray, I...), for example.

[robertbuessow]

The reason for the large amount of generate llvm code was the bounds check (building the strings etc). Therefore, I want to avoid that this is getting inlined into the specialized function.

[kuszmaul]

Does this @nospecialize actually do anything?

Julia doesn't specialize arguments that look like t::Type. (It does specialize arguments like ::Type{T}) where T.)

julia> foo(base::Int64, t::DataType) = base+sizeof(t)
foo (generic function with 1 method)

julia> Base.specializations(@which foo(3,Int))
Base.MethodSpecializations(MethodInstance for foo(::Int64, ::DataType))

julia> Base.specializations(@which foo(3,UInt8))
Base.MethodSpecializations(MethodInstance for foo(::Int64, ::DataType))

julia> bar(base::Int64, ::Type{T}) where T = base+sizeof(T)
bar (generic function with 1 method)

julia> Base.specializations(@which bar(3,Int))
Base.MethodSpecializations(MethodInstance for bar(::Int64, ::Type{Int64}))

julia> Base.specializations(@which bar(3,UInt8))
Base.MethodSpecializations(svec(MethodInstance for bar(::Int64, ::Type{Int64}), MethodInstance for bar(::Int64, ::Type{UInt8}), nothing, nothing, nothing, nothing, nothing))

[robertbuessow]

No, good catch. I added it because I was surprised that there is still more code generated by the constructor (I believe). But it didn't change anything for the reason that you point out.