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

provide better protection mechanism than R_PreserveObject / R_ReleaseObject? #382

Closed
kevinushey opened this issue Oct 18, 2015 · 2 comments

Comments

@kevinushey
Copy link
Contributor

The R_PreserveObject / R_ReleaseObject mechanism basically works by placing items in a protected pairlist called R_PreciousList. The problem here is that having a large vector of Rcpp objects can lead to sub-optimal behaviour on destruction of that vector, basically because the order in which a vector's elements are destructed is not specified by the C++ standards. Example:

Suppose we have a large vector of numeric vectors, e.g. std::vector<NumericVector> x of size 1000000. If elements are destructed from start to end, then each destruction involves a deep recursion into the R_PreciousList in order to unprotect that R object. This manifests itself, for example, in tidyverse/dplyr#1396.

It just so happens that, with libc++, vector elements are destructed in reverse order (and so we end up just popping elements off the R_PreciousList as desired); this does not seem to be the case with libstdc++ (and so the performance regression shows up there, with deep recursions).

I'm not sure if Rcpp can be helpful here, but if we can I think it would be prudent to do so.

@github-actions
Copy link

This issue is stale (365 days without activity) and will be closed in 31 days unless new activity is seen. Please feel free to re-open it is still a concern, possibly with additional data.

eddelbuettel added a commit that referenced this issue Jan 19, 2021
second pass at 'precious_{preserve,remove}' (addresses #382, #1081)
@eddelbuettel
Copy link
Member

This is now taken care of in Rcpp 1.0.6.2 via #1133. Big thanks to everybody who helped with this, it is a very nice fix.

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

No branches or pull requests

2 participants