layout optimization internal changes (support pointers inlining/unboxing into parents/codegen) [disabled] #33886

vtjnash · 2019-11-18T21:21:39Z

This includes fairly general support for pointers inside immutable structs appearing in first class aggregates (FCA) throughout the system (e.g. gc, codegen, layout). Currently it's also pretty widely enabled, although we may want to disable it for the initial merge, then slowly enable larger pieces of it as we gain understanding of the effects on the ecosystem.

This replaces #18632. It solves the "fundamental problems" described in that PR with the following rules (these need to be added to the docs and NEWs):

If a struct declaration is self-referential (aka references_name), and thus couldn't qualify for isbits, it also doesn't qualify for inlining. This solves the problem with cycles.
It a struct declaration can be incompletely initialized (n_initialized != n_fields), it doesn't quality for inlining. This solves the problem of tracking #undef. This may be relaxed in the future by adding an extra byte to track this state, although I'm not particularly keen on doing the extra work to support this edge case.
Note that Union{T, S} will not inline ("union-split") if either T or S contains a pointer (wouldn't be legal to codegen this, and somewhat hard to describe to the gc)

I'm interested in folks thoughts on how they'd like to see this land. Should I merge the support for each component in a separate commit (without users until all is done), or just land this as one big commit but disabled? One PR or several? And specific pieces you'd like to see land first?

@nanosoldier runbenchmarks(ALL, vs=":master")

JeffBezanson · 2019-11-18T22:14:01Z

I would vote for merging all the code, with stack allocation of temporaries enabled, but new memory layouts disabled. That can be enabled in a separate commit, to make it easier to revert if it is too breaking.

andyferris · 2019-11-18T23:18:17Z

Very cool! :) I look forward to trying this out.

Can I ask - what are "first-class aggregates"? Would this include Arrays of structs containing managed pointers?

It a struct declaration can be incompletely initialized (n_initialized != n_fields), it doesn't quality for inlining. This solves the problem of tracking #undef

This is interesting. How do you know if a given struct might contain undef? Has it got to do with the existance of certain inner constructors? Is it tracked by instance? Why is this necessary? (couldn't the GC just check if the pointer within the struct is null? similarly for codegen inserting undefined reference errors wherever necessary?)

Note that Union{T, S} will not inline ("union-split") if either T or S contains a pointer (wouldn't be legal to codegen this, and somewhat hard to describe to the gc)

Just gonna say... this is going to pessimize some missing code (also nothing/something else style of code, which includes iteration). But I get that it's hard to implement.

nanosoldier · 2019-11-19T04:52:28Z

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

JeffBezanson · 2019-11-21T02:00:59Z

Just gonna say... this is going to pessimize some missing code (also nothing/something else style of code, which includes iteration).

IIUC, it shouldn't be worse than it is currently.

andyferris · 2019-11-21T02:56:52Z

IIUC, it shouldn't be worse than it is currently.

No, of course - I didn't mean to imply otherwise, but was just musing over common patterns like iteration and Union{Missing, T}.

c42f · 2019-11-21T06:56:45Z

How do you know if a given struct might contain undef? Has it got to do with the existance of certain inner constructors? Is it tracked by instance? Why is this necessary?

This is currently inferred during lowering by taking the minimum of the number of arguments to new() expressions within inner constructors. It is stored per data type in the DataType ninitialized field when the data type is created. It's used by codegen to avoid emitting checks for undef when we know the field is initialized (ie, when the field index is < ninitialized).

Includes codegen support for immutable objects that contain pointers appearing on stack (well, in registers, since LLVM support of non-integral addrspace pointers inside aggregates in memory is poor), and includes layout support, so that most (non-self-referential) immutable objects can be stored inline inside their parent allocation. Currently fully disabled, aside from some optimizations and improvements to object_id / isa tests. Co-Authored-By: Oscar Blumberg <[email protected]>

JeffBezanson · 2019-12-06T22:14:10Z

src/datatype.c

+ return jl_islayout_inline(eltype, &fsz, &al);
+}
+
+// whether this type is unique'd by pointer


Suggested change

// whether this type is unique'd by pointer

// whether instances of this type can use pointer comparison for `===`

andyferris · 2019-12-06T23:33:42Z

So... I seem to remember being able to put Expr(:new, ...) anywhere. (I think I made a new “inner” constructor to Dict once when experimenting... possibly outside of Base...). I’m assuming such an operation now becomes unsafe if it fills less slots than the “real” inner constructors?

vtjnash · 2019-12-07T00:18:30Z

Nah, that’s always been unsafe and inadvisable.

KristofferC · 2019-12-17T18:38:56Z

base/array.jl

@@ -177,14 +177,20 @@ false
 isbitsunion(u::Union) = allocatedinline(u)
 isbitsunion(x) = false

-function _unsetindex!(A::Array{T}, i::Int) where {T}
+@inbounds function _unsetindex!(A::Array{T}, i::Int) where {T}


What does @inbounds on a function do?

Supposed to be @inline.

See #33886 (comment)

Includes codegen support for immutable objects that contain pointers appearing on stack (well, in registers, since LLVM support of non-integral addrspace pointers inside aggregates in memory is poor), and includes layout support, so that most (non-self-referential) immutable objects can be stored inline inside their parent allocation. Currently fully disabled, aside from some optimizations and improvements to object_id / isa tests. Co-Authored-By: Oscar Blumberg <[email protected]>

vtjnash added needs docs Documentation for this change is required needs news A NEWS entry is required for this change labels Nov 18, 2019

vtjnash force-pushed the jn/codegen-ptrbits branch from 57ad49d to a301cf9 Compare November 18, 2019 22:09

vtjnash force-pushed the jn/codegen-ptrbits branch from a301cf9 to 149dc3a Compare November 22, 2019 03:47

vtjnash force-pushed the jn/codegen-ptrbits branch from 149dc3a to 5949e8c Compare December 2, 2019 18:34

vtjnash removed needs docs Documentation for this change is required needs news A NEWS entry is required for this change labels Dec 2, 2019

vtjnash changed the title ~~[WIP] layout optimization: support pointers inlining (unboxing) into parents~~ layout optimization internal changes (support pointers inlining/unboxing into parents/codegen) [disabled] Dec 2, 2019

JeffBezanson reviewed Dec 6, 2019

View reviewed changes

JeffBezanson added this to the 1.4 milestone Dec 12, 2019

JeffBezanson approved these changes Dec 17, 2019

View reviewed changes

JeffBezanson merged commit 630a551 into master Dec 17, 2019

JeffBezanson deleted the jn/codegen-ptrbits branch December 17, 2019 17:45

KristofferC reviewed Dec 17, 2019

View reviewed changes

ararslan added a commit that referenced this pull request Dec 17, 2019

Replace erroneous inbounds function annotation with inline

08345ab

See #33886 (comment)

ararslan mentioned this pull request Dec 17, 2019

Replace erroneous inbounds function annotation with inline #34127

Closed

mateuszbaran mentioned this pull request Dec 28, 2019

Investigate performance impact of the new layout in Julia 1.4 JuliaArrays/HybridArrays.jl#8

Closed

palday mentioned this pull request May 12, 2020

Failure in callback under Julia-1.5.0-DEV JuliaOpt/NLopt.jl#144

Closed

projekter mentioned this pull request Jun 2, 2024

QuadSort and BlitSort JuliaCollections/SortingAlgorithms.jl#95

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

layout optimization internal changes (support pointers inlining/unboxing into parents/codegen) [disabled] #33886

layout optimization internal changes (support pointers inlining/unboxing into parents/codegen) [disabled] #33886

vtjnash commented Nov 18, 2019

JeffBezanson commented Nov 18, 2019

andyferris commented Nov 18, 2019

nanosoldier commented Nov 19, 2019

JeffBezanson commented Nov 21, 2019

andyferris commented Nov 21, 2019

c42f commented Nov 21, 2019

JeffBezanson Dec 6, 2019

andyferris commented Dec 6, 2019

vtjnash commented Dec 7, 2019

KristofferC Dec 17, 2019

JeffBezanson Dec 17, 2019

ararslan Dec 17, 2019

	// whether this type is unique'd by pointer
	// whether instances of this type can use pointer comparison for `===`

layout optimization internal changes (support pointers inlining/unboxing into parents/codegen) [disabled] #33886

layout optimization internal changes (support pointers inlining/unboxing into parents/codegen) [disabled] #33886

Conversation

vtjnash commented Nov 18, 2019

JeffBezanson commented Nov 18, 2019

andyferris commented Nov 18, 2019

nanosoldier commented Nov 19, 2019

JeffBezanson commented Nov 21, 2019

andyferris commented Nov 21, 2019

c42f commented Nov 21, 2019

JeffBezanson Dec 6, 2019

Choose a reason for hiding this comment

andyferris commented Dec 6, 2019

vtjnash commented Dec 7, 2019

KristofferC Dec 17, 2019

Choose a reason for hiding this comment

JeffBezanson Dec 17, 2019

Choose a reason for hiding this comment

ararslan Dec 17, 2019

Choose a reason for hiding this comment