Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

For small files, don't recompile anything #284

Merged
merged 2 commits into from
Sep 20, 2018
Merged

For small files, don't recompile anything #284

merged 2 commits into from
Sep 20, 2018

Conversation

quinnj
Copy link
Member

@quinnj quinnj commented Sep 19, 2018

This was a change I had wanted to experiment with and @tk3369 prompted the work by showing how much over-compiling was costing us in #275. On the benchmarks we were running in that issue with this branch, we now see:

julia> perf()
2018-09-19T07:53:00.698 reading 50 x 1 file
  0.110446 seconds (112.06 k allocations: 5.198 MiB)
2018-09-19T07:53:00.928 reading 100 x 1 file
  0.005255 seconds (3.70 k allocations: 166.945 KiB)
2018-09-19T07:53:00.933 reading 150 x 1 file
  0.014252 seconds (5.58 k allocations: 237.680 KiB, 62.17% gc time)
2018-09-19T07:53:00.948 reading 200 x 1 file
  0.002573 seconds (7.96 k allocations: 361.148 KiB)
2018-09-19T07:53:00.95 reading 250 x 1 file
  0.002736 seconds (8.75 k allocations: 399.305 KiB)
2018-09-19T07:53:00.953 reading 300 x 1 file
  0.003196 seconds (10.42 k allocations: 461.695 KiB)
2018-09-19T07:53:00.957 reading 350 x 1 file
  0.003694 seconds (12.13 k allocations: 546.273 KiB)
2018-09-19T07:53:00.961 reading 400 x 1 file
  0.004209 seconds (13.83 k allocations: 612.805 KiB)
2018-09-19T07:53:00.965 reading 450 x 1 file
  0.006593 seconds (15.55 k allocations: 680.398 KiB)
2018-09-19T07:53:00.972 reading 500 x 1 file
  0.005080 seconds (17.29 k allocations: 747.898 KiB)
2018-09-19T07:53:00.978 reading 550 x 1 file
  0.005779 seconds (19.21 k allocations: 812.602 KiB)
2018-09-19T07:53:00.984 reading 600 x 1 file
  0.006613 seconds (23.38 k allocations: 1.069 MiB)
2018-09-19T07:53:00.99 reading 650 x 1 file
  0.006636 seconds (22.89 k allocations: 1.062 MiB)
2018-09-19T07:53:00.997 reading 700 x 1 file
  0.007651 seconds (24.83 k allocations: 1.130 MiB)
2018-09-19T07:53:01.005 reading 750 x 1 file
  0.007381 seconds (26.75 k allocations: 1.197 MiB)
2018-09-19T07:53:01.013 reading 800 x 1 file
  0.008498 seconds (28.70 k allocations: 1.266 MiB)
2018-09-19T07:53:01.022 reading 850 x 1 file
  0.009268 seconds (30.65 k allocations: 1.335 MiB)
2018-09-19T07:53:01.031 reading 900 x 1 file
  0.009300 seconds (32.60 k allocations: 1.404 MiB)
2018-09-19T07:53:01.041 reading 950 x 1 file
  0.012026 seconds (34.55 k allocations: 1.472 MiB)
2018-09-19T07:53:01.053 reading 1000 x 1 file
  0.009923 seconds (36.49 k allocations: 1.541 MiB)

@quinnj
Copy link
Member Author

quinnj commented Sep 19, 2018

We probably still need to experiment with the right threshold of when we switch from no-recompiling to specialized kernels for larger files.

@nalimilan
Copy link
Member

That sounds really nice, but it's impossible to spot what changes in the middle of moving code. Maybe make a first commit just to move code around?

@quinnj quinnj closed this Sep 19, 2018
@quinnj quinnj reopened this Sep 19, 2018
@codecov
Copy link

codecov bot commented Sep 20, 2018

Codecov Report

Merging #284 into master will increase coverage by 0.15%.
The diff coverage is 97.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
+ Coverage   94.56%   94.72%   +0.15%     
==========================================
  Files           7        7              
  Lines         607      625      +18     
==========================================
+ Hits          574      592      +18     
  Misses         33       33
Impacted Files Coverage Δ
src/typedetection.jl 97.75% <100%> (-0.03%) ⬇️
src/tables.jl 96.82% <100%> (+1.47%) ⬆️
src/CSV.jl 100% <100%> (ø) ⬆️
src/filedetection.jl 86.76% <87.5%> (ø) ⬆️
src/deprecated.jl 95.39% <0%> (-0.03%) ⬇️
src/validate.jl 94.44% <0%> (+0.32%) ⬆️

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 eb02e13...5f6f0e9. Read the comment docs.

@quinnj
Copy link
Member Author

quinnj commented Sep 20, 2018

Good idea @nalimilan, I pushed a commit to master that just did the code movement, and rebased this PR so the diff is much cleaner.

src/CSV.jl Outdated Show resolved Hide resolved
src/filedetection.jl Outdated Show resolved Hide resolved
@quinnj quinnj merged commit 8c9f8c2 into master Sep 20, 2018
@quinnj quinnj deleted the jq/fastsmall branch September 20, 2018 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants