From 063ec92c94281c6c1a7c9f35005266e496b3e5dd Mon Sep 17 00:00:00 2001 From: Henrik Bengtsson Date: Fri, 19 Jan 2024 13:25:21 -0800 Subject: [PATCH 1/2] PERFORMANCE: Avoid passing down top-level future strategy (i.e. plan('next')) to future. This avoids some data-transfer overhead that may happen for some strategies [#713] --- DESCRIPTION | 4 ++-- R/Future-class.R | 24 ++++++++++++++---------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 817f8d70..11ff2b2e 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -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, @@ -39,5 +39,5 @@ LazyLoad: TRUE ByteCompile: TRUE URL: https://future.futureverse.org, https://github.com/HenrikBengtsson/future BugReports: https://github.com/HenrikBengtsson/future/issues -RoxygenNote: 7.2.3 +RoxygenNote: 7.3.0 Roxygen: list(markdown = TRUE) diff --git a/R/Future-class.R b/R/Future-class.R index 479eb5ce..78386d17 100644 --- a/R/Future-class.R +++ b/R/Future-class.R @@ -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) }) @@ -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) @@ -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): ") + strategiesR <- "default" + } else { ## Identify package namespaces needed for strategies pkgsS <- lapply(strategiesR, FUN = environment) pkgsS <- lapply(pkgsS, FUN = environmentName) @@ -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): ") } ## Make sure to load and attach all package needed @@ -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) From 3cbdb567904aa9e2d8814beb624015783a098681 Mon Sep 17 00:00:00 2001 From: Henrik Bengtsson Date: Fri, 19 Jan 2024 15:08:09 -0800 Subject: [PATCH 2/2] NEWS: Mention performance improvement [ci skip] --- NEWS.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 16e1e2c9..b56cbe31 100644 --- a/NEWS.md +++ b/NEWS.md @@ -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]