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

check run_dev() performance #1092

Closed
VincentGuyader opened this issue Aug 10, 2023 · 2 comments
Closed

check run_dev() performance #1092

VincentGuyader opened this issue Aug 10, 2023 · 2 comments

Comments

@VincentGuyader
Copy link
Member

in dev branch, the code :

  if (install_required_packages) {
    install_dev_deps("attachment", force_install = install_required_packages)
    to_install <- attachment::att_from_rscript(path = try_dev)
    install_dev_deps(dev_deps = to_install, force_install = install_required_packages)
  } 

is launched everytime someone launch run_dev().

how long does it take ? maybe try without by setting install_required_packages=FALSE by default, and suggest install_required_packages = TRUE if an error occur

@VincentGuyader VincentGuyader changed the title check run_dev.R performance check run_dev() performance Aug 10, 2023
@ilyaZar
Copy link
Contributor

ilyaZar commented Aug 10, 2023

yes!
I thought about filing another feature request with the exact same idea.

you want to be able to call golem::run_dev() repeatadly without setting any arguments (i.e. install_required_packages=FALSE all the time) because many people in a production setting might be reluctant to update

so I would vote for install_required_packages=FALSE

@ColinFay
Copy link
Member

As far as I can tell, the package installation will happen just once (if the package is not installed).

After that, the action is very quick (if there is no package to install), given that install_dev_deps() only perform the install if !rlang::is_installed(pak).

for (
    pak in dev_deps
  ) {
    if (!rlang::is_installed(pak)) {
      f(pak, ...)
    }
  }

I'm closing, as I'm not sure it's worth the added complexity of erroring + asking the user to manually install.

f <- \(install_required_packages = TRUE){
    if (install_required_packages) {
      golem:::install_dev_deps("attachment", force_install = install_required_packages)
      to_install <- attachment::att_from_rscript(path = "/private/tmp/deleteme/dev/run_dev.R")
      print(to_install)
      golem:::install_dev_deps(dev_deps = to_install, force_install = install_required_packages)
    }
}
bench::mark({
    f()
})

[1] "httpuv" "sass"   "golem" 
# A tibble: 1 × 13
  expression      min median `itr/sec` mem_alloc `gc/sec` n_itr  n_gc total_time
  <bch:expr> <bch:tm> <bch:>     <dbl> <bch:byt>    <dbl> <int> <dbl>   <bch:tm>
1 { f() }      2.22ms 2.77ms      314.    47.7KB     6.25   151     3      480ms
# ℹ 4 more variables: result <list>, memory <list>, time <list>, gc <list>

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

No branches or pull requests

3 participants