migrate modules used by the HLT menu to multithreading (various) #10958

fwyzard · 2015-08-26T15:43:19Z

This is the part of #10909 that affects more than one group.

I'm still working on it, to see if there is a more efficient solution.

- L1GtVhdlWriterCore: get rid of an unnecessary const_cast - L1GtTriggerMenuXmlParser: fix the input parameter of strtol

cmsbuild · 2015-08-26T15:46:12Z

A new Pull Request was created by @fwyzard (Andrea Bocci) for CMSSW_7_6_X.

migrate to modules used by the HLT menu multithreading (part 1)

It involves the following packages:

HLTrigger/HLTcore
HLTrigger/HLTfilters
L1Trigger/GlobalTrigger
L1TriggerConfig/L1GtConfigProducers

@Martin-Grunewald, @perrotta, @cmsbuild, @mulhearn, @fwyzard can you please review it and eventually sign? Thanks.
@Martin-Grunewald this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

Dr15Jones · 2015-08-26T16:01:39Z

HLTrigger/HLTcore/interface/TriggerExpressionData.h

@@ -224,7 +223,7 @@ class Data {

  // l1 values and status
  const L1GlobalTriggerReadoutRecord  * m_l1tResults;
-  const L1GtTriggerMenu               * m_l1tMenu;
+  std::unique_ptr<L1GtTriggerMenu>      m_l1tMenu;


Should this be std::unique_ptr<const L1GtTriggerMenu>?

Ah, I see it can't because of the call to buildGtConditionMap. Would it be possible to change L1GtTriggerMenu to have a constructor which calls that function?

I just checked the code for L1GtTriggerMenu and the copy constructor does call buildGtConditionMap so you don't have to.

Dr15Jones · 2015-08-26T16:29:07Z

@fwyzard I believe I have the proper solution to your L1GtTriggerMenu problem. At the moment, when reading back a L1GtTriggerMenu from the database, one of the member data is not initialized because that member data is marked as transient:
http://cmslxr.fnal.gov/source/CondFormats/L1TObjects/src/classes_def.xml?v=CMSSW_7_6_X_2015-08-25-2300#0221

This is why people were calling buildGtConditionMap which is not thread safe to do on the product stored in the EventSetup. However, there is a mechanism to 'kick' an object right after it has been read back from the database. We can use this mechanism to call buildGtConditionMap after the object has been read from the DB but before the object is put into the EventSetup.

The trick is to use an 'Initializer' for the DataProxy
http://cmslxr.fnal.gov/source/CondCore/ESSources/interface/DataProxy.h?v=CMSSW_7_6_X_2015-08-25-2300#0039

I think the following should work:
replace
http://cmslxr.fnal.gov/source/CondCore/L1TPlugins/src/plugin.cc?v=CMSSW_7_6_X_2015-08-25-2300#0185

with

namespace cond {
   struct L1GtTriggerMenuInitializer {
      void operator()( L1GtTriggerMenu& iMenu) const {
         iMenu. buildGtConditionMap();
      }
  };
}

REGISTER_PLUGIN_INIT(L1GtTriggerMenuRcd, L1GtTriggerMenu, cond::L1GtTriggerMenuInitializer);

fwyzard · 2015-08-26T16:32:55Z

Hi Chris, thanks for your comments. I was working on a separate branch to implement something like that, and discussing a possible solution on the AlCa hypernews... what you propose should indeed fix the problem, I'll try it later today. About std::make_unique<>, can we actually use it? It's only in C++14, not in C++11.

Dr15Jones · 2015-08-26T16:35:15Z

CMSSW_7_6 now uses the C++14 flags for the compiler.

fwyzard · 2015-08-27T07:09:24Z

hi Chris,
do you know why this works:

namespace {
  struct L1GtTriggerMenuInitializer { void operator()(L1GtTriggerMenu & _) { _.buildGtConditionMap(); } };
}
REGISTER_PLUGIN_INIT(L1GtTriggerMenuRcd, L1GtTriggerMenu, L1GtTriggerMenuInitializer);

while this does not:

REGISTER_PLUGIN_INIT(L1GtTriggerMenuRcd, L1GtTriggerMenu, [](L1GtTriggerMenu & _){ _.buildGtConditionMap(); });

and neither does:

REGISTER_PLUGIN_INIT(L1GtTriggerMenuRcd, L1GtTriggerMenu, std::mem_fn(& L1GtTriggerMenu::buildGtConditionMap));

?

…e EventSetup

cmsbuild · 2015-08-27T07:41:17Z

Pull request #10958 was updated. @perrotta, @cmsbuild, @danduggan, @monttj, @Martin-Grunewald, @deguio, @fwyzard, @ggovi, @vadler, @mulhearn can you please check and sign again.

L1GtTriggerMenu is now properly initialised by the EventSetup framework, so there is no longer need to call (const_cast<L1GtTriggerMenu *>(l1GtTriggerMenu))->buildGtConditionMap(); or to keep a local copy for that purpose

- change most configuration parameters into const members - change into a stream::EDProducer

cmsbuild · 2015-08-27T07:46:16Z

Pull request #10958 was updated. @perrotta, @cmsbuild, @danduggan, @monttj, @Martin-Grunewald, @deguio, @fwyzard, @ggovi, @vadler, @mulhearn can you please check and sign again.

fwyzard · 2015-08-27T07:47:13Z

@cmsbuild please test

cmsbuild · 2015-08-27T07:51:22Z

The tests are being triggered in jenkins.

fwyzard · 2015-08-27T07:58:06Z

+1

fwyzard · 2015-08-27T08:19:18Z

tracked at #10964

cmsbuild · 2015-08-27T09:53:43Z

+1
Tested at: 80fa995
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-10958/7678/summary.html

cmsbuild · 2015-08-27T10:53:30Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-10958/7678/summary.html

Dr15Jones · 2015-08-27T14:18:10Z

You asked about why certain alternate forms of REGISTER_PLUGIN_INIT do not work. Ultimately the third argument of REGISTER_PLUGIN_INIT is a type passed as a template argument. The two expressions (one lambda one std::mem_fn) are not types.

fwyzard · 2015-08-27T15:31:57Z

You asked about why certain alternate forms of REGISTER_PLUGIN_INIT do
not work. Ultimately the third argument of REGISTER_PLUGIN_INIT is a type
passed as a template argument. The two expressions (one lambda one
std::mem_fn) are not types.

Right. Understood, thanks.

mulhearn · 2015-09-01T14:30:08Z

+1

deguio · 2015-09-01T15:09:28Z

+1

ggovi · 2015-09-01T15:12:40Z

+1

…part1 migrate to modules used by the HLT menu multithreading (various)

cleanup some const_cast

646526a

- L1GtVhdlWriterCore: get rid of an unnecessary const_cast - L1GtTriggerMenuXmlParser: fix the input parameter of strtol

cmsbuild added this to the Next CMSSW_7_6_X milestone Aug 26, 2015

cmsbuild added l1-pending hlt-pending pending-signatures tests-pending orp-pending comparison-pending labels Aug 26, 2015

Dr15Jones reviewed Aug 26, 2015
View reviewed changes

This was referenced Aug 27, 2015

migrate modules used by the HLT menu to multithreading (76x) #10964

Closed

migrate modules used by the HLT menu to multithreading (various) (75x) #10970

Merged

initialise L1GtTriggerMenu objects before making them available in th…

5e1b22d

…e EventSetup

fwyzard force-pushed the migrate_to_multithreading_75x_part1 branch from 63f6762 to 978abf1 Compare August 27, 2015 07:38

cmsbuild added dqm-pending analysis-pending db-pending labels Aug 27, 2015

fwyzard force-pushed the migrate_to_multithreading_75x_part1 branch from 978abf1 to dca8ada Compare August 27, 2015 07:41

fwyzard added 3 commits August 27, 2015 09:46

fix use of L1GtTriggerMenu

cc5ec5c

L1GtTriggerMenu is now properly initialised by the EventSetup framework, so there is no longer need to call (const_cast<L1GtTriggerMenu *>(l1GtTriggerMenu))->buildGtConditionMap(); or to keep a local copy for that purpose

L1GlobalTrigger: change into a stream::EDProducer

56d36ae

- change most configuration parameters into const members - change into a stream::EDProducer

TriggerResultsFilter: change into a edm::stream::EDFilter

80fa995

fwyzard force-pushed the migrate_to_multithreading_75x_part1 branch from e513792 to 80fa995 Compare August 27, 2015 07:46

cmsbuild added tests-started and removed tests-pending labels Aug 27, 2015

cmsbuild added hlt-approved and removed hlt-pending labels Aug 27, 2015

cmsbuild added tests-approved and removed tests-started labels Aug 27, 2015

cmsbuild added comparison-available and removed comparison-pending labels Aug 27, 2015

fwyzard changed the title ~~migrate to modules used by the HLT menu multithreading (part 1)~~ migrate to modules used by the HLT menu multithreading (various) Aug 30, 2015

cmsbuild added l1-approved and removed l1-pending labels Sep 1, 2015

cmsbuild added dqm-approved and removed dqm-pending labels Sep 1, 2015

cmsbuild added db-approved and removed db-pending labels Sep 1, 2015

davidlange6 added a commit that referenced this pull request Sep 1, 2015

Merge pull request #10958 from fwyzard/migrate_to_multithreading_75x_…

0ebdd65

…part1 migrate to modules used by the HLT menu multithreading (various)

davidlange6 merged commit 0ebdd65 into cms-sw:CMSSW_7_6_X Sep 1, 2015

fwyzard changed the title ~~migrate to modules used by the HLT menu multithreading (various)~~ migrate modules used by the HLT menu to multithreading (various) Sep 4, 2015

fwyzard mentioned this pull request Sep 21, 2015

multithreading fix for 74x: L1GtTriggerMenu-related modules #11354

Merged

fwyzard deleted the migrate_to_multithreading_75x_part1 branch September 23, 2015 19:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate modules used by the HLT menu to multithreading (various) #10958

migrate modules used by the HLT menu to multithreading (various) #10958

fwyzard commented Aug 26, 2015

cmsbuild commented Aug 26, 2015

Dr15Jones Aug 26, 2015

Dr15Jones Aug 26, 2015

Dr15Jones Aug 26, 2015

Dr15Jones commented Aug 26, 2015

fwyzard commented Aug 26, 2015 via email

Dr15Jones commented Aug 26, 2015

fwyzard commented Aug 27, 2015

cmsbuild commented Aug 27, 2015

cmsbuild commented Aug 27, 2015

fwyzard commented Aug 27, 2015

cmsbuild commented Aug 27, 2015

fwyzard commented Aug 27, 2015

fwyzard commented Aug 27, 2015

cmsbuild commented Aug 27, 2015

cmsbuild commented Aug 27, 2015

Dr15Jones commented Aug 27, 2015

fwyzard commented Aug 27, 2015

mulhearn commented Sep 1, 2015

deguio commented Sep 1, 2015

ggovi commented Sep 1, 2015

migrate modules used by the HLT menu to multithreading (various) #10958

migrate modules used by the HLT menu to multithreading (various) #10958

Conversation

fwyzard commented Aug 26, 2015

cmsbuild commented Aug 26, 2015

Dr15Jones Aug 26, 2015

Choose a reason for hiding this comment

Dr15Jones Aug 26, 2015

Choose a reason for hiding this comment

Dr15Jones Aug 26, 2015

Choose a reason for hiding this comment

Dr15Jones commented Aug 26, 2015

fwyzard commented Aug 26, 2015 via email

Dr15Jones commented Aug 26, 2015

fwyzard commented Aug 27, 2015

cmsbuild commented Aug 27, 2015

cmsbuild commented Aug 27, 2015

fwyzard commented Aug 27, 2015

cmsbuild commented Aug 27, 2015

fwyzard commented Aug 27, 2015

fwyzard commented Aug 27, 2015

cmsbuild commented Aug 27, 2015

cmsbuild commented Aug 27, 2015

Dr15Jones commented Aug 27, 2015

fwyzard commented Aug 27, 2015

mulhearn commented Sep 1, 2015

deguio commented Sep 1, 2015

ggovi commented Sep 1, 2015