Avoid split constructor for the Range concept

[WalterDehnen]

The current Range concept requires

Range::Range(Range &range, split);
Range::Range(Range &range, proportional_split);    // optional

This means that Range cannot be a POD type and that other constructors must be explicitly declared. Moreover, it also requires the definitions of tbb::split and tbb::proportional_split to be visible in the definition of Range (often requiring surrounding these constructors with #if and #endif directives).

Proposal: instead require

Range Range::split();
Range Range::split(std::size_t left, std::size_t right);   // optional

Granted, this requires a bit more coding on the tbb side in case of a proportional split, but the point is to make tbb more user friendly.

Such a change in the concept can be rolled out as optional, then deprecating the old version etc. All this can easily be implemented using SFINAE methods to detect the presence these methods.

[kboyarinov]

@WalterDehnen, thank you for highlighting this interesting use case. We will consider making some changes in that regard. We will need to discuss this in details because this change introduces potential breaks for some implementations of Range where the method split was used for something else. May I also ask you to share more details about your use case when having Range object as POD is practically important to have better motivation for any further changes?

[WalterDehnen]

Hello,

thanks for your email. Here are some further arguments for this.

1. Use case example. I have a simulation code using particles. The particles can have different types and are stored in blocks, which in turn are organised in a linked list. Each block holds particles of the same type and is linked together with other blocks holding such particles. This allows easy re-sizing (by adding blocks) w/o re-allocating. In order to iterate over particles (either all or all of a given type), I use a range-like object ‘particle_range', holding a block pointer and an index inside the block. The splitting of such a range is first done at the block level, then within a block.

2. Argument: constructor semantics The split constructors required with the existing tbb Range concept have a strange/unconventional semantics, in that they fundamentally alter their argument. Common constructor semantics is that the arguments to the constructor (with the explicit exception of xvalues for a move constructor), are not altered, or if they are, not in a fundamental way (for example an allocator-like object is used to allocate memory needed by the newly constructed object — this necessarily requires non-constant access to the allocator, but does not fundamentally change its meaning). With the proposed

Range Range::split(); // for general splitting Range Range::split(size_t left, size_t right); // for proportional splitting

the semantics is clear: a non-constant member function alters the object in a way reflected in its name ’split’.

3. Argument: independence of tbb headers The idea is that the concept ‘Range' should only have necessary dependencies. Here, the dependence on any tbb internals, such as tbb::split or tbb::proportional_split are unnecessary, since the related functionality can be expressed equally well without these tbb internals. This is particularly important for code that may or may not use TBB. In my use case (which may or may not use tbb), this means that the definition of my ‘particle_range’ would not need to see an #include tbb.h (or equivalent) and can be expressed without conditional macros like

#ifdef UsingTBB particle_range(particle_range &range_to_split, tbb::split) : particle_range(range_to_split.split()) {} #endif

Best, Walter Dehnen.

On 9. May 2024, at 17:01, Konstantin Boyarinov @.***> wrote:

@WalterDehnen https://github.com/WalterDehnen, thank you for highlighting this interesting use case. We will consider making some changes in that regard. We will need to discuss this in details because this change introduces potential breaks for some implementations of Range where the method split was used for something else. May I also ask you to share more details about your use case when having Range object as POD is practically important to have better motivation for any further changes?

— Reply to this email directly, view it on GitHub https://github.com/oneapi-src/oneTBB/issues/1376#issuecomment-2102835119, or unsubscribe https://github.com/notifications/unsubscribe-auth/AI2TD7HMJMMBQJBC3YTDS3DZBOFS7AVCNFSM6AAAAABHFJ6DX6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBSHAZTKMJRHE. You are receiving this because you were mentioned.

[arunparkugan]

@kboyarinov is this issue is still relevant?

[WalterDehnen]

The 'issue' is poor code design/style and hence always relevant.