Skip to content

Commit 7f1fb9f

Browse files
kevinusheykevinushey
authored
re-export tbbmalloc from RcppParallel.dll on windows (#262)
* re-export tbbmalloc from RcppParallel.dll on windows downstream packages on windows link with '-lRcppParallel' alone, and so can only use tbb symbols that RcppParallel.dll exports. the tbb runtime symbols come along implicitly -- RcppParallel references them, so those archive members are linked in and re-exported via their '-export:' directives -- but nothing references the tbbmalloc entry points, so packages using the scalable allocator (e.g. via RcppArmadillo's ARMA_USE_TBB_ALLOC) fail to link with undefined references to scalable_malloc and scalable_free. link tbbmalloc with --whole-archive so its objects are always included and re-exported. tbbmalloc must precede tbb on the link line: both archives bundle an itt_notify object defining the same symbols, and with tbbmalloc's copy already linked, tbb's is never pulled in. older (non-oneTBB) toolchains like rtools42 keep the plain link: there, tbb and tbbmalloc both define DllMain and so cannot be linked wholesale, and their objects carry no '-export:' directives to re-export anyhow. * add test for downstream use of the scalable allocator generates a small package using scalable_malloc and scalable_free via the RcppParallel::CxxFlags() / RcppParallelLibs() flags, installs it, and confirms the allocator can be used. on windows, this exercises the tbbmalloc re-export from RcppParallel.dll; elsewhere, the run-time resolution of tbbmalloc symbols loaded by RcppParallel::.onLoad(). the test builds a package, so it runs only on CI (or with NOT_CRAN), and only when the tbb backend is enabled. * clear R_TESTS when building the test package R CMD check sets R_TESTS=startup.Rs when running tests, which breaks R sub-processes run with a different working directory. the Rscript invocations in the test package's Makevars failed at startup because of this, expanding CxxFlags() and RcppParallelLibs() to nothing, so compilation failed with tbb/scalable_allocator.h not found.
1 parent d637cbe commit 7f1fb9f

3 files changed

Lines changed: 154 additions & 7 deletions

File tree

‎NEWS.md‎

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

3+
* Fixed linking of downstream packages using the TBB scalable allocator
4+
on Windows, e.g. via RcppArmadillo's `ARMA_USE_TBB_ALLOC`. RcppParallel
5+
now links the whole Rtools `tbbmalloc` archive into `RcppParallel.dll`
6+
and re-exports its API, so that `scalable_malloc`, `scalable_free`, and
7+
friends can be resolved by packages linking with `-lRcppParallel`.
8+
39
* Fixed installation on Windows toolchains providing an older (non-oneTBB)
410
copy of TBB, e.g. Rtools42: the tbb stub library is now built by
511
re-exporting the static TBB library, rather than wrapping the oneTBB

‎tests/scalable-allocator.R‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
2+
# Check that packages using TBB's scalable allocator can be compiled, linked,
3+
# and loaded against this installation of RcppParallel.
4+
#
5+
# On Windows, downstream packages link only '-lRcppParallel', and so this
6+
# requires RcppParallel.dll to re-export the tbbmalloc API on their behalf.
7+
# On other platforms, the symbols are left undefined at link time, and
8+
# resolved from the tbbmalloc library loaded when RcppParallel is loaded.
9+
#
10+
# https://github.com/RcppCore/RcppParallel/pull/262
11+
12+
library(RcppParallel)
13+
14+
# building the test package requires a toolchain, so keep this test
15+
# off of CRAN's machines
16+
ci <- nzchar(Sys.getenv("CI")) || identical(Sys.getenv("NOT_CRAN"), "true")
17+
if (!ci) {
18+
writeLines("Not running on CI; skipping scalable allocator test.")
19+
quit(save = "no")
20+
}
21+
22+
# the scalable allocator is only available with the TBB backend
23+
if (!RcppParallel:::TBB_ENABLED) {
24+
writeLines("TBB is not enabled; skipping scalable allocator test.")
25+
quit(save = "no")
26+
}
27+
28+
# generate the test package
29+
pkgRoot <- file.path(tempdir(), "scalabletest")
30+
dir.create(file.path(pkgRoot, "src"), recursive = TRUE, showWarnings = FALSE)
31+
32+
writeLines(con = file.path(pkgRoot, "DESCRIPTION"), c(
33+
"Package: scalabletest",
34+
"Type: Package",
35+
"Title: Test Linking to the TBB Scalable Allocator",
36+
"Version: 0.1.0",
37+
"Author: RcppParallel Authors",
38+
"Maintainer: RcppParallel Authors <noreply@example.com>",
39+
"Description: Confirms that packages can use the TBB scalable allocator.",
40+
"License: GPL-2",
41+
"Imports: RcppParallel"
42+
))
43+
44+
writeLines(con = file.path(pkgRoot, "NAMESPACE"), c(
45+
"useDynLib(scalabletest)",
46+
"importFrom(RcppParallel, RcppParallelLibs)"
47+
))
48+
49+
writeLines(con = file.path(pkgRoot, "src", "Makevars"), c(
50+
'PKG_CXXFLAGS = $(shell "${R_HOME}/bin/Rscript" -e "RcppParallel::CxxFlags()")',
51+
'PKG_LIBS = $(shell "${R_HOME}/bin/Rscript" -e "RcppParallel::RcppParallelLibs()")'
52+
))
53+
54+
writeLines(con = file.path(pkgRoot, "src", "Makevars.win"), c(
55+
'PKG_CXXFLAGS = $(shell "${R_HOME}/bin${R_ARCH_BIN}/Rscript.exe" -e "RcppParallel::CxxFlags()")',
56+
'PKG_LIBS = $(shell "${R_HOME}/bin${R_ARCH_BIN}/Rscript.exe" -e "RcppParallel::RcppParallelLibs()")'
57+
))
58+
59+
writeLines(con = file.path(pkgRoot, "src", "scalable.cpp"), c(
60+
"#include <tbb/scalable_allocator.h>",
61+
"",
62+
"#include <R.h>",
63+
"#include <Rinternals.h>",
64+
"",
65+
'extern "C" SEXP scalable_roundtrip(SEXP sizeSEXP)',
66+
"{",
67+
" int size = Rf_asInteger(sizeSEXP);",
68+
"",
69+
" double* data = (double*) scalable_malloc(size * sizeof(double));",
70+
" if (data == NULL)",
71+
" return Rf_ScalarLogical(FALSE);",
72+
"",
73+
" for (int i = 0; i < size; i++)",
74+
" data[i] = i;",
75+
"",
76+
" double total = 0.0;",
77+
" for (int i = 0; i < size; i++)",
78+
" total += data[i];",
79+
"",
80+
" scalable_free(data);",
81+
"",
82+
" double expected = (double) size * (size - 1) / 2;",
83+
" return Rf_ScalarLogical(total == expected);",
84+
"}"
85+
))
86+
87+
# install it, making sure child processes resolve this RcppParallel
88+
libDir <- file.path(tempdir(), "library")
89+
dir.create(libDir, recursive = TRUE, showWarnings = FALSE)
90+
Sys.setenv(R_LIBS = paste(.libPaths(), collapse = .Platform$path.sep))
91+
92+
# R CMD check sets R_TESTS=startup.Rs, which breaks R sub-processes run
93+
# with a different working directory -- like the Rscript invocations in
94+
# the test package's Makevars (see also tests/doRUnit.R)
95+
Sys.setenv(R_TESTS = "")
96+
97+
rExe <- file.path(R.home("bin"), if (.Platform$OS.type == "windows") "R.exe" else "R")
98+
args <- c("CMD", "INSTALL", "--no-multiarch", paste0("--library=", shQuote(libDir)), shQuote(pkgRoot))
99+
100+
output <- suppressWarnings(system2(rExe, args, stdout = TRUE, stderr = TRUE))
101+
writeLines(output)
102+
103+
status <- attr(output, "status")
104+
if (is.numeric(status) && status != 0L)
105+
stop("error installing test package (status code ", status, ")")
106+
107+
# load it, and check that the scalable allocator can be used
108+
invisible(loadNamespace("scalabletest", lib.loc = libDir))
109+
ok <- .Call("scalable_roundtrip", 1000L, PACKAGE = "scalabletest")
110+
writeLines(paste("scalable allocator round trip:", if (isTRUE(ok)) "OK" else "FAILED"))
111+
stopifnot(isTRUE(ok))

‎tools/config/configure.R‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -224,19 +224,49 @@ define(
224224
)
225225

226226
# set PKG_LIBS
227-
pkgLibs <- if (!is.na(tbbLib)) {
228-
227+
pkgLibs <- if (.Platform$OS.type == "windows") {
228+
229+
if (!is.na(tbbLib) && file.exists(file.path(tbbInc, "oneapi"))) {
230+
231+
# downstream packages link with '-lRcppParallel' alone, so
232+
# RcppParallel.dll must provide the tbbmalloc API (scalable_malloc
233+
# and friends) even though RcppParallel itself never calls it; use
234+
# --whole-archive so those objects are linked in, and re-exported
235+
# via their '-export:' directives. tbbmalloc must precede tbb here:
236+
# both archives bundle an itt_notify object defining the same
237+
# symbols, and with tbbmalloc's copy already linked, tbb's is never
238+
# pulled in, avoiding duplicate definition errors
239+
c(
240+
"-Wl,-L\"$(TBB_LIB)\"",
241+
"-Wl,--whole-archive",
242+
"-l$(TBB_MALLOC_NAME)",
243+
"-Wl,--no-whole-archive",
244+
"-l$(TBB_NAME)"
245+
)
246+
247+
} else if (!is.na(tbbLib)) {
248+
249+
# with an older (non-oneTBB) toolchain like Rtools42, tbb and
250+
# tbbmalloc both define DllMain, so tbbmalloc cannot be linked
251+
# wholesale; its objects also carry no '-export:' directives, so
252+
# nothing would be re-exported anyhow -- just link as needed
253+
c(
254+
"-Wl,-L\"$(TBB_LIB)\"",
255+
"-l$(TBB_NAME)",
256+
"-l$(TBB_MALLOC_NAME)"
257+
)
258+
259+
}
260+
261+
} else if (!is.na(tbbLib)) {
262+
229263
c(
230264
"-Wl,-L\"$(TBB_LIB)\"",
231265
sprintf("-Wl,-rpath,%s", shQuote(tbbLib)),
232266
"-l$(TBB_NAME)",
233267
"-l$(TBB_MALLOC_NAME)"
234268
)
235-
236-
} else if (.Platform$OS.type == "windows") {
237-
238-
NULL
239-
269+
240270
} else if (R.version$os == "emscripten") {
241271

242272
c(

0 commit comments

Comments
 (0)