Migrate to tbb::task_group

[Dr15Jones]

PR description:

The tbb::task ABI has been deprecated by Intel's developers in favor of tbb::task_group.

PR validation:

The code compiles and all framework related unit tests succeed.

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32804/20977

• This PR adds an extra 472KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

• code-format:
  https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32804/20977/code-format.patch
  e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32804/20977/code-format.patch | patch -p1
  You can also run scram build code-format to apply code format directly

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32804/20979

• This PR adds an extra 320KB to repository

[Dr15Jones]

please test

[cmsbuild]

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

FWCore/Concurrency
FWCore/Framework
FWCore/Integration
FWCore/TestProcessor
IOPool/Input

@makortel, @smuzaffar, @Dr15Jones can you please review it and eventually sign? Thanks.
@makortel, @wddgit this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

[Dr15Jones]

hold

additional changes are expected but a fuller test is useful.

[cmsbuild]

Pull request has been put on hold by @Dr15Jones
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

[cmsbuild]

-1

Failed Tests: Build ClangBuild
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-422c34/12683/summary.html
COMMIT: db792e2
CMSSW: CMSSW_11_3_X_2021-02-03-0800/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/32804/12683/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation error when building:

>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-02-03-0800/src/GeneratorInterface/LHEInterface/plugins/LHEFilter.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-02-03-0800/src/GeneratorInterface/LHEInterface/plugins/LHE2HepMCConverter.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-02-03-0800/src/GeneratorInterface/LHEInterface/plugins/ExternalLHEProducer.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-02-03-0800/src/GeneratorInterface/LHEInterface/plugins/ExternalLHEAsciiDumper.cc
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-02-03-0800/src/GeneratorInterface/LHEInterface/plugins/ExternalLHEProducer.cc: In lambda function:
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-02-03-0800/src/GeneratorInterface/LHEInterface/plugins/ExternalLHEProducer.cc:303:45: error: no matching function for call to 'make_functor_task(tbb::internal::allocate_root_proxy, ExternalLHEProducer::beginRunProduce(edm::Run&, const edm::EventSetup&)::::)'
  303 |                                            });
      |                                             ^
In file included from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-02-03-0800/src/FWCore/Framework/src/Worker.h:47,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-02-03-0800/src/FWCore/Framework/src/Factory.h:5,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-02-03-0800/src/FWCore/Framework/interface/MakerMacros.h:4,


Clang Build

I found compilation warning while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

See details on the summary page.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32804/20992

• This PR adds an extra 328KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File GeneratorInterface/LHEInterface/plugins/ExternalLHEProducer.cc modified in PR(s): Refactoring gen weight storage in EDM + Nano integration #32167

[santocch]

+1

[qliphy]

ping @cms-sw/generators-l2

[agrohsje]

+generators

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

[qliphy]

+1

[mrodozov]

@Dr15Jones DD4hep uses tbb. Perhaps the changes in this PR somehow caused a re-calculation of the geometry by DD4hep, which then produced statistical fluctuations in the results of simulation using DD4hep. The comparison differences for 11634.911 are not significant.

DD4Hep is not using TBB since cms-sw/cmsdist#6554 when we removed it

[vgvassilev]

Hi @Dr15Jones, we suspect this define is causing a recent CXXMODULES IB regression. Would it be possible to move this define (I saw another one in the cc file) to the build system and possibly add it to the rootcling invocation (if scram does not do that automatically).

cc: @davidlange6

[Dr15Jones]

@vgvassilev in principal, setting this at compile time should work. We did it in the header since that was the recommendation of the TBB developers.

@smuzaffar do you want to try it as a build directive?

[davidlange6]

in one of the headers including tbb/task_group.h but not consistently. I think thats why we noticed. I can experiment on the modules IB branch if you want.

[Dr15Jones]

in one of the headers including tbb/task_group.h but not consistently.

That was intentional. It is only turned on where the feature is needed. Again, that was the recommendation of the tbb developers.

[smuzaffar]

@Dr15Jones , yes we can set it at build time. @davidlange6 , are you testing it by adding it in the tbb.xml toolfile? You can add it in the toolfile and then scram should set itfo rboth normal build and rootcling

[smuzaffar]

this macro is now defined via tbb toolfile ( see #33140 and cms-sw/cmsdist#6713 )

[vgvassilev]

in one of the headers including tbb/task_group.h but not consistently.

That was intentional. It is only turned on where the feature is needed. Again, that was the recommendation of the tbb developers.

I'd assume they did not know about the fact we use C++ modules in the dictionaries. In theory, we can still use the macro the way we do, and only pass the relevant -D to rootcling. Not sure if that's worth it.