Couple of tuple speed-ups and fixes #20720

pabloferz · 2017-02-21T18:29:13Z

Should fix #20718

Sacha0 · 2017-02-21T18:50:48Z

base/tuple.jl

@@ -160,7 +160,8 @@ heads(t::Tuple, ts::Tuple...) = (t[1], heads(ts...)...)
 tails() = ()
 tails(t::Tuple, ts::Tuple...) = (tail(t), tails(ts...)...)
 map(f, ::Tuple{}, ts::Tuple...) = ()


Tangentially, this method seems incongruous with the spirit of map?

julia> map(+, (), (1,2)) () julia> map(+, [], [1,2]) ERROR: DimensionMismatch("dimensions must match") Stacktrace: [1] promote_shape(::Tuple{Base.OneTo{Int64}}, ::Tuple{Base.OneTo{Int64}}) at ./indices.jl:79 [2] _array_for(::Type{Any}, ::Base.Iterators.Zip2{Array{Any,1},Array{Int64,1}}, ::Base.HasShape) at ./array.jl:406 [3] collect(::Base.Generator{Base.Iterators.Zip2{Array{Any,1},Array{Int64,1}},Base.##3#4{Base.#+}}) at ./array.jl:416 [4] map(::Function, ::Array{Any,1}, ::Array{Int64,1}) at ./abstractarray.jl:1888

Sacha0 · 2017-02-21T18:55:18Z

base/tuple.jl

@@ -160,7 +160,8 @@ heads(t::Tuple, ts::Tuple...) = (t[1], heads(ts...)...)
 tails() = ()
 tails(t::Tuple, ts::Tuple...) = (tail(t), tails(ts...)...)
 map(f, ::Tuple{}, ts::Tuple...) = ()
-map(f, t1::Tuple, t2::Tuple, ts::Tuple...) = (f(heads(t1, t2, ts...)...), map(f, tails(t1, t2, ts...)...)...)
+map(f, t1::Tuple, t2::Tuple, ts::Tuple...) = (@_inline_meta;


Does this definition not result in arbitrarily deep inlining in the same manner that the two argument form does without this additional definition that limits inlining?

You're right, I looked at this backwards, is heads and tails what should be fine to inline. I'll try another solution.

pabloferz · 2017-02-21T21:42:38Z

Updated to avoid code blowup for large tuples.

This should also fix #20723 now. Unlike map(+, [], [1,2]), map(+, (), (1,2)) would throw ERROR: BoundsError: attempt to access (). ~~Should I make it throw DimensionMismatch("dimensions must match") instead?~~

~~NOTE: Will write tests for the later, once we decide what to throw.~~

Sacha0 · 2017-02-21T21:46:31Z

base/tuple.jl

@@ -132,7 +132,7 @@ All16{T,N} = Tuple{T,T,T,T,T,T,T,T,
                   T,T,T,T,T,T,T,T,Vararg{T,N}}
 function map(f, t::Any16)
    n = length(t)
-    A = Array{Any}(n)
+    A = Array{Any,1}(n)


Perhaps Vector?

Can't use Vector here because of bootstrapping.

Sacha0 · 2017-02-21T21:46:46Z

base/tuple.jl

@@ -148,19 +148,22 @@ function map(f, t::Tuple, s::Tuple)
 end
 function map(f, t::Any16, s::Any16)
    n = length(t)
-    A = Array{Any}(n)
+    A = Array{Any,1}(n)


Likewise here, perhaps Vector?

Sacha0 · 2017-02-21T21:51:52Z

base/tuple.jl

-map(f, t1::Tuple, t2::Tuple, ts::Tuple...) = (f(heads(t1, t2, ts...)...), map(f, tails(t1, t2, ts...)...)...)
+tails(t::Tuple, ts::Tuple...) = (@_inline_meta; (tail(t), tails(ts...)...))
+map(f, ::Tuple{}...) = ()
+map(f, t::Tuple, ts::Tuple...) = (@_inline_meta;


Do these definitions exceed the line-length recommendation? If not, perhaps condense these definitions to single lines?

It is 92 characters, right? (I don't recall exactly) If so, then indeed this exceeds the line-length recommendation.

Yes. I see the first definition exceeds the limit by a few characters though the second does not; I assume you split the second definition for consistency, so cheers :).

these should be written as long-form methods if they need to split across multiple lines

Sacha0 · 2017-02-21T22:03:59Z

base/tuple.jl

@@ -20,7 +20,7 @@ endof(t::Tuple) = length(t)
 size(t::Tuple, d) = d==1 ? length(t) : throw(ArgumentError("invalid tuple dimension $d"))
 getindex(t::Tuple, i::Int) = getfield(t, i)
 getindex(t::Tuple, i::Real) = getfield(t, convert(Int, i))
-getindex(t::Tuple, r::AbstractArray{<:Any,1}) = tuple([t[ri] for ri in r]...)
+getindex(t::Tuple, r::AbstractArray{<:Any,1}) = (n = length(eachindex(r)); ntuple(i->t[r[i]], n))


Could you explain this change's purpose? Could indexing r with indices 1:n (rather than iterating over r's elements, or indexing r more genernically) be an issue? If not, why length(eachindex(r)) rather than simply length(r)? Does using ntuple here not potentially hurt inference?

Could indexing r with indices 1:n be an issue?

I feel like we are slowly moving to support non traditional indexing, to handle things like, e.g., OffsetArrays. That's why I wrote it like this.

Does using ntuple here not potentially hurt inference?

This is already non-inferable so it doesn't hurt changing it to make it faster.

I feel like we are slowly moving to support non traditional indexing, to handle things like, e.g., OffsetArrays. That's why I wrote it like this.

Exactly :). The proposed definition indexes r from 1:n, as i iterates over 1:n in ntuple. So if indexing r from 1:n is not correct, the proposed definition will not be correct.

This is already non-inferable so it doesn't hurt changing it to make it faster.

Consider

julia> foo(t::Tuple, r::AbstractArray{<:Any,1}) = tuple([t[ri] for ri in r]...) foo (generic function with 1 method) julia> bar(t::Tuple, r::AbstractArray{<:Any,1}) = (n = length(eachindex(r)); ntuple(i->t[r[i]], n)) bar (generic function with 1 method) julia> @code_warntype foo((1,2,3), [1]) Variables: #self#::#foo t::Tuple{Int64,Int64,Int64} r::Array{Int64,1} #1::##1#2{Tuple{Int64,Int64,Int64}} Body: begin #1::##1#2{Tuple{Int64,Int64,Int64}} = $(Expr(:new, :($(QuoteNode(##1#2{Tuple{Int64,Int64,Int64}}))), :(t))) SSAValue(0) = #1::##1#2{Tuple{Int64,Int64,Int64}} SSAValue(1) = $(Expr(:new, :($(QuoteNode(Base.Generator{Array{Int64,1},##1#2{Tuple{Int64,Int64,Int64}}}))), SSAValue(0), :(r))) SSAValue(2) = $(Expr(:invoke, MethodInstance for collect(::Base.Generator{Array{Int64,1},##1#2{Tuple{Int64,Int64,Int64}}}), :(Base.collect), SSAValue(1))) return (Core._apply)(Main.tuple, SSAValue(2))::Tuple{Vararg{Int64,N} where N} end::julia> foo(t::Tuple, r::AbstractArray{<:Any,1}) = tuple([t[ri] for ri in r]...) foo (generic function with 1 method) julia> bar(t::Tuple, r::AbstractArray{<:Any,1}) = (n = length(eachindex(r)); ntuple(i->t[r[i]], n)) bar (generic function with 1 method) julia> @code_warntype foo((1,2,3), [1]) Variables: #self#::#foo t::Tuple{Int64,Int64,Int64} r::Array{Int64,1} #1::##1#2{Tuple{Int64,Int64,Int64}} Body: begin #1::##1#2{Tuple{Int64,Int64,Int64}} = $(Expr(:new, :($(QuoteNode(##1#2{Tuple{Int64,Int64,Int64}}))), :(t))) SSAValue(0) = #1::##1#2{Tuple{Int64,Int64,Int64}} SSAValue(1) = $(Expr(:new, :($(QuoteNode(Base.Generator{Array{Int64,1},##1#2{Tuple{Int64,Int64,Int64}}}))), SSAValue(0), :(r))) SSAValue(2) = $(Expr(:invoke, MethodInstance for collect(::Base.Generator{Array{Int64,1},##1#2{Tuple{Int64,Int64,Int64}}}), :(Base.collect), SSAValue(1))) return (Core._apply)(Main.tuple, SSAValue(2))::Tuple{Vararg{Int64,N} where N} end::Tuple{Vararg{Int64,N} where N} julia> @code_warntype bar((1,2,3), [1]) Variables: #self#::#bar t::Tuple{Int64,Int64,Int64} r::Array{Int64,1} n::Int64 #3::##3#4{Tuple{Int64,Int64,Int64},Array{Int64,1}} Body: begin $(Expr(:inbounds, false)) # meta: location abstractarray.jl eachindex 744 # meta: location abstractarray.jl indices1 63 # meta: location abstractarray.jl indices 56 SSAValue(3) = (Base.arraysize)(r::Array{Int64,1}, 1)::Int64 # meta: location tuple.jl map 124 SSAValue(2) = SSAValue(3) # meta: pop location # meta: pop location # meta: pop location # meta: pop location $(Expr(:inbounds, :pop)) n::Int64 = (Core.typeassert)((Base.select_value)((Base.slt_int)(SSAValue(2), 0)::Bool, 0, SSAValue(2))::Int64, Int64)::Int64 #3::##3#4{Tuple{Int64,Int64,Int64},Array{Int64,1}} = $(Expr(:new, :($(QuoteNode(##3#4{Tuple{Int64,Int64,Int64},Array{Int64,1}}))), :(t), :(r))) SSAValue(0) = #3::##3#4{Tuple{Int64,Int64,Int64},Array{Int64,1}} return $(Expr(:invoke, MethodInstance for ntuple(::##3#4{Tuple{Int64,Int64,Int64},Array{Int64,1}}, ::Int64), :(Main.ntuple), SSAValue(0), :(n))) end::Tuple julia> julia> @code_warntype bar((1,2,3), [1]) Variables: #self#::#bar t::Tuple{Int64,Int64,Int64} r::Array{Int64,1} n::Int64 #3::##3#4{Tuple{Int64,Int64,Int64},Array{Int64,1}} Body: begin $(Expr(:inbounds, false)) # meta: location abstractarray.jl eachindex 744 # meta: location abstractarray.jl indices1 63 # meta: location abstractarray.jl indices 56 SSAValue(3) = (Base.arraysize)(r::Array{Int64,1}, 1)::Int64 # meta: location tuple.jl map 124 SSAValue(2) = SSAValue(3) # meta: pop location # meta: pop location # meta: pop location # meta: pop location $(Expr(:inbounds, :pop)) n::Int64 = (Core.typeassert)((Base.select_value)((Base.slt_int)(SSAValue(2), 0)::Bool, 0, SSAValue(2))::Int64, Int64)::Int64 #3::##3#4{Tuple{Int64,Int64,Int64},Array{Int64,1}} = $(Expr(:new, :($(QuoteNode(##3#4{Tuple{Int64,Int64,Int64},Array{Int64,1}}))), :(t), :(r))) SSAValue(0) = #3::##3#4{Tuple{Int64,Int64,Int64},Array{Int64,1}} return $(Expr(:invoke, MethodInstance for ntuple(::##3#4{Tuple{Int64,Int64,Int64},Array{Int64,1}}, ::Int64), :(Main.ntuple), SSAValue(0), :(n))) end::Tuple

The existing definition yields Tuple{Vararg{Int64,N} where N} whereas the proposed definition yields only Tuple; some information is lost?

Somehow I forgot that eachindex does not necessarily return an iterable that can be indexed with UnitRanges. I updated the code with an alternative.

Re. inferability: I see that we'd loose some information, but I'd argue that when working with tuples what we most commonly want to infer is the length, so loosing the eltype here is not that bad, if it buy us some speed. Though, I could be convinced otherwise.

pabloferz · 2017-02-22T04:14:15Z

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

nanosoldier · 2017-02-22T07:39:25Z

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

felixrehren · 2017-02-22T09:01:33Z

Those sqrtm tests seem very noisy. I can't think how these changes to tuples would have affected them. cc @jrevels

Sacha0 · 2017-02-25T16:55:34Z

base/tuple.jl

+function getindex(t::Tuple, r::AbstractArray{<:Any,1})
+    s = Ref(start(r))
+    ntuple(j -> ((i, s[]) = next(r, s[]); t[i]), length(r))
+end


This change seems orthogonal to the other changes in this pull request. Perhaps split this change into a separate pull request for additional discussion and to attract additional review eyes? (I remain concerned about losing eltype information with this change, and see no reason that that information is not important in general.)

With the changes to ntuple, the eltype for this would be inferred. But I'll probably left this aside anyway.

Interesting :). Out of curiosity, which of the simultaneous ntuple changes make the new getindex version's eltype inferable? (The generator splat to array splat change?)

If this version does not lose eltype inferability, then cheers form this end. (Though having another pair of eyes on these changes would still be worthwhile.) Best!

Turns out that inference works different when you directly return something like c1 ? r1 : c2 ? r2 : c3 ? r3 ... to when you assign that to a variable and then return the variable. So basically what improved things was assigning the tuple before returning it.

I won't leave this version as is as it messes inference when using this function inside macros in some circumstances (because of the closure). But I have a slightly different version that doesn't have that limitation.

Nice find! Inference's sensitivity to such things is somewhat disconcerting. Best!

Sacha0 · 2017-02-25T17:00:22Z

base/tuple.jl

+    return t
+end
+
+_ntuple(f::Function, n::Integer) = (@_noinline_meta; ([f(i) for i = 1:n]...))


Why switch from splatting a generator to splatting an array?

For the time being, is faster than using the generator.

Sacha0 · 2017-02-25T17:01:23Z

base/tuple.jl

+        n == 7  ? (f(1), f(2), f(3), f(4), f(5), f(6), f(7)) :
+        n == 8  ? (f(1), f(2), f(3), f(4), f(5), f(6), f(7), f(8)) :
+        n == 9  ? (f(1), f(2), f(3), f(4), f(5), f(6), f(7), f(8), f(9)) :
+        n == 10 ? (f(1), f(2), f(3), f(4), f(5), f(6), f(7), f(8), f(9), f(10)) :


Does this change impact behavior for 10 < n < 16? (Out of curiosity, what motivates this change?)

This is faster in some cases or at worst has equal performance than the existing implementation.

Interesting that LLVM doesn't reduce this to a calculated-jump. If you care enough to try, would a binary tree implementation be better? (It's less nice from a code-simplicity standpoint, so don't feel you have to do this.)

pabloferz · 2017-02-25T23:34:49Z

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

nanosoldier · 2017-02-26T00:26:50Z

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: failed process: Process(`sudo cset shield -e su nanosoldier -- -c ./benchscript.sh`, ProcessExited(1)) [1]

Logs and partial data can be found here
cc @jrevels

pabloferz · 2017-03-14T19:04:38Z

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

nanosoldier · 2017-03-14T19:29:17Z

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: failed process: Process(`sudo cset shield -e su nanosoldier -- -c ./benchscript.sh`, ProcessExited(1)) [1]

Logs and partial data can be found here
cc @jrevels

pabloferz · 2017-03-16T20:45:10Z

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

nanosoldier · 2017-03-17T00:29:05Z

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

pabloferz · 2017-03-17T00:40:08Z

Once more to see if anything is noise

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

nanosoldier · 2017-03-17T03:46:10Z

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

timholy

👍

timholy · 2017-03-17T08:45:01Z

base/range.jl

@@ -151,6 +151,10 @@ unitrange_last{T}(start::T, stop::T) =
    ifelse(stop >= start, convert(T,start+floor(stop-start)),
                          convert(T,start-oneunit(stop-start)))

+if isdefined(Main, :Base)
+    getindex(t::Tuple, r::UnitRange) = (o = r.start - 1; ntuple(n -> t[o + n], length(r)))


Maybe AbstractUnitRange? And use first(r) rather than r.start?

timholy · 2017-03-17T08:52:33Z

base/tuple.jl

+        n == 7  ? (f(1), f(2), f(3), f(4), f(5), f(6), f(7)) :
+        n == 8  ? (f(1), f(2), f(3), f(4), f(5), f(6), f(7), f(8)) :
+        n == 9  ? (f(1), f(2), f(3), f(4), f(5), f(6), f(7), f(8), f(9)) :
+        n == 10 ? (f(1), f(2), f(3), f(4), f(5), f(6), f(7), f(8), f(9), f(10)) :


Interesting that LLVM doesn't reduce this to a calculated-jump. If you care enough to try, would a binary tree implementation be better? (It's less nice from a code-simplicity standpoint, so don't feel you have to do this.)

timholy · 2017-03-17T08:55:36Z

base/tuple.jl

-map(f, t1::Tuple, t2::Tuple, ts::Tuple...) = (f(heads(t1, t2, ts...)...), map(f, tails(t1, t2, ts...)...)...)
+heads(ts::Tuple...) = map(t -> t[1], ts)
+tails(ts::Tuple...) = map(tail, ts)
+map(f, ::Tuple{}...) = ()


This is a breaking change because of this:

julia> map(+, (), (3,), (4,)) ()

Consequently, this PR will have to wait until 1.0-dev.

should that be a dimension mismatch error?

I thought this was a bug. X-ref: #20723

I don't really like that behavior (I like this better), but I'd be surprised if there isn't code out there that counts on the current behavior. So even though I'd call this an improvement, I think it's dangerous to introduce a behavior change at this stage of the release process unless there's reason (1) to be certain it's a bug, or (2) be almost certain no one had been using this. But don't take that as official policy, it's just my viewpoint.

since we had an alpha tag, I plan on running a pkgeval comparison before tagging a beta, so we could find out

Some of these are flaky tolerances or timeouts, but AxisArrays failure looks related https://gist.github.com/2290f8d80dea51c9dc4aa3b4de59daf2

JuliaArrays/AxisArrays.jl#74

Could we merge this then, or should we wait until a new version of AxisArrays is tagged?

@timholy what do you think?

Go for it. I just tagged a new release.

pabloferz · 2017-03-18T03:03:18Z

Interesting that LLVM doesn't reduce this to a calculated-jump.

It does if I remove the assignment and return directly, but then inference isn't able to at least figure out the eltype (it infers Tuple instead NTuple{N,T} where N for the appropriate T).

If you care enough to try, would a binary tree implementation be better?

For now I'd like to keep the code simplicity and leave other approaches for another occasion.

See JuliaLang/julia#20723 and JuliaLang/julia#20720

Sacha0

Thanks for sticking with this @pabloferz! :)

Sacha0 reviewed Feb 21, 2017

View reviewed changes

This was referenced Feb 21, 2017

Too many allocations when binary dot operators for tuples are used multiple times in a single line #20718

Closed

odd map definition over multiple tuples where the leading tuple is empty #20723

Closed

ararslan added the domain:collections Data structures holding multiple items, e.g. sets label Feb 21, 2017

pabloferz force-pushed the pz/maptups branch from 25dc48e to 5625cb5 Compare February 21, 2017 21:38

pabloferz changed the title ~~Couple of tuple speed-ups~~ Couple of tuple speed-ups and fixes Feb 21, 2017

Sacha0 reviewed Feb 21, 2017

View reviewed changes

pabloferz force-pushed the pz/maptups branch from 5625cb5 to aaa7ec9 Compare February 22, 2017 04:01

pabloferz force-pushed the pz/maptups branch from aaa7ec9 to 7d7373a Compare February 23, 2017 23:42

Sacha0 reviewed Feb 25, 2017

View reviewed changes

pabloferz mentioned this pull request Mar 7, 2017

Inference StackOverflow on BenchmarkTools when defining specialized getindex for Tuples and UnitRanges #20923

Closed

pabloferz force-pushed the pz/maptups branch 2 times, most recently from 7a711b7 to cbc0bc9 Compare March 14, 2017 19:03

pabloferz mentioned this pull request Mar 14, 2017

Can't index into a Dict{Int,Type} after running BaseBenchmarks.loadall!() #21028

Closed

pabloferz force-pushed the pz/maptups branch 2 times, most recently from 3ad3c51 to 26e8fed Compare March 16, 2017 20:35

pabloferz force-pushed the pz/maptups branch from 26e8fed to 6ce10ba Compare March 17, 2017 00:31

pabloferz requested a review from timholy March 17, 2017 00:40

timholy reviewed Mar 17, 2017

View reviewed changes

pabloferz force-pushed the pz/maptups branch from 6ce10ba to 82422e8 Compare March 18, 2017 15:43

pabloferz force-pushed the pz/maptups branch from 82422e8 to 8000518 Compare April 1, 2017 17:22

pabloferz added a commit to pabloferz/AxisArrays.jl that referenced this pull request Apr 2, 2017

Avoid relying on wrong behaviour or map

2a08739

See JuliaLang/julia#20723 and JuliaLang/julia#20720

pabloferz mentioned this pull request Apr 2, 2017

Avoid relying on wrong behaviour of map JuliaArrays/AxisArrays.jl#74

Merged

timholy mentioned this pull request Apr 3, 2017

Ensure that collect(::AbstractArray) always returns an Array #21257

Merged

Sacha0 approved these changes Apr 3, 2017

View reviewed changes

pabloferz added 2 commits April 6, 2017 12:15

Couple of tuple speed-ups

9f0a952

Homogenize behaviour of map over tuples when one is empty

1d1ff14

pabloferz force-pushed the pz/maptups branch from 8000518 to 1d1ff14 Compare April 6, 2017 17:16

pabloferz merged commit d5d7280 into JuliaLang:master Apr 6, 2017

pabloferz deleted the pz/maptups branch April 6, 2017 20:22

Couple of tuple speed-ups and fixes #20720

Couple of tuple speed-ups and fixes #20720

Conversation

pabloferz commented Feb 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pabloferz commented Feb 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pabloferz Feb 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pabloferz Feb 21, 2017 • edited Loading

Choose a reason for hiding this comment

Sacha0 Feb 21, 2017 • edited Loading

Choose a reason for hiding this comment

pabloferz Feb 22, 2017 • edited Loading

Choose a reason for hiding this comment

pabloferz commented Feb 22, 2017

nanosoldier commented Feb 22, 2017

felixrehren commented Feb 22, 2017

Choose a reason for hiding this comment

pabloferz Feb 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pabloferz Feb 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pabloferz Feb 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pabloferz commented Feb 25, 2017

nanosoldier commented Feb 26, 2017

pabloferz commented Mar 14, 2017

nanosoldier commented Mar 14, 2017

pabloferz commented Mar 16, 2017

nanosoldier commented Mar 17, 2017

pabloferz commented Mar 17, 2017

nanosoldier commented Mar 17, 2017

timholy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pabloferz Apr 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pabloferz commented Mar 18, 2017

Sacha0 left a comment

Choose a reason for hiding this comment

pabloferz commented Feb 21, 2017 •

edited

Loading

pabloferz Feb 21, 2017 •

edited

Loading

pabloferz Feb 21, 2017 •

edited

Loading

Sacha0 Feb 21, 2017 •

edited

Loading

pabloferz Feb 22, 2017 •

edited

Loading

pabloferz Feb 25, 2017 •

edited

Loading

pabloferz Feb 25, 2017 •

edited

Loading

pabloferz Feb 25, 2017 •

edited

Loading

pabloferz Apr 4, 2017 •

edited

Loading