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

Fix constraint arena construction #1151

Merged

Conversation

pavelkumbrasev
Copy link
Contributor

Description

Currently constrained arena might be initialized (first touch to fields) on inappropriate constraints because initializing thread doesn't apply constraints during initialization.
This PR reuses binding_observer to apply constraints on thread that performs initialization.

Fixes # - issue number(s) if exists

  • - git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details)

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

Copy link
Contributor

@aleksei-fedotov aleksei-fedotov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative approach would be deferring initialization of the fields until the task is spawned in the arena or the thread is joined the arena for task execution, essentially adding more laziness to the process. In this case, we can save on superfluous calls to enter and exit observer. It will even save on arena initialization if the task will not be spawned and thread would not be entering arena for execution at all.

Have this approach been considered?

@pavelkumbrasev
Copy link
Contributor Author

Alternative approach would be deferring initialization of the fields until the task is spawned in the arena or the thread is joined the arena for task execution, essentially adding more laziness to the process. In this case, we can save on superfluous calls to enter and exit observer. It will even save on arena initialization if the task will not be spawned and thread would not be entering arena for execution at all.

This is an interesting suggestion. I believe it will require extra synchronization on each arena entry (we already have one for task_arena::initialize) so I'm not sure if it will be effective.
Added TODO comment.

Signed-off-by: pavelkumbrasev <pavel.kumbrasev@intel.com>
Signed-off-by: pavelkumbrasev <pavel.kumbrasev@intel.com>
@pavelkumbrasev pavelkumbrasev force-pushed the dev/pavelkumbrasev/fix_constraint_arena_construction branch from b3d8975 to 120fbed Compare April 24, 2024 13:50
Signed-off-by: pavelkumbrasev <pavel.kumbrasev@intel.com>
src/tbb/arena.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@isaevil isaevil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pavelkumbrasev pavelkumbrasev merged commit c1a0e74 into master Apr 25, 2024
22 checks passed
@pavelkumbrasev pavelkumbrasev deleted the dev/pavelkumbrasev/fix_constraint_arena_construction branch April 25, 2024 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants