oneTBB icon indicating copy to clipboard operation
oneTBB copied to clipboard

Correct behavior for proportional_mode

Open pavelkumbrasev opened this issue 3 years ago • 3 comments

Description

This patch correct the behavior of static_partitioner when it used with custom range that does not support proportional splitting. For example such combination (static_partitioner + custom range) on machine with 6 threads leads to tasks with: [0,32], [32,64], [64,128], [128,256], [256,512], [512,1024] range distribution. Hence, leads to huge disbalance and performance degradation.

It happens because during partition at function we lost partitioner dividing proportions.

This patch propose always use proportional_split with calculated proportions with respect to available partitioner resources. The correct constructor for range will be called via line.

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

Type of change

  • [x] 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

  • [x] added - required for new features and some bug fixes
  • [ ] not needed

Documentation

  • [ ] updated in # - add PR number
  • [ ] needs to be updated
  • [x] not needed

Breaks backward compatibility

  • [ ] Yes
  • [x] No
  • [ ] Unknown

Notify the following users

@kboyarinov @aleksei-fedotov

Other information

Signed-off-by: pavelkumbrasev [email protected]

pavelkumbrasev avatar Sep 13 '22 08:09 pavelkumbrasev

I believe get_range_split_object entity becomes unnecessary as well as explicit-ness of type conversion operator for proportional_split class. Could you please update the patch accordingly?

aleksei-fedotov avatar Sep 15 '22 10:09 aleksei-fedotov

@aleksei-fedotov, it is not true. This helper is still required because the implicit conversion from proportional_split to split is prohibited because of explicit operator. The helper is to make such a conversion explicit. See the short reproducer: https://godbolt.org/z/eqK57EYK1 If the macro USE_RANGE_SPLIT_OBJ is defined to 0, the code fails to compile

kboyarinov avatar Sep 15 '22 12:09 kboyarinov

@aleksei-fedotov, it is not true. This helper is still required because the implicit conversion from proportional_split to split is prohibited because of explicit operator. The helper is to make such a conversion explicit. See the short reproducer: https://godbolt.org/z/eqK57EYK1 If the macro USE_RANGE_SPLIT_OBJ is defined to 0, the code fails to compile

Yeah, sure! And the explicitness of the type conversion operator is necessary since we don't want to accidentally lose the original proportion.

aleksei-fedotov avatar Sep 15 '22 13:09 aleksei-fedotov