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

interim release to help with future binary compatibility #234

Merged
merged 10 commits into from
Jan 28, 2025

Conversation

kevinushey
Copy link
Contributor

@kevinushey kevinushey commented Jan 23, 2025

This PR sets the ground-work for an RcppParallel 5.1.10 release, whose main goals are to remove the accidental direct dependency on TBB that packages get when using RcppParallel's utilities. Using gaston from CRAN as an example:

kevin@kevinushey-NPCV:~/Library/R/arm64/4.4/library
$ nm gaston/libs/gaston.so | grep tbb | grep U | c++filt
                 U tbb::interface5::internal::task_base::destroy(tbb::task&)
                 U tbb::interface7::internal::task_arena_base::internal_terminate()
                 U tbb::interface7::internal::task_arena_base::internal_initialize()
                 U tbb::interface9::global_control::internal_create()
                 U tbb::interface9::global_control::internal_destroy()
                 U tbb::task_group_context::cancel_group_execution()
                 U tbb::task_group_context::register_pending_exception()
                 U tbb::task_group_context::init()
                 U tbb::task_group_context::reset()
                 U tbb::task_group_context::~task_group_context()
                 U tbb::task::note_affinity(unsigned short)
                 U tbb::internal::throw_exception_v4(tbb::internal::exception_id)
                 U tbb::internal::get_initial_auto_partitioner_divisor()
                 U tbb::interface7::internal::task_arena_base::internal_execute(tbb::interface7::internal::delegate_base&) const
                 U tbb::task_group_context::is_group_execution_cancelled() const
                 U tbb::internal::allocate_child_proxy::allocate(unsigned long) const
                 U tbb::internal::allocate_continuation_proxy::allocate(unsigned long) const
                 U tbb::internal::allocate_root_with_context_proxy::free(tbb::task&) const
                 U tbb::internal::allocate_root_with_context_proxy::allocate(unsigned long) const
                 U typeinfo for tbb::task

What this means is that any attempts to RcppParallel which change tbb internals will break packages like gaston, since those symbols may no longer be available (e.g. because they've moved to different namespaces, or APIs have changed altogether).

This PR resolves the issue by hiding the implementation details for parallelFor and parallelReduce within RcppParallel. This was harder than I expected because we allowed classes of the form:

struct InnerProduct : public RcppParallel::Worker {
  void join(const InnerProduct& rhs) { ... }
};

And so, we have no knowledge of the actual user-defined class or its type from within the RcppParallel worker. Hence, we need to take the template type passed to tbbParallelReduce(), use that to construct a whole bunch of type-erased methods within a non-template class, and then pass that blob down. Gross.

I'm hoping that, sometime after R 4.5.0 is released, we'll be able to delete this "glue" and have a more sane interface, but we'll see.

In any event, after doing all of this and then reinstalling gaston from sources, I see:

$ nm gaston/libs/gaston.so | grep tbb | grep U | c++filt
                 U RcppParallel::tbbParallelFor(unsigned long, unsigned long, RcppParallel::Worker&, unsigned long, int)
                 U RcppParallel::tbbParallelReduceImpl(unsigned long, unsigned long, RcppParallel::ReducerWrapper&, unsigned long, int)

and so we have now successfully hidden the TBB internals from gaston.

@kevinushey
Copy link
Contributor Author

RcppParallel 5.1.10 built from this branch is now on CRAN; I'm going to take care of merging this (and handling merge conflicts) soon.

@kevinushey kevinushey merged commit 762a579 into master Jan 28, 2025
3 checks passed
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 this pull request may close these issues.

1 participant