From 5b8d75667bd20b369dcddd675da7a4781782366e Mon Sep 17 00:00:00 2001 From: jmbromley <14954519+jmbromley@users.noreply.github.com> Date: Thu, 29 Feb 2024 18:00:02 +0000 Subject: [PATCH 1/7] Make groupsOf... family of functions fully tail recursive. This commit makes the `groupsOf...` family of functions fully tail recursive by forcing them to use the tail recursive version of List.take (normally List.take is only tail recursive for lists larger than 1000, but since the `groupsOf...` functions are themselves recursive this can result in potential call stack overflow from the successive accumulation of (up to) 1000-long non-recursive List.take calls during the recursion). This is an alternative to PR #46 which would instead just add a note to the documentation warning users about the potential overflow. --- src/List/Extra.elm | 43 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/src/List/Extra.elm b/src/List/Extra.elm index 99e6b11..e0f744f 100644 --- a/src/List/Extra.elm +++ b/src/List/Extra.elm @@ -2011,7 +2011,7 @@ groupsOfWithStep size step list = else let thisGroup = - List.take size xs + takeTailRec size xs in if size == List.length thisGroup then let @@ -2026,6 +2026,45 @@ groupsOfWithStep size step list = go list [] +{- List.take starts out non-tail-recursive and switches to a tail-recursive + implementation after the first 1000 iterations. For functions which are themselves + recursive and use List.take on each call (e.g. List.Extra.groupsOf), this can result + in potential call stack overflow from the successive accumulation of up to 1000-long + non-recursive List.take calls. Here we provide an always tail recursive version of + List.take to avoid this problem. The code is taken directly from the implementation + of elm/core and carries the following copywrite: + + Copyright 2014-present Evan Czaplicki + + Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: + + 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. + + 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution. + + 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission. + + THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +-} +takeTailRec : Int -> List a -> List a +takeTailRec n list = + List.reverse (takeReverse n list []) + + +takeReverse : Int -> List a -> List a -> List a +takeReverse n list kept = + if n <= 0 then + kept + + else + case list of + [] -> + kept + + x :: xs -> + takeReverse (n - 1) xs (x :: kept) + + {-| `groupsOfVarying ns` takes `n` elements from a list for each `n` in `ns`, splitting the list into variably sized segments groupsOfVarying [ 2, 3, 1 ] [ "a", "b", "c", "d", "e", "f" ] @@ -2105,7 +2144,7 @@ greedyGroupsOfWithStep size step list = else go (List.drop step xs) - (List.take size xs :: acc) + (takeTailRec size xs :: acc) in go list [] From ecf9bad8e3f4472cf7960d3cf1c883ed0cd3b60b Mon Sep 17 00:00:00 2001 From: jmbromley <14954519+jmbromley@users.noreply.github.com> Date: Fri, 1 Mar 2024 15:23:35 +0000 Subject: [PATCH 2/7] Move copyright notice to main LICENSE file. --- LICENSE | 14 ++++++++++++++ src/List/Extra.elm | 11 ----------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/LICENSE b/LICENSE index afd0993..efc8454 100644 --- a/LICENSE +++ b/LICENSE @@ -110,6 +110,20 @@ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +Fully tail recursive take function (and helper): + +Copyright 2014-present Evan Czaplicki + + Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: + + 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. + + 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution. + + 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission. + + THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + --- String.Extra: diff --git a/src/List/Extra.elm b/src/List/Extra.elm index e0f744f..80220ed 100644 --- a/src/List/Extra.elm +++ b/src/List/Extra.elm @@ -2034,17 +2034,6 @@ groupsOfWithStep size step list = List.take to avoid this problem. The code is taken directly from the implementation of elm/core and carries the following copywrite: - Copyright 2014-present Evan Czaplicki - - Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: - - 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. - - 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution. - - 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission. - - THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -} takeTailRec : Int -> List a -> List a takeTailRec n list = From e7220f8329cab8c39c42f9f9d26d5c84e91c4c55 Mon Sep 17 00:00:00 2001 From: jmbromley <14954519+jmbromley@users.noreply.github.com> Date: Mon, 18 Mar 2024 23:38:11 +0000 Subject: [PATCH 3/7] Fix line spacing to match elm-format standard. --- src/List/Extra.elm | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/List/Extra.elm b/src/List/Extra.elm index 80220ed..ee6ed6c 100644 --- a/src/List/Extra.elm +++ b/src/List/Extra.elm @@ -2026,6 +2026,7 @@ groupsOfWithStep size step list = go list [] + {- List.take starts out non-tail-recursive and switches to a tail-recursive implementation after the first 1000 iterations. For functions which are themselves recursive and use List.take on each call (e.g. List.Extra.groupsOf), this can result @@ -2035,6 +2036,8 @@ groupsOfWithStep size step list = of elm/core and carries the following copywrite: -} + + takeTailRec : Int -> List a -> List a takeTailRec n list = List.reverse (takeReverse n list []) From a515049ae0eba2606f1092850c3d9a714c193d4e Mon Sep 17 00:00:00 2001 From: Jakub Hampl Date: Thu, 21 Mar 2024 17:33:42 +0000 Subject: [PATCH 4/7] Commits benchmarks --- benchmarks/src/Benchmarks.elm | 44 +++++++-- benchmarks/src/List/Extra/GroupsOf.elm | 123 +++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 9 deletions(-) create mode 100644 benchmarks/src/List/Extra/GroupsOf.elm diff --git a/benchmarks/src/Benchmarks.elm b/benchmarks/src/Benchmarks.elm index b5c5fd5..0bae165 100644 --- a/benchmarks/src/Benchmarks.elm +++ b/benchmarks/src/Benchmarks.elm @@ -20,6 +20,7 @@ import Benchmark.Runner.Alternative as BenchmarkRunner import List.Extra import List.Extra.Unfoldr import List.Extra.UniquePairs +import List.Extra.GroupsOf import Set exposing (Set) import Set.Extra.AreDisjoint import Set.Extra.SymmetricDifference @@ -29,13 +30,15 @@ import String.Extra.IsBlank main : BenchmarkRunner.Program main = describe "for core-extra" - [ application - , array - , arrayExtra - , listExtra - , tupleExtra - , setExtra - , stringExtra + [ + -- application + -- , array + -- , arrayExtra + -- , + listExtra + -- , tupleExtra + -- , setExtra + -- , stringExtra ] |> BenchmarkRunner.program @@ -182,17 +185,40 @@ listExtra = List.range 1 100 in describe "List.Extra" - [ rank "uniquePairs" + ([ rank "uniquePairs" (\uniquePairs -> uniquePairs intList) [ ( "original (++)", List.Extra.UniquePairs.originalConcat ) , ( "tail-recursive", List.Extra.UniquePairs.tailRecursive ) ] - , rank "unfoldr" + , rank "unfoldr" (\unfoldr -> unfoldr subtractOneUntilZero 100) [ ( "original", List.Extra.Unfoldr.nonTailRecursive ) , ( "tail-recursive", List.Extra.Unfoldr.tailRecursive ) ] + ] + ++ List.concatMap toComparisonsGroupsOfWithStep (List.range 1 4) + ) + +toComparisonsGroupsOfWithStep : Int -> List Benchmark +toComparisonsGroupsOfWithStep exponent = + let + listSize = + 10 ^ exponent + + range = + List.range 1 listSize + in + [ rank ("groupsOfWithStep 3 2 [1.." ++ String.fromInt listSize ++ "]") + (\impl -> impl 3 2 range) + [ ( "using elm-core's List.tail", List.Extra.GroupsOf.coreTailGroupsOfWithStep ) + , ( "using fully tail-recursive List.tail", List.Extra.GroupsOf.tailRecGroupsOfWithStep ) + ] + , rank ("greedyGroupsOfWithStep 3 2 [1.." ++ String.fromInt listSize ++ "]") + (\impl -> impl 3 2 range) + [ ( "using elm-core's List.tail", List.Extra.GroupsOf.coreTailGreedyGroupsOfWithStep ) + , ( "using fully tail-recursive List.tail", List.Extra.GroupsOf.tailRecGreedyGroupsOfWithStep ) ] + ] tupleExtra : Benchmark diff --git a/benchmarks/src/List/Extra/GroupsOf.elm b/benchmarks/src/List/Extra/GroupsOf.elm new file mode 100644 index 0000000..9682430 --- /dev/null +++ b/benchmarks/src/List/Extra/GroupsOf.elm @@ -0,0 +1,123 @@ +module List.Extra.GroupsOf exposing (coreTailGroupsOfWithStep, coreTailGreedyGroupsOfWithStep, tailRecGroupsOfWithStep, tailRecGreedyGroupsOfWithStep) + +import Benchmark +import Benchmark.Runner.Alternative as BenchmarkRunner + + +coreTailGroupsOfWithStep : Int -> Int -> List a -> List (List a) +coreTailGroupsOfWithStep size step list = + if size <= 0 || step <= 0 then + [] + + else + let + go : List a -> List (List a) -> List (List a) + go xs acc = + if List.isEmpty xs then + List.reverse acc + + else + let + thisGroup = + List.take size xs + in + if size == List.length thisGroup then + let + rest = + List.drop step xs + in + go rest (thisGroup :: acc) + + else + List.reverse acc + in + go list [] + + +coreTailGreedyGroupsOfWithStep : Int -> Int -> List a -> List (List a) +coreTailGreedyGroupsOfWithStep size step list = + if size <= 0 || step <= 0 then + [] + + else + let + go : List a -> List (List a) -> List (List a) + go xs acc = + if List.isEmpty xs then + List.reverse acc + + else + go + (List.drop step xs) + (List.take size xs :: acc) + in + go list [] + + +tailRecGroupsOfWithStep : Int -> Int -> List a -> List (List a) +tailRecGroupsOfWithStep size step list = + if size <= 0 || step <= 0 then + [] + + else + let + go : List a -> List (List a) -> List (List a) + go xs acc = + if List.isEmpty xs then + List.reverse acc + + else + let + thisGroup = + takeTailRec size xs + in + if size == List.length thisGroup then + let + rest = + List.drop step xs + in + go rest (thisGroup :: acc) + + else + List.reverse acc + in + go list [] + + +tailRecGreedyGroupsOfWithStep : Int -> Int -> List a -> List (List a) +tailRecGreedyGroupsOfWithStep size step list = + if size <= 0 || step <= 0 then + [] + + else + let + go : List a -> List (List a) -> List (List a) + go xs acc = + if List.isEmpty xs then + List.reverse acc + + else + go + (List.drop step xs) + (takeTailRec size xs :: acc) + in + go list [] + + +takeTailRec : Int -> List a -> List a +takeTailRec n list = + List.reverse (takeReverse n list []) + + +takeReverse : Int -> List a -> List a -> List a +takeReverse n list kept = + if n <= 0 then + kept + + else + case list of + [] -> + kept + + x :: xs -> + takeReverse (n - 1) xs (x :: kept) From 2deef416d0cb014920145fc64c3ef3a949ea64a0 Mon Sep 17 00:00:00 2001 From: Jakub Hampl Date: Thu, 21 Mar 2024 17:37:34 +0000 Subject: [PATCH 5/7] Fix CI --- benchmarks/src/Benchmarks.elm | 15 ++++++++------- benchmarks/src/List/Extra/GroupsOf.elm | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/benchmarks/src/Benchmarks.elm b/benchmarks/src/Benchmarks.elm index 0bae165..a500852 100644 --- a/benchmarks/src/Benchmarks.elm +++ b/benchmarks/src/Benchmarks.elm @@ -18,9 +18,9 @@ import Benchmark exposing (Benchmark, describe) import Benchmark.Alternative exposing (rank) import Benchmark.Runner.Alternative as BenchmarkRunner import List.Extra +import List.Extra.GroupsOf import List.Extra.Unfoldr import List.Extra.UniquePairs -import List.Extra.GroupsOf import Set exposing (Set) import Set.Extra.AreDisjoint import Set.Extra.SymmetricDifference @@ -30,12 +30,12 @@ import String.Extra.IsBlank main : BenchmarkRunner.Program main = describe "for core-extra" - [ - -- application - -- , array - -- , arrayExtra - -- , - listExtra + [ -- application + -- , array + -- , arrayExtra + -- , + listExtra + -- , tupleExtra -- , setExtra -- , stringExtra @@ -199,6 +199,7 @@ listExtra = ++ List.concatMap toComparisonsGroupsOfWithStep (List.range 1 4) ) + toComparisonsGroupsOfWithStep : Int -> List Benchmark toComparisonsGroupsOfWithStep exponent = let diff --git a/benchmarks/src/List/Extra/GroupsOf.elm b/benchmarks/src/List/Extra/GroupsOf.elm index 9682430..8e2f025 100644 --- a/benchmarks/src/List/Extra/GroupsOf.elm +++ b/benchmarks/src/List/Extra/GroupsOf.elm @@ -1,4 +1,4 @@ -module List.Extra.GroupsOf exposing (coreTailGroupsOfWithStep, coreTailGreedyGroupsOfWithStep, tailRecGroupsOfWithStep, tailRecGreedyGroupsOfWithStep) +module List.Extra.GroupsOf exposing (coreTailGreedyGroupsOfWithStep, coreTailGroupsOfWithStep, tailRecGreedyGroupsOfWithStep, tailRecGroupsOfWithStep) import Benchmark import Benchmark.Runner.Alternative as BenchmarkRunner From c6d8f14f5ebe5b52e7f56cb0689ddd9ebd9fc968 Mon Sep 17 00:00:00 2001 From: Jakub Hampl Date: Thu, 21 Mar 2024 17:42:12 +0000 Subject: [PATCH 6/7] Fix forgotten comment --- benchmarks/src/Benchmarks.elm | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/benchmarks/src/Benchmarks.elm b/benchmarks/src/Benchmarks.elm index a500852..6899522 100644 --- a/benchmarks/src/Benchmarks.elm +++ b/benchmarks/src/Benchmarks.elm @@ -30,15 +30,13 @@ import String.Extra.IsBlank main : BenchmarkRunner.Program main = describe "for core-extra" - [ -- application - -- , array - -- , arrayExtra - -- , - listExtra - - -- , tupleExtra - -- , setExtra - -- , stringExtra + [ application + , array + , arrayExtra + , listExtra + , tupleExtra + , setExtra + , stringExtra ] |> BenchmarkRunner.program From fd35d470de23418fe8370b0d0a1963afc273c4a5 Mon Sep 17 00:00:00 2001 From: jmbromley <14954519+jmbromley@users.noreply.github.com> Date: Fri, 22 Mar 2024 21:25:34 +0000 Subject: [PATCH 7/7] Fix copyright comment. --- src/List/Extra.elm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/List/Extra.elm b/src/List/Extra.elm index ee6ed6c..c7a9ac3 100644 --- a/src/List/Extra.elm +++ b/src/List/Extra.elm @@ -2033,7 +2033,7 @@ groupsOfWithStep size step list = in potential call stack overflow from the successive accumulation of up to 1000-long non-recursive List.take calls. Here we provide an always tail recursive version of List.take to avoid this problem. The code is taken directly from the implementation - of elm/core and carries the following copywrite: + of elm/core and shares its copyright (see LICENSE file). -}