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

Generate precompile statements for Julia 1.7 #2955

Merged
merged 1 commit into from
Dec 3, 2021
Merged

Conversation

bkamins
Copy link
Member

@bkamins bkamins commented Dec 2, 2021

Fixes #2921

@nalimilan - I have removed the extra precompile statements for all=true to reduce the size of the code parsed (it slightly reduces the load time) and since it was only used in H2O tests which are dead now (I have retained some minimal things that are done when all=true but they are minor).

@bkamins bkamins linked an issue Dec 2, 2021 that may be closed by this pull request
@bkamins bkamins added the CI label Dec 2, 2021
@bkamins bkamins added this to the 1.3 milestone Dec 2, 2021
@bkamins
Copy link
Member Author

bkamins commented Dec 2, 2021

Compilation time of 1.3 release (first timing is with precompilation):

$ julia -e "@time using DataFrames"
 16.620894 seconds (1.89 M allocations: 131.628 MiB, 0.18% gc time, 0.11% compilation time)

bogum@DESKTOP-4FNINJ9 C:\WORK
$ julia -e "@time using DataFrames"
  1.108809 seconds (1.84 M allocations: 129.151 MiB, 1.34% gc time, 0.67% compilation time)

bogum@DESKTOP-4FNINJ9 C:\WORK
$ julia -e "@time using DataFrames"
  1.113932 seconds (1.84 M allocations: 129.147 MiB, 1.33% gc time, 0.64% compilation time)

bogum@DESKTOP-4FNINJ9 C:\WORK
$ julia -e "@time using DataFrames"
  1.075805 seconds (1.84 M allocations: 129.151 MiB, 1.38% gc time, 0.72% compilation time)

vs 1.2.2 (first timing is with precompilation):

$ julia -e "@time using DataFrames"
 24.227153 seconds (1.92 M allocations: 140.367 MiB, 0.14% gc time, 0.08% compilation time)

$ julia -e "@time using DataFrames"
  1.164665 seconds (1.90 M allocations: 138.797 MiB, 2.87% gc time, 0.64% compilation time)

$ julia -e "@time using DataFrames"
  1.124894 seconds (1.90 M allocations: 138.797 MiB, 3.13% gc time, 0.68% compilation time)

$ julia -e "@time using DataFrames"
  1.173578 seconds (1.90 M allocations: 138.801 MiB, 2.98% gc time, 0.65% compilation time)

(and I think it is worth to reduce the all part to get those smaller load times)

@bkamins bkamins closed this Dec 2, 2021
@bkamins bkamins reopened this Dec 2, 2021
@nalimilan
Copy link
Member

You mean that statements that are only used when all=true make using DataFrames slower, even after the initial precompilation? I find it surprising (and really bad).

@bkamins
Copy link
Member Author

bkamins commented Dec 3, 2021

I have run the test on DataFrames 1.2.2.

With all=true precompilation statements:

$ julia -e "@time using DataFrames"
 25.981184 seconds (1.94 M allocations: 141.029 MiB, 0.13% gc time, 0.08% compilation time)

and now manually in DataFrames 1.2.2 I removed these statements:

$ julia -e "@time using DataFrames"
 20.441751 seconds (1.93 M allocations: 139.962 MiB, 0.18% gc time, 0.11% compilation time)

(these are times of first run including pre-compilation)

@bkamins
Copy link
Member Author

bkamins commented Dec 3, 2021

After precompilation the relevant thing is number of allocations (as timings are naturally somewhat variable).

In 1.2.2 with removed all=true precompilation statements:

1.90 M allocations: 137.934 MiB

and when keeping these statements:

1.91 M allocations: 139.007 MiB

so it is a bit better to remove them (not much but bettet)

@bkamins
Copy link
Member Author

bkamins commented Dec 3, 2021

I have run more tests and the difference in compilation time after first precompilation is very small, so it can be ignored. Still I would also prefer to reduce precompilation time + not have so much code in the package that is not used.

@nalimilan
Copy link
Member

OK. I guess we'll see whether H2Oai or another benchmark would benefit from adding more precompile statements in the future.

@bkamins bkamins merged commit 12c586c into main Dec 3, 2021
@bkamins bkamins deleted the bk/prepare_release branch December 3, 2021 16:55
@bkamins
Copy link
Member Author

bkamins commented Dec 3, 2021

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataFrames errors on loading with --depwarn=error Regenerate precompile statements for 1.3 release
2 participants