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

feature request: option to switch off memory profiling #62

Closed
fabian-s opened this issue Oct 6, 2019 · 7 comments
Closed

feature request: option to switch off memory profiling #62

fabian-s opened this issue Oct 6, 2019 · 7 comments
Labels
feature a feature request or enhancement

Comments

@fabian-s
Copy link
Contributor

fabian-s commented Oct 6, 2019

since memory profiling does not seem to work reliably for parallelized code (see below and HenrikBengtsson/profmem#14), bench should allow switching off memory profiling or wrap parse_allocations in tryCatch so that timings are returned reliably even if memory does not work.

a no_memprof option would also be nice for #60.

reprex:

keep_busy <- function(n = 1e3) {
  r <- rnorm(n)
  p <- pnorm(r)
  q <- qnorm(p)
  o <- order(q)
}
bench::mark(parallel::mclapply(seq_len(1e3), keep_busy))
#> Error in parse(text = trace): <text>:1:158: unexpected symbol
#> 1: c("rnorm", "FUN", "lapply", "doTryCatch", "tryCatchOne", "tryCatchList", "tryCatch", "try", "sendMaster", "FUN", "lapply", "<Anonymous>", "eval", "eval", " "tryCatchOne
#>                                                                                                                                                                  ^

Created on 2019-10-06 by the reprex package (v0.3.0)

Session info
devtools::session_info()
#> ─ Session info ──────────────────────────────────────────────────────────
#>  setting  value                       
#>  version  R version 3.6.1 (2019-07-05)
#>  os       Linux Mint 19.2             
#>  system   x86_64, linux-gnu           
#>  ui       X11                         
#>  language en_US                       
#>  collate  en_US.UTF-8                 
#>  ctype    en_US.UTF-8                 
#>  tz       Europe/Berlin               
#>  date     2019-10-06                  
#> 
#> ─ Packages ──────────────────────────────────────────────────────────────
#>  package      * version date       lib source        
#>  assertthat     0.2.1   2019-03-21 [1] CRAN (R 3.6.1)
#>  backports      1.1.4   2019-04-10 [1] CRAN (R 3.6.1)
#>  bench          1.0.4   2019-09-06 [1] CRAN (R 3.6.1)
#>  callr          3.3.1   2019-07-18 [1] CRAN (R 3.6.1)
#>  cli            1.1.0   2019-03-19 [1] CRAN (R 3.6.1)
#>  codetools      0.2-16  2018-12-24 [4] CRAN (R 3.5.2)
#>  crayon         1.3.4   2017-09-16 [1] CRAN (R 3.6.1)
#>  desc           1.2.0   2018-05-01 [1] CRAN (R 3.6.1)
#>  devtools       2.1.0   2019-07-06 [1] CRAN (R 3.6.1)
#>  digest         0.6.20  2019-07-04 [1] CRAN (R 3.6.1)
#>  doParallel   * 1.0.15  2019-08-02 [1] CRAN (R 3.6.1)
#>  doSNOW       * 1.0.18  2019-07-27 [1] CRAN (R 3.6.1)
#>  evaluate       0.14    2019-05-28 [1] CRAN (R 3.6.1)
#>  fansi          0.4.0   2018-10-05 [1] CRAN (R 3.6.1)
#>  foreach      * 1.4.7   2019-07-27 [1] CRAN (R 3.6.1)
#>  fs             1.3.1   2019-05-06 [1] CRAN (R 3.6.1)
#>  future         1.14.0  2019-07-02 [1] CRAN (R 3.6.1)
#>  future.apply   1.3.0   2019-06-18 [1] CRAN (R 3.6.1)
#>  globals        0.12.4  2018-10-11 [1] CRAN (R 3.6.1)
#>  glue           1.3.1   2019-03-12 [1] CRAN (R 3.6.1)
#>  highr          0.8     2019-03-20 [1] CRAN (R 3.6.1)
#>  htmltools      0.3.6   2017-04-28 [1] CRAN (R 3.6.1)
#>  iterators    * 1.0.12  2019-07-26 [1] CRAN (R 3.6.1)
#>  knitr          1.24    2019-08-08 [1] CRAN (R 3.6.1)
#>  listenv        0.7.0   2018-01-21 [1] CRAN (R 3.6.1)
#>  magrittr       1.5     2014-11-22 [1] CRAN (R 3.6.1)
#>  memoise        1.1.0   2017-04-21 [1] CRAN (R 3.6.1)
#>  pillar         1.4.2   2019-06-29 [1] CRAN (R 3.6.1)
#>  pkgbuild       1.0.4   2019-08-05 [1] CRAN (R 3.6.1)
#>  pkgconfig      2.0.2   2018-08-16 [1] CRAN (R 3.6.1)
#>  pkgload        1.0.2   2018-10-29 [1] CRAN (R 3.6.1)
#>  prettyunits    1.0.2   2015-07-13 [1] CRAN (R 3.6.1)
#>  processx       3.4.1   2019-07-18 [1] CRAN (R 3.6.1)
#>  profmem        0.5.0   2018-01-30 [1] CRAN (R 3.6.1)
#>  ps             1.3.0   2018-12-21 [1] CRAN (R 3.6.1)
#>  R6             2.4.0   2019-02-14 [1] CRAN (R 3.6.1)
#>  Rcpp           1.0.2   2019-07-25 [1] CRAN (R 3.6.1)
#>  remotes        2.1.0   2019-06-24 [1] CRAN (R 3.6.1)
#>  rlang          0.4.0   2019-06-25 [1] CRAN (R 3.6.1)
#>  rmarkdown      1.15    2019-08-21 [1] CRAN (R 3.6.1)
#>  rprojroot      1.3-2   2018-01-03 [1] CRAN (R 3.6.1)
#>  sessioninfo    1.1.1   2018-11-05 [1] CRAN (R 3.6.1)
#>  snow         * 0.4-3   2018-09-14 [1] CRAN (R 3.6.1)
#>  stringi        1.4.3   2019-03-12 [1] CRAN (R 3.6.1)
#>  stringr        1.4.0   2019-02-10 [1] CRAN (R 3.6.1)
#>  testthat       2.2.1   2019-07-25 [1] CRAN (R 3.6.1)
#>  tibble         2.1.3   2019-06-06 [1] CRAN (R 3.6.1)
#>  usethis        1.5.1   2019-07-04 [1] CRAN (R 3.6.1)
#>  utf8           1.1.4   2018-05-24 [1] CRAN (R 3.6.1)
#>  vctrs          0.2.0   2019-07-05 [1] CRAN (R 3.6.1)
#>  withr          2.1.2   2018-03-15 [1] CRAN (R 3.6.1)
#>  xfun           0.9     2019-08-21 [1] CRAN (R 3.6.1)
#>  yaml           2.2.0   2018-07-25 [1] CRAN (R 3.6.1)
#>  zeallot        0.1.0   2018-01-28 [1] CRAN (R 3.6.1)
#> 
#> [1] /home/fabian-s/R/x86_64-pc-linux-gnu-library/3.6
#> [2] /usr/local/lib/R/site-library
#> [3] /usr/lib/R/site-library
#> [4] /usr/lib/R/library
@EmilRehnberg
Copy link

Thank you for suggesting this @fabian-s !
I think the bench::mark interface is really convenient for bench marking so I'd love this too instead of having to hack something together doing the same thing (just way worse).

@jimhester jimhester added the feature a feature request or enhancement label Oct 7, 2019
@jimhester
Copy link
Member

I probably won't be working on bench for a bit, but happy to review PRs if anyone wants to implement something in the meantime.

@fabian-s
Copy link
Contributor Author

fabian-s commented Oct 8, 2019

ok, fair enough -- if I were to implement this I would

  1. add a "memory = TRUE" argument to mark
  2. change can_profile_memory <- capabilities("profmem") to profile_memory <- capabilities("profmem") & memory
  3. not run the first for-loop in mark unless isTRUE(check) | memory
  4. in parse_allocations, wrap readRprofmem in try, and also return an empty_Rprofmem if it fails

does that seem reasonable?

@fabian-s
Copy link
Contributor Author

fabian-s commented Oct 8, 2019

PS: Note that to enable benchmarking for parallelized code, this comment seems to say you'd have to eventually use (the next version of) profmem instead of utils::RProfmem to actually fix the issue reliably. Do you want me to make that change as well, since it doesn't add a new dependency you don't already have anyways?

@jangorecki
Copy link

jangorecki commented Oct 15, 2019

Measuring memory is not only a problem for a parallelized code, it is not reliable because it tracks only R C allocVector[3]. Recent example: https://mobile.twitter.com/healthandstats/status/1182840075001819136
IMO it should be disabled by default.

@jimhester
Copy link
Member

jimhester commented Oct 16, 2019

I added a warning in the documentation to clarify what this tracks and avoid misinterpreting the results, I will add an option to control the profiling directly in the future, so this issue will remain open to track that.

@jimhester
Copy link
Member

Fixed by fce1e23

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

Successfully merging a pull request may close this issue.

4 participants