Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable building with the system tbb library #107

Closed
bryteise opened this issue Mar 2, 2020 · 7 comments · Fixed by #141
Closed

Enable building with the system tbb library #107

bryteise opened this issue Mar 2, 2020 · 7 comments · Fixed by #141

Comments

@bryteise
Copy link

bryteise commented Mar 2, 2020

I'd like to build against the system tbb instead of the embedded tbb provided by the source package. Looking at the build process this seems like a bigger patch though. Would that be something you would consider adding?

@eddelbuettel
Copy link
Member

Tough. In general, we can support this -- I once enabled just that for nloptr.

But this is a much larger and complicated package so I am not sure. Call to be made by @kevinushey.

@kevinushey
Copy link
Contributor

Seems possible in theory. We load TBB when RcppParallel is loaded here:

RcppParallel/R/hooks.R

Lines 7 to 23 in b216ba2

# load tbb and tbbmalloc on supported platforms
tbb <- tbbLibPath()
if (!is.null(tbb)) {
if (!file.exists(tbb)) {
warning(paste("TBB library", tbb, "not found."))
} else {
dllInfo <<- dyn.load(tbb, local = FALSE, now = TRUE)
}
}
tbbMalloc <- tbbLibPath("malloc")
if (!is.null(tbbMalloc)) {
if (!file.exists(tbbMalloc)) {
warning(paste("TBB malloc library", tbbMalloc, "not found."))
} else {
mallocDllInfo <<- dyn.load(tbbMalloc, local = FALSE, now = TRUE)
}
}

We'd just have to swap out those paths for something separately provided / requested by the user.

However, I'm not sure if TBB is intended to be used in this way, or if things could subtly break.

@bryteise
Copy link
Author

bryteise commented Mar 3, 2020

I patched tbbLibPath to just return the path to the system installed libtbb as a workaround to avoid shipping the vendored libtbb. Currently my project's opencv is linking against the system libtbb without issue. Things often are a little trickier when dlopening though as RccpParallel does since you won't see library version mismatches at build time.

@eddelbuettel
Copy link
Member

Keep at it. This will be worthwhile.

@dsteuer
Copy link

dsteuer commented May 5, 2020

Hunting for a solution of a different issue I came around the debian patch for using the system tbb.
https://salsa.debian.org/r-pkg-team/r-cran-rcppparallel/-/blob/master/debian/patches/use_debian_packaged_libtbb.patch

Would you advise against using of the system's tbb? Otherwise I would build for OpenSUSE just like debian does.

@eddelbuettel
Copy link
Member

It is not my package, hence not my call. In general, I like using system libs, but there is a risk of a delta.

@kevinushey
Copy link
Contributor

I wouldn't advise against it, I just haven't tested it yet. I'd like to eventually do this properly but haven't yet been able to carve out the time to do it (but would gladly accept a PR)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants