Add documentation for parallel_phase feature #1646

isaevil · 2025-02-21T14:34:10Z

Description

Document parallel_phase and leave_policy extensions to task_arena in Developer Reference.

Fixes # - issue number(s) if exists

Type of change

Choose one or multiple, leave empty if none of the other choices apply

Add a respective label(s) to PR if you have permissions

bug fix - change that fixes an issue
new feature - change that adds functionality
tests - change in tests
infrastructure - change in infrastructure and CI
documentation - documentation update

Tests

added - required for new features and some bug fixes
not needed

Documentation

updated in # - add PR number
needs to be updated
not needed

Breaks backward compatibility

Yes
No
Unknown

Notify the following users

List users with @ to send notifications

Other information

Signed-off-by: Isaev, Ilya <ilya.isaev@intel.com>

doc/main/reference/parallel_phase_for_task_arena.rst

aepanchi · 2025-02-21T15:35:54Z

doc/main/reference/parallel_phase_for_task_arena.rst

+with the following API:
+* Add enumeration class ``leave_policy`` to ``task_arena``.
+* Add ``leave_policy`` as the last parameter to ``task_arena`` constructors and ``task_arena::initialize`` methods.
+  This allows to choose retention policy for worker threads in case when no more parallel work in the arena. 


Suggested change

This allows to choose retention policy for worker threads in case when no more parallel work in the arena.

This allows you to define a retention policy for worker threads when no parallel work is in the arena.

I am not sure how to better state this but at the same time to not overwhelm documentation with lots of details. The idea is that the feature allows users to tell the scheduler, what policy to use when TBB's worker threads are going to leave task_arena because they failed to find any work.

maybe smth like: This feature allows you to inform the scheduler about the preferred policy for worker threads when they are about to leave task_arena due to a lack of available work.

doc/main/reference/parallel_phase_for_task_arena.rst

Co-authored-by: Alexandra <alexandra.epanchinzeva@intel.com>

doc/main/reference/parallel_phase_for_task_arena.rst

aepanchi

Everything looks good just a couple of syntax errors to fix, and it will be good to go.

aepanchi · 2025-02-27T10:06:43Z

doc/main/reference/parallel_phase_for_task_arena.rst

+with the following API:
+* Add enumeration class ``leave_policy`` to ``task_arena``.
+* Add ``leave_policy`` as the last parameter to ``task_arena`` constructors and ``task_arena::initialize`` methods.
+  This allows to choose retention policy for worker threads in case when no more parallel work in the arena. 


maybe smth like: This feature allows you to inform the scheduler about the preferred policy for worker threads when they are about to leave task_arena due to a lack of available work.

doc/main/reference/parallel_phase_for_task_arena.rst

Co-authored-by: Alexandra <alexandra.epanchinzeva@intel.com>

Signed-off-by: Isaev, Ilya <ilya.isaev@intel.com>

doc/main/reference/parallel_phase_for_task_arena.rst

aleksei-fedotov · 2025-03-04T13:50:09Z

doc/main/reference/parallel_phase_for_task_arena.rst

+.. cpp:function:: scoped_parallel_phase::scoped_parallel_phatse(task_arena& ta, bool with_fast_leave = false)
+
+Constructs a ``scoped_parallel_phase`` object that starts a parallel phase in the specified ``task_arena``.
+If ``with_fast_leave`` is ``true``, the worker threads leave policy is temporarily set to ``fast``.


Do we need to specify the behavior if task_arena was already initialized using fast leave policy? It might not be clear to the reader. Although, there is this "temporarily" word which sort of indicates that the behavior returns back to the policy specified at arena's construction site.

Similar question to the description of end_parallel_phase below.

Added note about the effect of this setting with arena initialized with fast leave policy.

aleksei-fedotov · 2025-03-04T14:01:52Z

doc/main/reference/parallel_phase_for_task_arena.rst

How about referencing the corresponding RFC document from this file?

The links to the RFC and to the specification (are there other?) are not permanent and therefore will become obsolete someday. I suggest to put permanent links so that they stay relevant as long as possible.

vossmjp · 2025-03-04T19:11:45Z

doc/main/reference/parallel_phase_for_task_arena.rst

+====================================================================
+
+.. note::
+    To enable this feature, set ``TBB_PREVIEW_PARALLEL_PHASE`` macro to 1.


I think we should add, as suggested by Aleksei, a link to the RFC and request feedback on the feature. I'd suggest as part of this note. By putting the link here, we direct users to the RFC where they become aware of the exit conditions and what type of feedback we are seeking. That might drive more feedback.

In the medium term, all preview features will have matching RFCs and so this can become a regular part of preview feature documentation.

Added the link.

doc/main/reference/parallel_phase_for_task_arena.rst

vossmjp · 2025-03-04T19:21:23Z

doc/main/reference/parallel_phase_for_task_arena.rst

+.. cpp:enum:: leave_policy::automatic
+
+When passed to a constructor or the ``initialize`` method, the initialized ``task_arena`` has
+default policy for worker threads.


Suggested change

default policy for worker threads.

default policy for worker thread retention.

Perhaps, "has the default (possibly system specific) policy for how quickly worker threads leave the arena when there is no more work available in the arena and when the arena is not in a parallel phase."

vossmjp · 2025-03-04T19:59:09Z

doc/main/reference/parallel_phase_for_task_arena.rst

+
+When passed to a constructor or the ``initialize`` method, the initialized ``task_arena`` has
+policy to not retain worker threads in ``task_arena``.
+


I don't think "retention" or "retain" is defined anywhere. And I think we need to describe the effect within a phase and then between phases. For example, "to not retain worker threads" is not clear unless we say what we mean by retention. For sure, the arena retains the workers while there are active tasks to work on.

Maybe something like, "has policy that allows worker threads to more quickly leave the arena when there is no more work available in the arena and when the arena is not in a parallel phase."

Applied suggested wording. I also added some notes on behavior when threads re-join the arena with no active parallel phases.

vossmjp · 2025-03-04T20:11:54Z

doc/main/reference/parallel_phase_for_task_arena.rst

+                    fast = /* unspecifed */,
+                };
+
+                task_arena(int max_concurrency = automatic, unsigned reserved_for_masters = 1,


What about the copy constructor? It copies settings from the other arena, including the leave policy. I think this behavior should be stated somewhere?

Since the API for the copy constructor is not changed, I think the only way to state that is to copy description from the spec and add info about the leave policy.

Listed copy constructor in member functions.

Co-authored-by: Mike Voss <michaelj.voss@intel.com> Co-authored-by: Aleksei Fedotov <aleksei.fedotov@intel.com>

Signed-off-by: Isaev, Ilya <ilya.isaev@intel.com>

aleksei-fedotov

Looks good to me!

aleksei-fedotov · 2025-03-05T15:17:55Z

doc/main/reference/parallel_phase_for_task_arena.rst

The links to the RFC and to the specification (are there other?) are not permanent and therefore will become obsolete someday. I suggest to put permanent links so that they stay relevant as long as possible.

vossmjp

Looks go to me.

Document parallel phase feature in reference

a62bff9

Signed-off-by: Isaev, Ilya <ilya.isaev@intel.com>

isaevil requested a review from aepanchi February 21, 2025 14:34

github-actions bot added the documentation label Feb 21, 2025

isaevil added 2 commits February 21, 2025 15:38

Change wording

0026112

Signed-off-by: Isaev, Ilya <ilya.isaev@intel.com>

Fix incorrect description

bb5fda3

Signed-off-by: Isaev, Ilya <ilya.isaev@intel.com>

aepanchi reviewed Feb 21, 2025

View reviewed changes

isaevil and others added 2 commits February 24, 2025 12:20

Apply suggestions from code review

6e8fa40

Co-authored-by: Alexandra <alexandra.epanchinzeva@intel.com>

Apply suggestions from code review

c534f74

Co-authored-by: Alexandra <alexandra.epanchinzeva@intel.com>

isaevil commented Feb 24, 2025

View reviewed changes

doc/main/reference/parallel_phase_for_task_arena.rst Outdated Show resolved Hide resolved

Update doc/main/reference/parallel_phase_for_task_arena.rst

23502d6

isaevil requested a review from aepanchi February 25, 2025 10:43

aepanchi previously approved these changes Feb 27, 2025

View reviewed changes

Apply suggestions from code review

5f90426

Co-authored-by: Alexandra <alexandra.epanchinzeva@intel.com>

isaevil dismissed aepanchi’s stale review via 5f90426 February 28, 2025 10:55

Change description, update alignment

489d9a7

Signed-off-by: Isaev, Ilya <ilya.isaev@intel.com>

isaevil requested review from aleksei-fedotov, vossmjp, akukanov, dnmokhov and kboyarinov February 28, 2025 11:11

aleksei-fedotov reviewed Mar 4, 2025

View reviewed changes

vossmjp reviewed Mar 4, 2025

View reviewed changes

isaevil and others added 2 commits March 5, 2025 09:24

Apply suggestions from code review

f334dc2

Co-authored-by: Mike Voss <michaelj.voss@intel.com> Co-authored-by: Aleksei Fedotov <aleksei.fedotov@intel.com>

Add details about policies, notes about ending parallel phases

4d648a2

Signed-off-by: Isaev, Ilya <ilya.isaev@intel.com>

isaevil requested review from aleksei-fedotov and vossmjp March 5, 2025 09:51

aleksei-fedotov approved these changes Mar 5, 2025

View reviewed changes

vossmjp approved these changes Mar 6, 2025

View reviewed changes

isaevil merged commit e02bdf2 into master Mar 6, 2025
27 checks passed

isaevil deleted the dev/isaevil/parallel_phase_doc branch March 6, 2025 16:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add documentation for parallel_phase feature #1646

Add documentation for parallel_phase feature #1646

isaevil commented Feb 21, 2025

aepanchi Feb 21, 2025

isaevil Feb 24, 2025 •

edited

Loading

aepanchi Feb 27, 2025

isaevil Feb 28, 2025

aepanchi left a comment

aepanchi Feb 27, 2025

aleksei-fedotov Mar 4, 2025

isaevil Mar 5, 2025

aleksei-fedotov Mar 4, 2025

isaevil Mar 5, 2025

aleksei-fedotov Mar 5, 2025

vossmjp Mar 4, 2025

vossmjp Mar 4, 2025

isaevil Mar 5, 2025

vossmjp Mar 4, 2025

vossmjp Mar 4, 2025

isaevil Mar 5, 2025

vossmjp Mar 4, 2025

isaevil Mar 5, 2025

vossmjp Mar 4, 2025

isaevil Mar 5, 2025

isaevil Mar 5, 2025

aleksei-fedotov left a comment

aleksei-fedotov Mar 5, 2025

vossmjp left a comment

	This allows to choose retention policy for worker threads in case when no more parallel work in the arena.
	This allows you to define a retention policy for worker threads when no parallel work is in the arena.

	default policy for worker threads.
	default policy for worker thread retention.


		When passed to a constructor or the ``initialize`` method, the initialized ``task_arena`` has
		policy to not retain worker threads in ``task_arena``.

Add documentation for parallel_phase feature #1646

Add documentation for parallel_phase feature #1646

Conversation

isaevil commented Feb 21, 2025

Description

Type of change

Tests

Documentation

Breaks backward compatibility

Notify the following users

Other information

Choose a reason for hiding this comment

isaevil Feb 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aepanchi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aleksei-fedotov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vossmjp left a comment

Choose a reason for hiding this comment

isaevil Feb 24, 2025 •

edited

Loading