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

Introduce a specialization threshold and try to avoid generated code #211

Merged
merged 1 commit into from
Nov 6, 2020

Conversation

quinnj
Copy link
Member

@quinnj quinnj commented Nov 6, 2020

Vastly improves #209. I actually thought this would trip us up sooner,
but kind of forgot about it. We had some generous use of @generated
code in a number of places, most aggregiously in
Tables.allocatecolumns and Tables.eachcolumn. It simplifies the code
to either just use a slow fallback, or skip the generated code entirely
by using macro expansion up to a certain limit. I tried to test out a
number of cases, comparing time to compile vs. resulting compiled code
performance, but it can be tricky to cover a wide variety of workflows.
I might try to generate a wider variety to compare benchmarks pre/post
this PR.

Vastly improves #209. I actually thought this would trip us up sooner,
but kind of forgot about it. We had some generous use of `@generated`
code in a number of places, most aggregiously in
`Tables.allocatecolumns` and `Tables.eachcolumn`. It simplifies the code
to either just use a slow fallback, or skip the generated code entirely
by using macro expansion up to a certain limit. I tried to test out a
number of cases, comparing time to compile vs. resulting compiled code
performance, but it can be tricky to cover a wide variety of workflows.
I might try to generate a wider variety to compare benchmarks pre/post
this PR.
@codecov
Copy link

codecov bot commented Nov 6, 2020

Codecov Report

Merging #211 into master will decrease coverage by 1.18%.
The diff coverage is 89.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #211      +/-   ##
==========================================
- Coverage   95.29%   94.10%   -1.19%     
==========================================
  Files           6        6              
  Lines         510      492      -18     
==========================================
- Hits          486      463      -23     
- Misses         24       29       +5     
Impacted Files Coverage Δ
src/Tables.jl 86.88% <ø> (ø)
src/tofromdatavalues.jl 100.00% <ø> (ø)
src/utils.jl 86.79% <85.71%> (-9.68%) ⬇️
src/namedtuples.jl 98.63% <92.30%> (-1.37%) ⬇️
src/fallbacks.jl 97.08% <100.00%> (+0.06%) ⬆️

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 2422eb2...719d09b. Read the comment docs.

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.

1 participant