Skip to content

Commit

Permalink
Merge branch 'feature/getExpression-performance' into develop
Browse files Browse the repository at this point in the history
  • Loading branch information
HenrikBengtsson committed Jan 19, 2024
2 parents b0189cf + fc45b92 commit f1da4a4
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 12 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Package: future
Version: 1.33.1
Version: 1.33.1-9001
Title: Unified Parallel and Distributed Processing in R for Everyone
Imports:
digest,
Expand Down
8 changes: 7 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
# Version (development version)

* ...
## Performance

* Decreased the overhead of launching futures that occurred for future
strategies that used a complex `workers` argument. For example,
`plan(cluster, workers = cl)`, where `cl` is a `cluster` object,
would come with an extra overhead, because the `workers` object was
unnecessarily transferred to the cluster nodes.


# Version 1.33.1 [2023-12-21]
Expand Down
24 changes: 14 additions & 10 deletions R/Future-class.R
Original file line number Diff line number Diff line change
Expand Up @@ -736,9 +736,15 @@ getExpression.Future <- local({
tmpl_enter_plan <- bquote_compile({
## covr: skip=2
.(enter)

## Record the original future strategy set on this worker
...future.strategy.old <- future::plan("list")

## Prevent 'future.plan' / R_FUTURE_PLAN settings from being nested
options(future.plan = NULL)
Sys.unsetenv("R_FUTURE_PLAN")

## Use the next-level-down ("popped") future strategy
future::plan(.(strategiesR), .cleanup = FALSE, .init = FALSE)
})

Expand All @@ -752,7 +758,8 @@ getExpression.Future <- local({
Sys.unsetenv("R_FUTURE_PLAN")
else
Sys.setenv(R_FUTURE_PLAN = .(oenv))
future::plan(.(strategies), .cleanup = FALSE, .init = FALSE)
## Revert to the original future strategy
future::plan(...future.strategy.old, .cleanup = FALSE, .init = FALSE)
## FIXME: If we move .(exit) here, then 'R CMD check' on MS Windows
## complain about leftover RscriptXXXXX temporary files. /2022-07-21
## .(exit)
Expand Down Expand Up @@ -797,9 +804,12 @@ getExpression.Future <- local({
## Pass down the default or the remain set of future strategies?
strategiesR <- strategies[-1]
## mdebugf("Number of remaining strategies: %d", length(strategiesR))

## Identify packages needed by the futures
if (length(strategiesR) > 0L) {

## Use default future strategy + identify packages needed by the futures
if (length(strategiesR) == 0L) {
if (debug) mdebug("Packages needed by future strategies (n = 0): <none>")
strategiesR <- "default"
} else {
## Identify package namespaces needed for strategies
pkgsS <- lapply(strategiesR, FUN = environment)
pkgsS <- lapply(pkgsS, FUN = environmentName)
Expand All @@ -808,8 +818,6 @@ getExpression.Future <- local({
pkgsS <- intersect(pkgsS, loadedNamespaces())
if (debug) mdebugf("Packages needed by future strategies (n = %d): %s", length(pkgsS), paste(sQuote(pkgsS), collapse = ", "))
pkgs <- unique(c(pkgs, pkgsS))
} else {
if (debug) mdebug("Packages needed by future strategies (n = 0): <none>")
}

## Make sure to load and attach all package needed
Expand All @@ -822,10 +830,6 @@ getExpression.Future <- local({
enter <- bquote_apply(tmpl_enter_packages)
}

## Make sure to set all nested future strategies needed
## Use default future strategy?
if (length(strategiesR) == 0L) strategiesR <- "default"

## Pass down future strategies
enter <- bquote_apply(tmpl_enter_plan)
exit <- bquote_apply(tmpl_exit_plan)
Expand Down

0 comments on commit f1da4a4

Please sign in to comment.