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.

Sign up for GitHub

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

Jump to bottom

More latency/inferrability work #21

Closed
wants to merge 3 commits into from
Closed

More latency/inferrability work #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6076b9c
Comment out precompile
timholy Jul 30, 2021
96f3309
Improve inferrability and use enums rather than types for some settings
timholy Jul 30, 2021
7d51444
Change how precompile works
timholy Jul 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Observables = "510215fc-4207-5dde-b226-833fc4488ee2"
[compat]
GeometryBasics = "0.4.1"
Match = "^1"
Observables = "0.3, 0.4"
Observables = "0.4"
julia = "1"

[extras]
Expand Down
11 changes: 7 additions & 4 deletions src/GridLayoutBase.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
module GridLayoutBase

if isdefined(Base, :Experimental) && isdefined(Base.Experimental, Symbol("@compiler_options"))
@eval Base.Experimental.@compiler_options compile=min optimize=1
end

using GeometryBasics
using Observables
using Match

const DEFAULT_COLGAP = Ref{Any}(20.0)
const DEFAULT_ROWGAP = Ref{Any}(20.0)
const DEFAULT_COLGAP = Ref{Float64}(20.0)
const DEFAULT_ROWGAP = Ref{Float64}(20.0)
# These function refs can be mutated by other packages to override the default
# way of retrieving default column and row gap sizes
const DEFAULT_ROWGAP_GETTER = Ref{Function}(() -> DEFAULT_ROWGAP[])
Expand Down Expand Up @@ -39,9 +43,8 @@ export protrusionsobservable, suggestedbboxobservable, reportedsizeobservable, a
export ncols, nrows
export contents, content

if Base.VERSION >= v"1.4.2"
if Base.VERSION >= v"1.4.2" && ccall(:jl_generating_output, Cint, ()) == 1
include("precompile.jl")
_precompile_()
end

end
15 changes: 8 additions & 7 deletions src/geometry_integration.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@ width(rect::Rect{2}) = right(rect) - left(rect)
height(rect::Rect{2}) = top(rect) - bottom(rect)


function BBox(left::Number, right::Number, bottom::Number, top::Number)
function BBox(left::Float32, right::Float32, bottom::Float32, top::Float32)
mini = (left, bottom)
maxi = (right, top)
return Rect2f(mini, maxi .- mini)
end

BBox(left::Number, right::Number, bottom::Number, top::Number) =
BBox(Float32(left)::Float32, Float32(right)::Float32, Float32(bottom)::Float32, Float32(top)::Float32)

function RowCols(ncols::Int, nrows::Int)
return RowCols(
Expand All @@ -23,17 +24,17 @@ function RowCols(ncols::Int, nrows::Int)
)
end

Base.getindex(rowcols::RowCols, ::Left) = rowcols.lefts
Base.getindex(rowcols::RowCols, ::Right) = rowcols.rights
Base.getindex(rowcols::RowCols, ::Top) = rowcols.tops
Base.getindex(rowcols::RowCols, ::Bottom) = rowcols.bottoms
Base.getindex(rowcols::RowCols, side::Side) = side == Left ? rowcols.lefts :
side == Right ? rowcols.rights :
side == Top ? rowcols.tops :
side == Bottom ? rowcols.bottoms : throw_side(side)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this style better than the dispatch version? aren't four methods compiled anyway?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah I see now that you've changed it to an enum

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this style with a couple of types should be avoided in general?

Copy link
Collaborator Author

@timholy timholy Aug 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends: if Side is effectively "closed" then an enum has a big advantage, but a big weakness if your goal is external extensibility. More detail: recall that Julia specializes methods for each specific argument type:

julia> abstract type StaticNumber <: Number end

julia> struct One <: StaticNumber end

julia> struct Two <: StaticNumber end

julia> f(n::StaticNumber) = nothing
f (generic function with 1 method)

julia> f(One())

julia> f(Two())

julia> using MethodAnalysis

julia> methodinstances(f)
2-element Vector{Core.MethodInstance}:
 MethodInstance for f(::One)
 MethodInstance for f(::Two)

That means f has to be compiled twice. That's not so bad for such simple f, but if it has n arguments and each comes in 2 flavors then pretty soon you're at 2^n combinations, which is a big big problem. But with this version:

julia> @enum StaticNumber One Two

julia> f(n::StaticNumber) = nothing
f (generic function with 1 method)

julia> f(One)

julia> f(Two)

julia> using MethodAnalysis

julia> methodinstances(f)
1-element Vector{Core.MethodInstance}:
 MethodInstance for f(::StaticNumber)

it only has to be compiled once. If you have several such arguments to a multi-argument function, the resulting savings/costs can be quite large. This will presumably be more impactful in Makie itself which adds a lot more types.

The downside is that you can no longer use dispatch to extend Side unless you do it in this package. As long as you're OK with that, the advantages here are surely worth having.


"""
eachside(f)
Calls f over all sides (Left, Right, Top, Bottom), and creates a BBox from the result of f(side)
"""
function eachside(f)
    return BBox(f(Left()), f(Right()), f(Bottom()), f(Top()))
    return BBox(f(Left), f(Right), f(Bottom), f(Top))
end

"""
Expand Down
Loading