paws-r / paws

Paws, a package for Amazon Web Services in R
https://www.paws-r-sdk.com
Other
313 stars 37 forks source link

Cache Unix OS environmental variables #693

Closed DyfanJones closed 10 months ago

DyfanJones commented 10 months ago

get_env can be called up to 17 times with each operation. If on an unix system paws will call the system os environmental variables as last resort to find the environmental variable code link

# Get the value of an OS environment variable.
# NOTE: Does not work on Windows.
get_os_env <- function(var) {
  if (.Platform$OS.type == "unix") {
    value <- system(sprintf("echo $%s", var), intern = T)
  } else {
    value <- "" # Not implemented on Windows.
  }

  return(value)
}

This alone can be expensive (~5ms) and ultimately slow paws down (17x ~5ms ≈ 85ms). By caching os environmental variables onload we should get fairly big saving with little to no code changes.

image Figure: demoing performance gains for 17 calls to environmental variables

codecov[bot] commented 10 months ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (b0879ec) 84.11% compared to head (4372ad9) 84.03%.

:exclamation: Current head 4372ad9 differs from pull request most recent head 7577e96. Consider uploading reports for the commit 7577e96 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #693 +/- ## ========================================== - Coverage 84.11% 84.03% -0.08% ========================================== Files 201 204 +3 Lines 14533 14575 +42 ========================================== + Hits 12224 12248 +24 - Misses 2309 2327 +18 ``` [see 12 files with indirect coverage changes](https://app.codecov.io/gh/paws-r/paws/pull/693/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paws-r)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.