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.

Sign up for GitHub

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

Remove use of deprecated tbb::task_scheduler_init from FWCore #31313

Merged
merged 10 commits into from
Sep 11, 2020
Merged

Remove use of deprecated tbb::task_scheduler_init from FWCore #31313

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e9c55be
Switch to using tbb::global_control
Dr15Jones Aug 29, 2020
5c29b9c
Use edm::ThreadsController to constrain threads
Dr15Jones Aug 31, 2020
654bd25
Use tbb::global_control to determine if using only 1 thread
Dr15Jones Aug 31, 2020
e29b797
Use explicit tbb::task_arena to control number of threads
Dr15Jones Sep 1, 2020
b540c6b
Removed unused EventProcessor calls
Dr15Jones Sep 1, 2020
64122db
Removed unused file testDTUnpackingModule.cc
Dr15Jones Sep 1, 2020
8ee085c
Use cmsRun directly for test
Dr15Jones Sep 1, 2020
ab1c866
Fix crash in TestProcessor when exception occurred
Dr15Jones Sep 1, 2020
cb40a2f
properly set number of threads for PythonEventProcessor
Dr15Jones Sep 3, 2020
c645fae
Added comment about ThreadsController lifetime requirements
Dr15Jones Sep 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions CondFormats/CSCObjects/test/testCSCMapping.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

#include <cppunit/extensions/HelperMacros.h>
#include <FWCore/Utilities/interface/Exception.h>
#include <FWCore/Framework/interface/EventProcessor.h>
#include <FWCore/PluginManager/interface/ProblemTracker.h>
#include <FWCore/ParameterSet/interface/ParameterSet.h>
#include "FWCore/ParameterSetReader/interface/ParameterSetReader.h"
Expand Down Expand Up @@ -52,27 +51,12 @@ class testCSCMapping : public CppUnit::TestFixture {

void testRead();

int runIt(const std::string& config);

private:
const std::string myName_;
const int dashedLineWidth;
std::string dashedLine;
};

int testCSCMapping::runIt(const std::string& config) {
edm::AssertHandler ah;
int rc = 0;
try {
edm::EventProcessor proc(edm::getPSetFromConfig(config));
proc.run();
} catch (cms::Exception& e) {
std::cerr << "Exception caught: " << e.what() << std::endl;
rc = 1;
}
return rc;
}

void testCSCMapping::testRead() {
edm::FileInPath fip("CondFormats/CSCObjects/data/csc_slice_test_map.txt");
std::cout << "Attempt to set FileInPath to " << fip.fullPath() << std::endl;
Expand Down
16 changes: 0 additions & 16 deletions CondFormats/CSCObjects/test/testCSCTriggerMapping.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

#include <cppunit/extensions/HelperMacros.h>
#include <FWCore/Utilities/interface/Exception.h>
#include <FWCore/Framework/interface/EventProcessor.h>
#include "FWCore/PluginManager/interface/ProblemTracker.h"
#include <CondFormats/CSCObjects/interface/CSCTriggerMappingFromFile.h>
#include <DataFormats/MuonDetId/interface/CSCDetId.h>
Expand Down Expand Up @@ -42,27 +41,12 @@ class testCSCTriggerMapping : public CppUnit::TestFixture {

void testRead();

int runIt(const std::string& config);

private:
const std::string myName_;
const int dashedLineWidth;
std::string dashedLine;
};

int testCSCTriggerMapping::runIt(const std::string& config) {
edm::AssertHandler ah;
int rc = 0;
try {
edm::EventProcessor proc(edm::getPSetFromConfig(config));
proc.run();
} catch (cms::Exception& e) {
std::cerr << "Exception caught: " << e.what() << std::endl;
rc = 1;
}
return rc;
}

void testCSCTriggerMapping::testRead() {
std::cout << myName_ << ": --- t e s t C S C T r i g g e r M a p p i n g ---" << std::endl;
std::cout << "start " << dashedLine << std::endl;
Expand Down
6 changes: 1 addition & 5 deletions EventFilter/CSCTFRawToDigi/test/BuildFile.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,4 @@
<use name="CondFormats/CSCObjects"/>
<flags EDM_PLUGIN="1"/>
</library>

<bin name="testCSCTFRawToDigi" file="testCSCTFRawToDigi.cpp">
<use name="FWCore/ParameterSetReader"/>
<use name="cppunit"/>
</bin>
<test name="testCSCTFRawToDigi" command="cmsRun ${LOCALTOP}/src/EventFilter/CSCTFRawToDigi/test/testCSCTFRawToDigi_cfg.py"/>
91 changes: 0 additions & 91 deletions EventFilter/CSCTFRawToDigi/test/testCSCTFRawToDigi.cpp
View file
Open in desktop

This file was deleted.

22 changes: 22 additions & 0 deletions EventFilter/CSCTFRawToDigi/test/testCSCTFRawToDigi_cfg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import FWCore.ParameterSet.Config as cms
process = cms.Process('analyzer')
process.load('EventFilter.CSCTFRawToDigi.csctfunpacker_cfi')
process.load('EventFilter.CSCTFRawToDigi.csctfpacker_cfi')
process.maxEvents = cms.untracked.PSet( input = cms.untracked.int32(128) )
process.load('FWCore.MessageLogger.MessageLogger_cfi')
process.MessageLogger.cout.placeholder = cms.untracked.bool(False)
process.MessageLogger.cout.threshold = cms.untracked.string('INFO')
process.MessageLogger.debugModules = cms.untracked.vstring('*')
process.source = cms.Source('EmptySource')
process.csctfsinglegen = cms.EDProducer('CSCTFSingleGen')
process.csctfpacker.lctProducer = cms.InputTag('csctfsinglegen:')
process.csctfpacker.mbProducer = cms.InputTag('null:')
process.csctfpacker.trackProducer = cms.InputTag('null:')
process.csctfunpacker.producer = cms.InputTag('csctfpacker','CSCTFRawData')
process.csctfanalyzer = cms.EDAnalyzer('CSCTFAnalyzer',
mbProducer = cms.untracked.InputTag('csctfunpacker:DT'),
lctProducer = cms.untracked.InputTag('csctfunpacker:'),
trackProducer = cms.untracked.InputTag('csctfunpacker:'),
statusProducer = cms.untracked.InputTag('csctfunpacker:')
)
process.p = cms.Path(process.csctfsinglegen*process.csctfpacker*process.csctfunpacker*process.csctfanalyzer)
113 changes: 0 additions & 113 deletions EventFilter/DTRawToDigi/test/testDTUnpackingModule.cc
View file
Open in desktop

This file was deleted.

51 changes: 51 additions & 0 deletions FWCore/Concurrency/interface/ThreadsController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#ifndef FWCore_Concurrency_ThreadsController_h
#define FWCore_Concurrency_ThreadsController_h
// -*- C++ -*-
//
// Package: FWCore/Concurrency
// Class : ThreadsController
//
/**\class ThreadsController ThreadsController.h "ThreadsController.h"

Description: Controls how many threads and how much stack memory per thread

Usage:
The lifetime of the ThreadsController sets how long the options are in use.

*/
//
// Original Author: FWCore
// Created: Fri, 18 Nov 2016 20:30:42 GMT
//

// system include files
#include "tbb/global_control.h"
#include "tbb/task_arena.h"
#include <memory>

// user include files

// forward declarations

namespace edm {
class ThreadsController {
public:
ThreadsController() = delete;
explicit ThreadsController(unsigned int iNThreads)
: m_nThreads{tbb::global_control::max_allowed_parallelism, iNThreads}, m_stackSize{} {}
ThreadsController(unsigned int iNThreads, size_t iStackSize)
: m_nThreads{tbb::global_control::max_allowed_parallelism, iNThreads}, m_stackSize{makeStackSize(iStackSize)} {}

// ---------- member functions ---------------------------
void setStackSize(size_t iStackSize) { m_stackSize = makeStackSize(iStackSize); }

private:
static std::unique_ptr<tbb::global_control> makeStackSize(size_t iStackSize);

// ---------- member data --------------------------------
tbb::global_control m_nThreads;
std::unique_ptr<tbb::global_control> m_stackSize;
};
} // namespace edm

#endif
6 changes: 2 additions & 4 deletions FWCore/Concurrency/interface/setNThreads.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@
// Created by Chris Jones on 7/24/20.
//
#include <memory>
#include "tbb/task_scheduler_init.h"
#include "FWCore/Concurrency/interface/ThreadsController.h"

namespace edm {
unsigned int setNThreads(unsigned int iNThreads,
unsigned int iStackSize,
std::unique_ptr<tbb::task_scheduler_init>& oPtr);
unsigned int setNThreads(unsigned int iNThreads, unsigned int iStackSize, std::unique_ptr<ThreadsController>& oPtr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Beyond this PR, but what would you think about

Suggested change
unsigned int setNThreads(unsigned int iNThreads, unsigned int iStackSize, std::unique_ptr<ThreadsController>& oPtr);
std::tuple<unsigned int, std::unique_ptr<ThreadsController>> setNThreads(unsigned int iNThreads, unsigned int iStackSize,);

?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, given how it is called in cmsRun.cpp the two interfaces don't really make much difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be safe, I have to be sure the old ThreadsController is deleted before the new one is constructed. Previously when using tbb::task_scheduler_init if you didn't get rid of the old one before making a new one, the job would crash. I'm not sure if tbb::global_control has the same conditions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would that subtlety be worth a comment in the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a comment to the function declaration.

}
#endif /* FWCore_Concurrency_setNThreads_h */
Loading