traitecoevo / plant

Trait-Driven Models of Ecology and Evolution :evergreen_tree:
https://traitecoevo.github.io/plant
53 stars 20 forks source link

Avoid full rebuild on every change #312

Closed dfalster closed 2 years ago

dfalster commented 2 years ago

It feels like too much stuff is recompiled whenever we make a change. We want to avoid that.

Address this together with #311 as will be easier to address with fewer warnings

dfalster commented 2 years ago

With @devmitch working on #311, I thought I'd look agin at this. Some progress. Be great if you can weigh in @devmitch.

The problem is that whenever we change some code, the entire project seems to rebuild, which given it takes some time, makes the dev cycle slower and more painful than it should be.

There seem to be several reasons for this, some of which we can improve.

First, the bad news. A lot of the code is in headers, which depend on each other. The slowest piece to build is RcppExports.cpp, which calls on "../inst/include/plant.h", which calls on pretty much all other headers. So any change in any header (i.e. .h / .hpp) will cause RcppExports.cpp and many other objects to rebuild.

BUT, there's more rebuilding going on than is triggered by the above. And it seems this has two different triggers, with common cause.

  1. In the makefile, the compile command calls pkgbuild::compile_dll(). This call causes RcppExports.cpp to be regenerated anytime any of the code anywhere changes. So RcppExports.cpp is often being recompiled unnecessarily. (also creation of RcppExports.cpp prints a lot of noise to console). Changing the compile command to use pkgbuild::compile_dll(compile_attributes = FALSE) means RcppExports.cpp is not generated, so you only recompile what's needed. So this improves things when calling make from terminal or calling pkgbuild::compile_dll(compile_attributes = FALSE) in the R console.

  2. Calling devtools:load_all() in the r console (or any of the Rstudio build buttons), eventually makes a call to pkgbuild::compile_dll(), causing a rebuild of RcppExports.cpp as above. Only this time you can't avoid recreating RcppExports.cpp, as there's no possibility to pass argument compile_attributes = FALSE. A solution is to run pkgbuild::compile_dll(compile_attributes = FALSE); devtools::load_all()

Thoughts @aornugent @devmitch?

devmitch commented 2 years ago

Changing the compile command to use pkgbuild::compile_dll(compile_attributes = FALSE) ...

I'm guessing that if you do this, then you would need another command for when you explicitly want to regenerate RcppExports.cpp? When I try make attributes the output isn't the same as all the noise that appears when pkgbuild::compile_dll() is called without any extra flags, though I didn't try change any function signatures or anything.

A solution is to run pkgbuild::compile_dll(compile_attributes = FALSE); devtools::load_all()

Would it be possible to put this in a script maybe? To run it though the Rstudio interface (or at least through the console).

dfalster commented 2 years ago

I'm guessing that if you do this, then you would need another command for when you explicitly want to regenerate RcppExports.cpp Yes! make attributes seems to do that,

When I try make attributes the output isn't the same as all the noise that appears That difference may not be important. I don't know why it is so noisy.

From my inspection, dev tools::load_all seems to call pkgload::load_all, which calls pkgbuild::compile_dll, which has argument compile_attributes = pkg_links_to_cpp11(path) || pkg_links_to_rcpp(path, where those functions are from pkgbuild. These will return true, based on the presence of LinkingTo: Rcpp in the file DESCRIPTION.

Would it be possible to put this in a script maybe? To run it though the Rstudio interface (or at least through the console). Yes, but not sure it's worth it for one line?

aornugent commented 2 years ago

My new workflow is:

For changes to Rcpp In a terminal:

make RcppR6
make compile

In R

devtools::load_all

For changes to C++ (not Rcpp) In a terminal:

make compile

In R

devtools::load_all()