Skip to content

Commit 5835dae

Browse files
committed
Cache R CMD config values for the session
1 parent 836b3f9 commit 5835dae

7 files changed

Lines changed: 177 additions & 52 deletions

File tree

NEWS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# quickr (development version)
22

3+
* Repeated `quick()` compilations now reuse successful `R CMD config` results
4+
for the rest of the R session. Restart R after changing the compiler or
5+
Makevars configuration.
6+
37
# quickr 0.3.0
48

59
This release adds major new support for linear algebra, local functions,

R/aaa-utils.R

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,29 +54,69 @@ quickr_r_cmd <- function(
5454
r_cmd
5555
}
5656

57+
quickr_compiler_probe_cache <- new.env(parent = emptyenv())
58+
59+
quickr_cached_r_cmd_config_value <- function(
60+
name,
61+
cache = quickr_compiler_probe_cache
62+
) {
63+
stopifnot(is_string(name), is.environment(cache))
64+
65+
# Compiler configuration is fixed for the R session. Restart R after
66+
# changing the toolchain or its configuration files.
67+
cache_key <- paste("r_cmd_config", name, sep = "\r")
68+
if (exists(cache_key, envir = cache, inherits = FALSE)) {
69+
return(get(cache_key, envir = cache, inherits = FALSE))
70+
}
71+
72+
probe <- quickr_r_cmd_config_probe(name)
73+
if (isTRUE(probe$ok)) {
74+
assign(cache_key, probe$value, envir = cache)
75+
}
76+
probe$value
77+
}
78+
5779
quickr_r_cmd_config_value <- function(
5880
name,
5981
r_cmd = quickr_r_cmd(),
6082
system2 = base::system2
6183
) {
84+
quickr_r_cmd_config_probe(
85+
name = name,
86+
r_cmd = r_cmd,
87+
system2 = system2
88+
)$value
89+
}
90+
91+
quickr_r_cmd_config_probe <- function(
92+
name,
93+
r_cmd = quickr_r_cmd(),
94+
system2 = base::system2
95+
) {
96+
stopifnot(is_string(name), is_string(r_cmd), is.function(system2))
97+
6298
out <- tryCatch(
6399
suppressWarnings(system2(
64100
r_cmd,
65101
c("CMD", "config", name),
66102
stdout = TRUE,
67103
stderr = FALSE
68104
)),
69-
error = function(e) character()
105+
error = function(e) structure(character(), status = 1L)
70106
)
71107
status <- attr(out, "status")
72108
if (!is.null(status)) {
73-
return("")
109+
return(list(value = "", ok = FALSE))
74110
}
75111
value <- trimws(paste(out, collapse = " "))
76-
if (!nzchar(value) || grepl("^ERROR:", value)) {
77-
return("")
112+
if (!nzchar(value)) {
113+
return(list(value = "", ok = TRUE))
78114
}
79-
value
115+
if (grepl("^ERROR:", value)) {
116+
return(list(value = "", ok = FALSE))
117+
}
118+
119+
list(value = value, ok = TRUE)
80120
}
81121

82122

R/compiler.R

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ quickr_prefer_flang <- function(sysname = Sys.info()[["sysname"]]) {
165165
}
166166

167167
quickr_default_fortran_makevars_lines <- function(
168-
config_value = quickr_r_cmd_config_value
168+
config_value = quickr_cached_r_cmd_config_value
169169
) {
170170
fc <- trimws(config_value("FC"))
171171
if (!nzchar(fc)) {
@@ -191,7 +191,7 @@ quickr_fcompiler_env <- function(
191191
sysname = Sys.info()[["sysname"]],
192192
use_openmp = FALSE,
193193
link_flags = character(),
194-
config_value = quickr_r_cmd_config_value
194+
config_value = quickr_cached_r_cmd_config_value
195195
) {
196196
stopifnot(is.character(build_dir), length(build_dir) == 1L, nzchar(build_dir))
197197

R/parallel.R

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -173,25 +173,16 @@ openmp_directives <- function(parallel, private = NULL) {
173173
)
174174
}
175175

176-
openmp_config_value <- local({
177-
cached <- NULL
178-
179-
function(name, config_value = quickr_r_cmd_config_value) {
180-
if (is.null(cached)) {
181-
cached <<- list()
182-
}
183-
cached_value <- cached[[name]]
184-
if (!is.null(cached_value)) {
185-
return(cached_value)
186-
}
187-
value <- config_value(name)
188-
if (!nzchar(value)) {
189-
value <- ""
190-
}
191-
cached[[name]] <<- value
192-
value
176+
openmp_config_value <- function(
177+
name,
178+
config_value = quickr_cached_r_cmd_config_value
179+
) {
180+
value <- config_value(name)
181+
if (!nzchar(value)) {
182+
value <- ""
193183
}
194-
})
184+
value
185+
}
195186

196187
openmp_fflags <- function() {
197188
env_flags <- trimws(Sys.getenv("QUICKR_OPENMP_FFLAGS", ""))

R/quick.R

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ compile <- function(fsub, build_dir = tempfile(paste0(fsub@name, "-build-"))) {
213213

214214
# Link against the same BLAS/LAPACK/Fortran libs as the running R
215215
# to support generated calls to vendor BLAS (e.g., dgemm, dgesv).
216-
cfg <- quickr_r_cmd_config_value
216+
cfg <- quickr_cached_r_cmd_config_value
217217
BLAS_LIBS <- strsplit(cfg("BLAS_LIBS"), "[[:space:]]+")[[1]]
218218
LAPACK_LIBS <- strsplit(cfg("LAPACK_LIBS"), "[[:space:]]+")[[1]]
219219
FLIBS <- strsplit(cfg("FLIBS"), "[[:space:]]+")[[1]]
@@ -327,7 +327,7 @@ compile <- function(fsub, build_dir = tempfile(paste0(fsub@name, "-build-"))) {
327327
quickr_windows_add_dll_paths <- function(
328328
flags,
329329
os_type = .Platform$OS.type,
330-
config_value = quickr_r_cmd_config_value,
330+
config_value = quickr_cached_r_cmd_config_value,
331331
which = Sys.which
332332
) {
333333
if (!identical(os_type, "windows")) {

tests/testthat/test-compiler.R

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,35 @@
22

33
skip_on_cran()
44

5+
local_empty_compiler_probe_cache <- function(envir = parent.frame()) {
6+
cache <- get0(
7+
"quickr_compiler_probe_cache",
8+
envir = asNamespace("quickr"),
9+
inherits = FALSE
10+
)
11+
if (is.null(cache)) {
12+
return(invisible(NULL))
13+
}
14+
15+
old <- as.list.environment(cache, all.names = TRUE)
16+
withr::defer(
17+
{
18+
entries <- ls(envir = cache, all.names = TRUE)
19+
if (length(entries)) {
20+
rm(list = entries, envir = cache)
21+
}
22+
list2env(old, envir = cache)
23+
},
24+
envir = envir
25+
)
26+
27+
entries <- ls(envir = cache, all.names = TRUE)
28+
if (length(entries)) {
29+
rm(list = entries, envir = cache)
30+
}
31+
invisible(NULL)
32+
}
33+
534
test_that("quickr_r_cmd_config_value captures only stdout", {
635
expect_identical(
736
deparse(formals(quickr:::quickr_r_cmd_config_value)$system2),
@@ -49,6 +78,28 @@ test_that("quickr_r_cmd_config_value returns empty on command failure", {
4978
)
5079
})
5180

81+
test_that("R CMD config probe distinguishes empty values from errors", {
82+
empty_value <- function(...) character()
83+
expect_identical(
84+
quickr:::quickr_r_cmd_config_probe(
85+
"CC",
86+
r_cmd = "R",
87+
system2 = empty_value
88+
),
89+
list(value = "", ok = TRUE)
90+
)
91+
92+
error_value <- function(...) "ERROR: no information for variable 'CC'"
93+
expect_identical(
94+
quickr:::quickr_r_cmd_config_probe(
95+
"CC",
96+
r_cmd = "R",
97+
system2 = error_value
98+
),
99+
list(value = "", ok = FALSE)
100+
)
101+
})
102+
52103
test_that("quickr_flang_path prefers flang-new", {
53104
which_stub <- function(x) {
54105
if (x == "flang-new") {
@@ -459,3 +510,67 @@ test_that("compile cleans existing build directories and reports failures", {
459510
expect_true(dir.exists(build_dir))
460511
expect_false(file.exists(file.path(build_dir, "stale.txt")))
461512
})
513+
514+
test_that("quick reuses successful R CMD config probes", {
515+
local_empty_compiler_probe_cache()
516+
withr::local_options(quickr.fortran_compiler = "gfortran")
517+
518+
probe <- quickr_r_cmd_config_probe
519+
events <- list()
520+
local_mocked_bindings(
521+
quickr_r_cmd_config_probe = function(name, ...) {
522+
result <- probe(name, ...)
523+
events[[length(events) + 1L]] <<- list(name = name, ok = result$ok)
524+
result
525+
},
526+
.package = "quickr"
527+
)
528+
529+
fn <- function(x) {
530+
declare(type(x = double(1)))
531+
x + 1
532+
}
533+
534+
expect_quick_identical(fn, 1)
535+
first_events <- events
536+
expect_quick_identical(fn, 3)
537+
538+
successful <- unique(vapply(
539+
first_events[vapply(first_events, `[[`, logical(1), "ok")],
540+
`[[`,
541+
character(1),
542+
"name"
543+
))
544+
later <- events[seq_along(events) > length(first_events)]
545+
later_names <- vapply(later, `[[`, character(1), "name")
546+
expect_length(intersect(successful, later_names), 0L)
547+
})
548+
549+
test_that("quick retries failed R CMD config probes", {
550+
local_empty_compiler_probe_cache()
551+
withr::local_options(quickr.fortran_compiler = "gfortran")
552+
553+
probe <- quickr_r_cmd_config_probe
554+
blas_calls <- 0L
555+
local_mocked_bindings(
556+
quickr_r_cmd_config_probe = function(name, ...) {
557+
if (identical(name, "BLAS_LIBS")) {
558+
blas_calls <<- blas_calls + 1L
559+
if (blas_calls == 1L) {
560+
return(list(value = "", ok = FALSE))
561+
}
562+
}
563+
probe(name, ...)
564+
},
565+
.package = "quickr"
566+
)
567+
568+
fn <- function(x) {
569+
declare(type(x = double(1)))
570+
x + 1
571+
}
572+
573+
expect_quick_identical(fn, 1)
574+
expect_quick_identical(fn, 3)
575+
expect_identical(blas_calls, 2L)
576+
})

tests/testthat/test-openmp-utils.R

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -39,31 +39,6 @@ test_that("openmp_makevars_lines errors when linker flags are missing", {
3939
)
4040
})
4141

42-
test_that("openmp_config_value caches toolchain lookups", {
43-
cache_env <- environment(quickr:::openmp_config_value)
44-
old_cache <- cache_env$cached
45-
old_config <- cache_env$quickr_r_cmd_config_value
46-
withr::defer(cache_env$cached <- old_cache)
47-
withr::defer({
48-
if (is.null(old_config)) {
49-
rm(quickr_r_cmd_config_value, envir = cache_env)
50-
} else {
51-
cache_env$quickr_r_cmd_config_value <- old_config
52-
}
53-
})
54-
cache_env$cached <- NULL
55-
56-
calls <- 0
57-
cache_env$quickr_r_cmd_config_value <- function(...) {
58-
calls <<- calls + 1
59-
"value"
60-
}
61-
62-
expect_equal(quickr:::openmp_config_value("QUICKR_TEST_CACHE"), "value")
63-
expect_equal(quickr:::openmp_config_value("QUICKR_TEST_CACHE"), "value")
64-
expect_equal(calls, 1)
65-
})
66-
6742
test_that("get_pending_parallel returns NULL for NULL or non-scope", {
6843
expect_null(quickr:::get_pending_parallel(NULL))
6944
expect_null(quickr:::get_pending_parallel(list()))

0 commit comments

Comments
 (0)