Fix a few allocation/inference issues #885
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was from an effort to play around with precompilation some more. There's a tricky balance where if we precompile too much of `CSV.File`, then we run into the Base bug where there are Base methods missing and cause painful allocations via invoke (JuliaLang/julia#35972). The state of precompilation for CSV.jl is thus pretty disappointing: precompiling currently takes ~7.5s (which is fine, this is the number we're fine to have keep growing), loading CSV.jl precompiled takes ~2.5s (seems a little high, but not terrible), but the first call to `CSV.File` takes ~6.5s :sad_face:. At this point, I've spent too much time and don't understand the ins and outs of precompilation interactions with later runtime code to debug further, but hopefully we can recruit help from some experts to take CSV.jl precompilation to the next level. As-is, it's workable, so let's go with this for now and get this darn release out.
Codecov Report
@@ Coverage Diff @@
## main #885 +/- ##
==========================================
- Coverage 90.83% 90.66% -0.18%
==========================================
Files 9 9
Lines 2139 2142 +3
==========================================
- Hits 1943 1942 -1
- Misses 196 200 +4
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was from an effort to play around with precompilation some more.
There's a tricky balance where if we precompile too much of
CSV.File,then we run into the Base bug where there are Base methods missing and
cause painful allocations via invoke
(JuliaLang/julia#35972). The state of
precompilation for CSV.jl is thus pretty disappointing: precompiling
currently takes ~7.5s (which is fine, this is the number we're fine to
have keep growing), loading CSV.jl precompiled takes ~2.5s (seems a
little high, but not terrible), but the first call to
CSV.Filetakes~6.5s :sad_face:. At this point, I've spent too much time and don't
understand the ins and outs of precompilation interactions with later
runtime code to debug further, but hopefully we can recruit help from
some experts to take CSV.jl precompilation to the next level. As-is,
it's workable, so let's go with this for now and get this darn release
out.