From fa1133351cd2200c12301d7e0ab62ad7e3712d25 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Sat, 16 Oct 2021 11:19:36 -0400 Subject: [PATCH 1/3] Remove double allocations in Base.map by having to reverse the list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit instead create it recursively in-order, via tail-recursion Big perf improvement: Before: ```julia julia> @btime DataStructures.map(x->x*2, $(list((1:1000)...))); 148.476 μs (6469 allocations: 163.55 KiB) ``` After: ```julia julia> @btime DataStructures.map(x->x*2, $(list((1:1000)...))); 29.705 μs (1745 allocations: 42.89 KiB) ``` --- src/list.jl | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/list.jl b/src/list.jl index fbc93b96f..102b57dd4 100644 --- a/src/list.jl +++ b/src/list.jl @@ -70,12 +70,8 @@ end Base.map(f::Base.Callable, l::Nil) = l function Base.map(f::Base.Callable, l::Cons{T}) where T - first = f(l.head) - l2 = cons(first, nil(typeof(first) <: T ? T : typeof(first))) - for h in l.tail - l2 = cons(f(h), l2) - end - reverse(l2) + rest = Base.map(f, l.tail) + return cons(f(l.head), rest) end function Base.filter(f::Function, l::LinkedList{T}) where T From 811e1eaa1df3f23805ccae3f5fd3fbd60afd6896 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Sat, 16 Oct 2021 11:28:23 -0400 Subject: [PATCH 2/3] Unroll the recursive function into iteration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This avoids a StackOverflow error, and also turns out to be faster still, and lets us maintain the ability for map to return a list of a different eltype than the original, added in #27. Before: ```julia julia> @btime DataStructures.map(x->x*2, $(list((1:1000)...))); 29.705 μs (1745 allocations: 42.89 KiB) ``` After: ```julia julia> @btime DataStructures.map(x->x*2, $(list((1:1000)...))); 13.139 μs (1011 allocations: 47.65 KiB) ``` I'm not sure how there are fewer allocations.. But I can see how it'd be faster, with no recursive function calls. --- src/list.jl | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/list.jl b/src/list.jl index 102b57dd4..5b03a13f6 100644 --- a/src/list.jl +++ b/src/list.jl @@ -70,8 +70,20 @@ end Base.map(f::Base.Callable, l::Nil) = l function Base.map(f::Base.Callable, l::Cons{T}) where T - rest = Base.map(f, l.tail) - return cons(f(l.head), rest) + # Tail recursion manually unrolled into iterative loop with local stack to avoid + # StackOverflow exception. The recursive logic is: + # map(f::Base.Callable, l::Cons) = cons(f(head(l)), map(f, tail(l))) + # Recursion to iteration instructions: https://stackoverflow.com/a/159777/751061 + stack = Vector{T}() + while !(tail(l) isa Nil) + push!(stack, head(l)) + l::Cons{T} = tail(l)::Cons{T} + end + l2 = list(f(head(l))) # Note this might have a different eltype than T + for i in reverse(1:length(stack)) + l2 = cons(f(stack[i]), l2) + end + return l2 end function Base.filter(f::Function, l::LinkedList{T}) where T From b1034b85e140481378d83796898d85fec3c9df2c Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Sat, 16 Oct 2021 11:44:43 -0400 Subject: [PATCH 3/3] Fix handling of lists returning different type --- src/list.jl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/list.jl b/src/list.jl index 5b03a13f6..228b0ddab 100644 --- a/src/list.jl +++ b/src/list.jl @@ -79,7 +79,9 @@ function Base.map(f::Base.Callable, l::Cons{T}) where T push!(stack, head(l)) l::Cons{T} = tail(l)::Cons{T} end - l2 = list(f(head(l))) # Note this might have a different eltype than T + # Note the new list might have a different eltype than T + first = f(head(l)) + l2 = cons(first, nil(typeof(first) <: T ? T : typeof(first))) for i in reverse(1:length(stack)) l2 = cons(f(stack[i]), l2) end