Skip to content

Commit

Permalink
Merge pull request #1276 from non-Jedi/benchmark
Browse files Browse the repository at this point in the history
Remove usage of fieldnames function at runtime where possible
  • Loading branch information
bjarthur authored Apr 21, 2019
2 parents 4d50fb4 + ec2e1a3 commit 2fe9085
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 12 deletions.
13 changes: 8 additions & 5 deletions src/aesthetics.jl
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,14 @@ const NumericalAesthetic =
pad_categorical_y, Union{Missing,Bool}, missing
end

# Calculating fieldnames at runtime is expensive
const valid_aesthetics = fieldnames(Aesthetics)


function show(io::IO, data::Aesthetics)
maxlen = 0
print(io, "Aesthetics(")
for name in fieldnames(Aesthetics)
for name in valid_aesthetics
val = getfield(data, name)
if !ismissing(val) && val != nothing
print(io, "\n ", string(name), "=")
Expand Down Expand Up @@ -136,7 +139,7 @@ getindex(aes::Aesthetics, i::Integer, j::AbstractString) = getfield(aes, Symbol(
# Return the set of variables that are non-nothing.
function defined_aesthetics(aes::Aesthetics)
vars = Set{Symbol}()
for name in fieldnames(Aesthetics)
for name in valid_aesthetics
getfield(aes, name) === nothing || push!(vars, name)
end
vars
Expand Down Expand Up @@ -194,7 +197,7 @@ end
# Modifies: a
#
function update!(a::Aesthetics, b::Aesthetics)
for name in fieldnames(Aesthetics)
for name in valid_aesthetics
issomething(getfield(b, name)) && setfield(a, name, getfield(b, name))
end
nothing
Expand Down Expand Up @@ -226,7 +229,7 @@ json(a::Aesthetics) = join([string(a, ":", json(getfield(a, var))) for var in ae
function concat(aess::Aesthetics...)
cataes = Aesthetics()
for aes in aess
for var in fieldnames(Aesthetics)
for var in valid_aesthetics
if var in [:xviewmin, :yviewmin]
mu, mv = getfield(cataes, var), getfield(aes, var)
setfield!(cataes, var,
Expand Down Expand Up @@ -397,7 +400,7 @@ end
function inherit!(a::Aesthetics, b::Aesthetics;
clobber=[])
clobber_set = Set{Symbol}(clobber)
for field in fieldnames(Aesthetics)
for field in valid_aesthetics
aval = getfield(a, field)
bval = getfield(b, field)
if field in clobber_set
Expand Down
6 changes: 4 additions & 2 deletions src/data.jl
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
titles, Dict{Symbol, AbstractString}, Dict{Symbol, AbstractString}()
end

# Calculating fieldnames at runtime is expensive
const data_fields = fieldnames(Data)


# Produce a new Data instance chaining the values of one or more others.
Expand All @@ -58,7 +60,7 @@ end
#
function chain(ds::Data...)
chained_data = Data()
for name in fieldnames(Data)
for name in data_fields
vs = Any[getfield(d, name) for d in ds]
vs = Any[v for v in filter(issomething, vs)]
if isempty(vs)
Expand All @@ -75,7 +77,7 @@ end
function show(io::IO, data::Data)
maxlen = 0
print(io, "Data(")
for name in fieldnames(Data)
for name in data_fields
if getfield(data, name) != nothing
print(io, "\n ", string(name), "=")
show(io, getfield(data, name))
Expand Down
1 change: 0 additions & 1 deletion src/mapping.jl
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ end # module Row
# A new mapping with aliases evaluated and unrecognized aesthetics removed.
#
function cleanmapping(mapping::Dict)
valid_aesthetics = fieldnames(Aesthetics)
cleaned = Dict{Symbol, Any}()
for (key, val) in mapping
# skip the "order" pesudo-aesthetic, used to order layers
Expand Down
7 changes: 4 additions & 3 deletions src/scale.jl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ using IndirectArrays
using CategoricalArrays
using Printf

import Gadfly: element_aesthetics, isconcrete, concrete_length, discretize_make_ia, aes2str
import Gadfly: element_aesthetics, isconcrete, concrete_length, discretize_make_ia,
aes2str, valid_aesthetics
import Distributions: Distribution

include("color_misc.jl")
Expand Down Expand Up @@ -236,7 +237,7 @@ function apply_scale(scale::ContinuousScale,
label_var = Symbol(var, "_label")
end

if in(label_var, Set(fieldnames(typeof(aes))))
if in(label_var, valid_aesthetics)
setfield!(aes, label_var, make_labeler(scale))
end
end
Expand Down Expand Up @@ -391,7 +392,7 @@ function apply_scale(scale::DiscreteScale, aess::Vector{Gadfly.Aesthetics}, data
labeler = explicit_labeler
end

in(label_var, Set(fieldnames(typeof(aes)))) && setfield!(aes, label_var, labeler)
in(label_var, valid_aesthetics) && setfield!(aes, label_var, labeler)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion src/statistics.jl
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ function apply_statistic(stat::HistogramStatistic,
x_min -= 0.5 # adjust the left side of the bar
binwidth = 1.0
else
lims = fieldnames(typeof(stat.limits))
lims = keys(stat.limits)
x_min = in(:min, lims) ? stat.limits.min : Gadfly.concrete_minimum(vals)

isdiscrete = false
Expand Down

0 comments on commit 2fe9085

Please sign in to comment.