oneTBB icon indicating copy to clipboard operation
oneTBB copied to clipboard

Reintroduce public headers for various components

Open Lastique opened this issue 5 years ago • 14 comments

After the oneTBB 2021.1.1 release, certain library components were moved into implementation detail headers. The particular components we are using are:

  • Exceptions (e.g. user_abort). Required to be able to catch TBB exceptions in user's code.
  • Range split tags (e.g. split). Required to be able to implement custom blocked range types.

Currently, we have to include implementation detail headers to obtain these components. They may be available indirectly through other public headers, but it is not obvious which, and the public headers add unnecessary code that increases compile times.

Lastique avatar Dec 16 '20 21:12 Lastique

  • Exceptions (e.g. user_abort). Required to be able to catch TBB exceptions in user's code.

tbb::user_abort is a part of the tbb::concurrent_bounded_queue interface. Try to include <tbb/concurrent_queue.h>.

  • Range split tags (e.g. split). Required to be able to implement custom blocked range types.

tbb::split is defined in parallel algorithm and range related headers. See: https://spec.oneapi.com/versions/latest/elements/oneTBB/source/algorithms/split_tags/split_cls.html

The workaround is to include <tbb.h>.

alex-katranov avatar Dec 17 '20 06:12 alex-katranov

As I said in the original post, the point of having fine grained includes is to reduce compile times. Which is why I'm asking the dedicated headers and don't want to use tbb.h.

Lastique avatar Dec 17 '20 08:12 Lastique

Does not <tbb/concurrent_queue.h> and <tbb/blocked_range.h> suit your needs? Or instead of <tbb/concurrent_queue.h> would you like to have something like <tbb/exceptions.h> that lists all TBB exceptions?

alex-katranov avatar Dec 21 '20 08:12 alex-katranov

Does not <tbb/concurrent_queue.h> and <tbb/blocked_range.h> suit your needs?

No, because I don't need neither concurrent_queue nor blocked_range types where I need exceptions or split.

Or instead of <tbb/concurrent_queue.h> would you like to have something like <tbb/exceptions.h> that lists all TBB exceptions?

Yes, that would be better. Ideally, I'd be happy if there was a header per exception, e.g. <tbb/exceptions/user_abort.h>, and <tbb/exceptions.h> that includes all of them.

Lastique avatar Dec 21 '20 08:12 Lastique

user_abort implementation is about 5-6 lines in header files. Moving it in a separate header file will add about 25-30 lines of service code (license, includes, namespaces, ...). So, it does not solve the initial problem that we want to reduce compile time because in fact we increase the code base.

I would preferer balancing between fine grained headers and overheads that they introduce (e.g. touching too many files during the compilation is also an issue). It seems the current <tbb/detail/_exceptions.h> state is relatively balanced: it provides 4 useful declaration (25 lines) for 85 lines of overall size (compare with 4 files, each 30 lines + service header file (~40 lines) for internal needs).

As for split, I agree that it does not reasonable to include blocked_range while you are not using it.

alex-katranov avatar Dec 21 '20 08:12 alex-katranov

user_abort implementation is about 5-6 lines in header files. Moving it in a separate header file will add about 25-30 lines of service code (license, includes, namespaces, ...). So, it does not solve the initial problem that we want to reduce compile time because in fact we increase the code base.

Comments don't count, as they don't increase compiler times that much. Same with namespace open/close. What you win by placing user_abort into a header of its own is avoid including <new> and <stdexcept> and their dependent headers (especially the latter one, as it pulls std::basic_string and std::allocator) because they are not needed for user_abort definition.

However, as I said, <tbb/exceptions.h> would be an improvement and an acceptable solution.

Also, besides split, don't forget proportional_split, which should also be in the dedicated public header with split.

Lastique avatar Dec 21 '20 09:12 Lastique

avoid including <new> and <stdexcept>

Thank you for pointing to that. It seems we can think about public header for exceptions only and a separate header for internals where <new> and <stdexcept> are really required.

Also, besides split, don't forget proportional_split, which should also be in the dedicated public header with split.

Sure.

alex-katranov avatar Dec 21 '20 09:12 alex-katranov

@Lastique , what is the gain in terms of compile time you see on the environment used ?

anton-potapov avatar Nov 01 '21 09:11 anton-potapov

@Lastique is this issue still relevant for you? Could you please respond?

isaevil avatar Oct 05 '22 11:10 isaevil

As far as I can see, fine grained headers were not added yet, not in the latest TBB release. So yes, this is still relevant.

Lastique avatar Oct 05 '22 11:10 Lastique

@Lastique did you have a chance to check how noticeable the improvement of compile time in your environment as Anton asked?

isaevil avatar Oct 05 '22 13:10 isaevil

No, I did not perform the test.

Lastique avatar Oct 05 '22 14:10 Lastique

@Lastique is this issue is still relevant?

arunparkugan avatar Aug 13 '24 11:08 arunparkugan

@Lastique is this issue is still relevant?

As you can see in the source, the fine-grained headers are still missing. So, yes, it is still relevant.

Lastique avatar Aug 13 '24 12:08 Lastique