Skip to content

Conversation

@tkf
Copy link
Member

@tkf tkf commented Aug 10, 2020

An alternative fix to #412.

Compilation benchmark using the example in #412:

FOLDL_RECURSION_LIMIT w/ Julia 1.6.0-DEV.536 w/ Julia 1.5 w/ Julia 1.4
0 5 sec 8 sec 6 sec
1 6 sec 15 sec 8 sec
2 7 sec 21 sec 10 sec
3 8 sec 30 sec 12 sec
4 9 sec 34 sec 14 sec

FOLDL_RECURSION_LIMIT = Val(0) slows down ["teerf_filter", "noinit"] (and speedups ["filter_sum", "1000", "RandomFloats", "noinit", "base"]):

ID time ratio memory ratio
["filter_sum", "1000", "RandomFloats", "noinit", "base"] 0.31 (5%) ✅ 1.00 (1%)
["teerf_filter", "noinit"] 226.80 (5%) ❌ 832.57 (1%) ❌

--- https://github.com/JuliaFolds/Transducers-data/blob/benchmark-results/2020/08/10/070744/result.md

Interestingly, FOLDL_RECURSION_LIMIT = Val(1) improves the performance:

ID time ratio memory ratio
["filter_sum", "1000", "RandomFloats", "noinit", "base"] 0.26 (5%) ✅ 1.00 (1%)

--- https://github.com/JuliaFolds/Transducers-data/blob/benchmark-results/2020/08/10/064257/result.md

Re: ["filter_sum", "1000", "RandomFloats", "noinit", "base"], see #403.

Commit Message

Improve compilation latency by reducing recursion limit (#412)

The tail-call pattern introduced in eb430cf
(#403) for linear indexing arrays seem to invoke a large compiler
latency (see #412 for an example). This patch tries to mitigate the
problem by reducing the FOLDL_RECURSION_LIMIT to 1. This still
allows two recursive calls. All the benchmarks do not show
noticeable degradation (presumably because recursion deeper than this
is not exercised in the benchmarks). Note that setting it to 0 (one
recursive call) shows 2x slowdown in ["teerf_filter", "noinit"]
benchmark.

@codecov
Copy link

codecov bot commented Aug 10, 2020

Codecov Report

Merging #413 into master will increase coverage by 1.31%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #413      +/-   ##
==========================================
+ Coverage   89.52%   90.84%   +1.31%     
==========================================
  Files          25       25              
  Lines        1585     1594       +9     
==========================================
+ Hits         1419     1448      +29     
+ Misses        166      146      -20     
Flag Coverage Δ
#unittests 90.84% <ø> (+1.31%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/processes.jl 93.03% <ø> (ø)
src/library.jl 93.15% <0.00%> (-0.28%) ⬇️
src/show.jl 89.51% <0.00%> (+0.07%) ⬆️
src/unordered.jl 96.61% <0.00%> (+0.31%) ⬆️
src/progress.jl 89.79% <0.00%> (+1.02%) ⬆️
src/core.jl 85.56% <0.00%> (+1.06%) ⬆️
src/groupby.jl 82.14% <0.00%> (+5.77%) ⬆️
src/reduce.jl 84.55% <0.00%> (+9.14%) ⬆️
src/basics.jl 92.10% <0.00%> (+10.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f67507...05bbd6a. Read the comment docs.

@tkf tkf changed the title Use: FOLDL_RECURSION_LIMIT = Val(1) Improve compilation latency by reducing recursion limit Aug 10, 2020
@tkf tkf marked this pull request as ready for review August 10, 2020 17:28
@mergify mergify bot merged commit 3a57454 into master Aug 10, 2020
@mergify mergify bot deleted the small-recursion-limit branch August 10, 2020 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants