From d98544db70729a8c79c1e88c7ecd66bc2c52a07e Mon Sep 17 00:00:00 2001 From: Jacob Quinn Date: Thu, 15 Aug 2019 13:25:25 -0600 Subject: [PATCH 1/2] For structs with all isbits or isbitsunion fields, allow them to be stored inline in arrays --- base/array.jl | 30 +++++++++++++++--------------- base/arraymath.jl | 2 +- src/datatype.c | 10 ++++++---- src/julia.h | 1 + test/core.jl | 27 +++++++++++++++++++++++---- 5 files changed, 46 insertions(+), 24 deletions(-) diff --git a/base/array.jl b/base/array.jl index 1811e44f9e376..cb5697972f981 100644 --- a/base/array.jl +++ b/base/array.jl @@ -158,6 +158,8 @@ size(a::Array{<:Any,N}) where {N} = (@_inline_meta; ntuple(M -> size(a, M), Val( asize_from(a::Array, n) = n > ndims(a) ? () : (arraysize(a,n), asize_from(a, n+1)...) +allocatedinline(::Type{T}) where {T} = (@_pure_meta; ccall(:jl_array_store_unboxed, Cint, (Any,), T) != Cint(0)) + """ Base.isbitsunion(::Type{T}) @@ -172,14 +174,12 @@ julia> Base.isbitsunion(Union{Float64, String}) false ``` """ -isbitsunion(u::Union) = (@_pure_meta; ccall(:jl_array_store_unboxed, Cint, (Any,), u) != Cint(0)) +isbitsunion(u::Union) = allocatedinline(u) isbitsunion(x) = false -isptrelement(t::Type) = (@_pure_meta; ccall(:jl_array_store_unboxed, Cint, (Any,), t) == Cint(0)) - function _unsetindex!(A::Array{T}, i::Int) where {T} @boundscheck checkbounds(A, i) - if isptrelement(T) + if !allocatedinline(T) t = @_gc_preserve_begin A p = Ptr{Ptr{Cvoid}}(pointer(A)) unsafe_store!(p, C_NULL, i) @@ -212,7 +212,7 @@ function bitsunionsize(u::Union) end length(a::Array) = arraylen(a) -elsize(::Type{<:Array{T}}) where {T} = isbitstype(T) ? sizeof(T) : (isbitsunion(T) ? bitsunionsize(T) : sizeof(Ptr)) +elsize(::Type{<:Array{T}}) where {T} = isbitsunion(T) ? bitsunionsize(T) : (allocatedinline(T) ? sizeof(T) : sizeof(Ptr)) sizeof(a::Array) = Core.sizeof(a) function isassigned(a::Array, i::Int...) @@ -255,9 +255,7 @@ the same manner as C. function unsafe_copyto!(dest::Array{T}, doffs, src::Array{T}, soffs, n) where T t1 = @_gc_preserve_begin dest t2 = @_gc_preserve_begin src - if isbitstype(T) - unsafe_copyto!(pointer(dest, doffs), pointer(src, soffs), n) - elseif isbitsunion(T) + if isbitsunion(T) ccall(:memmove, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, UInt), pointer(dest, doffs), pointer(src, soffs), n * Base.bitsunionsize(T)) # copy selector bytes @@ -265,6 +263,8 @@ function unsafe_copyto!(dest::Array{T}, doffs, src::Array{T}, soffs, n) where T ccall(:jl_array_typetagdata, Ptr{UInt8}, (Any,), dest) + doffs - 1, ccall(:jl_array_typetagdata, Ptr{UInt8}, (Any,), src) + soffs - 1, n) + elseif allocatedinline(T) + unsafe_copyto!(pointer(dest, doffs), pointer(src, soffs), n) else ccall(:jl_array_ptr_copy, Cvoid, (Any, Ptr{Cvoid}, Any, Ptr{Cvoid}, Int), dest, pointer(dest, doffs), src, pointer(src, soffs), n) @@ -1547,11 +1547,11 @@ function vcat(arrays::Vector{T}...) where T end arr = Vector{T}(undef, n) ptr = pointer(arr) - if isbitstype(T) - elsz = Core.sizeof(T) - elseif isbitsunion(T) + if isbitsunion(T) elsz = bitsunionsize(T) selptr = ccall(:jl_array_typetagdata, Ptr{UInt8}, (Any,), arr) + elseif allocatedinline(T) + elsz = Core.sizeof(T) else elsz = Core.sizeof(Ptr{Cvoid}) end @@ -1559,16 +1559,16 @@ function vcat(arrays::Vector{T}...) where T for a in arrays na = length(a) nba = na * elsz - if isbitstype(T) - ccall(:memcpy, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, UInt), - ptr, a, nba) - elseif isbitsunion(T) + if isbitsunion(T) ccall(:memcpy, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, UInt), ptr, a, nba) # copy selector bytes ccall(:memcpy, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, UInt), selptr, ccall(:jl_array_typetagdata, Ptr{UInt8}, (Any,), a), na) selptr += na + elseif allocatedinline(T) + ccall(:memcpy, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, UInt), + ptr, a, nba) else ccall(:jl_array_ptr_copy, Cvoid, (Any, Ptr{Cvoid}, Any, Ptr{Cvoid}, Int), arr, ptr, a, pointer(a), na) diff --git a/base/arraymath.jl b/base/arraymath.jl index 7603f4d54ee65..233490804446d 100644 --- a/base/arraymath.jl +++ b/base/arraymath.jl @@ -94,7 +94,7 @@ function reverse(A::Array{T}; dims::Integer) where T end end else - if isbitstype(T) && M>200 + if allocatedinline(T) && M>200 for i = 1:sd ri = sd+1-i for j=0:stride:(N-stride) diff --git a/src/datatype.c b/src/datatype.c index aa87efef9102b..9ea0e9fcaa054 100644 --- a/src/datatype.c +++ b/src/datatype.c @@ -248,7 +248,7 @@ static unsigned union_isbits(jl_value_t *ty, size_t *nbytes, size_t *align) JL_N return 0; return na + nb; } - if (jl_isbits(ty)) { + if (jl_is_datatype(ty) && jl_datatype_isinlinealloc(ty)) { size_t sz = jl_datatype_size(ty); size_t al = jl_datatype_align(ty); if (*nbytes < sz) @@ -292,6 +292,7 @@ static int references_name(jl_value_t *p, jl_typename_t *name) JL_NOTSAFEPOINT void jl_compute_field_offsets(jl_datatype_t *st) { size_t sz = 0, alignm = 1; + size_t fldsz = 0, fldal = 0; int homogeneous = 1; jl_value_t *lastty = NULL; uint64_t max_offset = (((uint64_t)1) << 32) - 1; @@ -302,12 +303,14 @@ void jl_compute_field_offsets(jl_datatype_t *st) // compute whether this type can be inlined // based on whether its definition is self-referential if (w->types != NULL) { - st->isbitstype = st->isconcretetype && !st->mutabl; + st->isbitstype = st->isinlinealloc = st->isconcretetype && !st->mutabl; size_t i, nf = jl_svec_len(st->types); for (i = 0; i < nf; i++) { jl_value_t *fld = jl_svecref(st->types, i); if (st->isbitstype) st->isbitstype = jl_is_datatype(fld) && ((jl_datatype_t*)fld)->isbitstype; + if (st->isinlinealloc) + st->isinlinealloc = (jl_is_datatype(fld) && ((jl_datatype_t*)fld)->isbitstype) || jl_islayout_inline(fld, &fldsz, &fldal); if (!st->zeroinit) st->zeroinit = (jl_is_datatype(fld) && ((jl_datatype_t*)fld)->isinlinealloc) ? ((jl_datatype_t*)fld)->zeroinit : 1; if (i < st->ninitialized) { @@ -317,8 +320,7 @@ void jl_compute_field_offsets(jl_datatype_t *st) st->has_concrete_subtype &= !jl_is_datatype(fld) || ((jl_datatype_t *)fld)->has_concrete_subtype; } } - if (st->isbitstype) { - st->isinlinealloc = 1; + if (st->isinlinealloc) { size_t i, nf = jl_svec_len(w->types); for (i = 0; i < nf; i++) { jl_value_t *fld = jl_svecref(w->types, i); diff --git a/src/julia.h b/src/julia.h index 4054f49ba733b..64b7827db539b 100644 --- a/src/julia.h +++ b/src/julia.h @@ -929,6 +929,7 @@ STATIC_INLINE jl_value_t *jl_field_type_concrete(jl_datatype_t *st JL_PROPAGATES #define jl_datatype_align(t) (((jl_datatype_t*)t)->layout->alignment) #define jl_datatype_nbits(t) ((((jl_datatype_t*)t)->size)*8) #define jl_datatype_nfields(t) (((jl_datatype_t*)(t))->layout->nfields) +#define jl_datatype_isinlinealloc(t) (((jl_datatype_t *)(t))->isinlinealloc) // inline version with strong type check to detect typos in a `->name` chain STATIC_INLINE char *jl_symbol_name_(jl_sym_t *s) JL_NOTSAFEPOINT diff --git a/test/core.jl b/test/core.jl index 5d653ffec90b4..101803248f2dd 100644 --- a/test/core.jl +++ b/test/core.jl @@ -5852,9 +5852,9 @@ let b3 = B23367[b][1] # copy b via array assignment addr(@nospecialize x) = ccall(:jl_value_ptr, Ptr{Cvoid}, (Any,), x) @test addr(b) == addr(b) - @test addr(b) == addr(b2) - @test addr(b) == addr(b3) - @test addr(b2) == addr(b3) + # @test addr(b) == addr(b2) + # @test addr(b) == addr(b3) + # @test addr(b2) == addr(b3) @test b === b2 === b3 === b @test egal(b, b2) && egal(b2, b3) && egal(b3, b) @@ -5863,7 +5863,7 @@ let @test b.x === Int8(91) @test b.z === Int8(23) @test b.y === A23367((Int8(1), Int8(2), Int8(3), Int8(4), Int8(5), Int8(6), Int8(7))) - @test sizeof(b) == sizeof(Int) * 3 + @test sizeof(b) == 12 @test A23367(Int8(1)).x === Int8(1) @test A23367(Int8(0)).x === Int8(0) @test A23367(Int16(1)).x === Int16(1) @@ -5891,6 +5891,25 @@ for U in boxedunions end end +struct UnionFieldInlineStruct + x::Int64 + y::Union{Float64, Missing} +end + +@test sizeof(Vector{UnionFieldInlineStruct}(undef, 2)) == sizeof(UnionFieldInlineStruct) * 2 + +let x = UnionFieldInlineStruct(1, 3.14) + AInlineUnion = [x for i = 1:10] + @test sizeof(AInlineUnion) == sizeof(UnionFieldInlineStruct) * 10 + BInlineUnion = Vector{UnionFieldInlineStruct}(undef, 10) + copyto!(BInlineUnion, AInlineUnion) + @test AInlineUnion == BInlineUnion + @test BInlineUnion[end] == x + CInlineUnion = vcat(AInlineUnion, BInlineUnion) + @test sizeof(CInlineUnion) == sizeof(UnionFieldInlineStruct) * 20 + @test CInlineUnion[end] == x +end + # issue 31583 a31583 = "a" f31583() = a31583 === "a" From cf4796c4e63847d20f24f3f044f970a32c5f113f Mon Sep 17 00:00:00 2001 From: Jacob Quinn Date: Thu, 15 Aug 2019 20:45:40 -0600 Subject: [PATCH 2/2] Add NEWS entry for inline isbits/isbitsunion structs --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index d404d2eb30491..4bb7cc0701c7a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,6 +7,7 @@ New language features * Support for Unicode 12.1.0 ([#32002]). * Methods can now be added to an abstract type ([#31916]). * Support for unicode bold-digits and double-struck digits 0 through 9 as valid identifiers ([#32838]). +* Structs with all isbits and isbitsunion fields are now stored inline in arrays ([#32448]). Language changes ----------------