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

only freeze require world age in processess generating output #28290

Merged
merged 1 commit into from
Jul 26, 2018

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Jul 26, 2018

Alt. to #28274. Fixes #28042.

screen shot 2018-07-26 at 11 32 07

Precompilation time of CSV before:

 42.693428 seconds (3.82 M allocations: 232.627 MiB, 0.33% gc time)

After:

 44.479788 seconds (3.83 M allocations: 233.338 MiB, 0.42% gc time)

Would be good if people could try locally as well.

@ufechner7
Copy link

Did you also compare the time for loading the precompiled package?

@time using CSV

@pfitzseb
Copy link
Member

pfitzseb commented Jul 26, 2018

This does indeed fix the world age errors -- thanks @KristofferC :)

Performance does not seem to suffer too much from this.

With this fix:
precompiling CSV: 48.444573 seconds (4.84 M allocations: 281.981 MiB, 0.57% gc time)
using CSV: 1.787229 seconds (3.06 M allocations: 194.187 MiB, 11.02% gc time)

on 0.7.0-beta2.0:
precompiling CSV: 66.667827 seconds (4.72 M allocations: 293.743 MiB, 0.30% gc time)
using CSV: 2.267815 seconds (3.76 M allocations: 245.962 MiB, 7.88% gc time)

@KristofferC
Copy link
Member Author

On master I get

julia> @time using CSV
  1.543129 seconds (2.98 M allocations: 191.424 MiB, 8.26% gc time)

with this PR (but on top of #28118 because that is what I had available) I get

julia> @time using CSV
  1.431326 seconds (2.86 M allocations: 185.156 MiB, 2.72% gc time)

@ufechner7
Copy link

Performance looks good on my machine. :)

@KristofferC
Copy link
Member Author

Mac failure is FileWatching.

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.

4 participants