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

Sped up principal 'put' #20661

Merged
merged 30 commits into from Oct 2, 2017
Merged

Sped up principal 'put' #20661

merged 30 commits into from Oct 2, 2017

Conversation

Dr15Jones
Copy link
Contributor

Sped up the time it takes to 'put' data into the Event, LuminosityBlock and Run.

Using a configuration with the same topology as the HLT but with trivial modules, the code makes that configuration run 20% faster.

Added move constructor and move assignment operations.
Use move to deal with BranchID and ParentageID.
If a destructor is defined, the compiler is not allowed to generate a move constructor or move assignment.
Only ProducerSourceBase needs the ProductRegistryHelper functionality (it actually better to use ProducerBase).
Keep track of the need to record parentage for a product at the call to produces. This will help simplify the Event::put call.
Keep track of the ProductResolverIndex for each EDPutToken and use  the index directly.
The decision to keep the parentage is now determined at initialization time, not Event::put time.
Instead of calculating the ProductID from the BranchID, get the ProductID cached by the ProductResolver.
Avoided unnecessary synchronization and gave hints to the compiler about the likelihood of dealing with an exception.
Knowing the number of products to be consumed helps reserve space.
As data is being gotten from the Event, check to see if it was recorded in the parentage cache. If not, add to the extra container. If all items were obtained from the parentage cache and no new ones were added, use the cached parentageID.
Extended the number of cases that will cause an exception to be thrown if there is a problem with Event::put.
Also have Event::put using EDGetTokens directly use the index.
Use the typedef instead of the value type in order to help future maintenance.
Check that the type used in Event::put when using EDPutToken is what was expected.
Change Event::getRefBeforePut to use the same new infrastructure as Event::put.
We need to setup the Principal for the end transition before calling the source.
The interfaces using LuminosityBlock and Run will now only work properly if the class inherits from ProducerBase. Therefore these were moved to ProducerSourceBase and the relevant 'do' methods were made virtual.
Also made ProducerSourceBase a friend of ProducerBase to allow it to call commit_.
Run and LuminosityBlock now use EDPutToken indices to implement 'put'.
Created the PuttableSourceBase class to separate the handling of the 'produces' and 'put' interface from ProducerSourceBase. This allows sharing of the implementation with other sources that do not want to use ProducerSourceBase's transition handling implementation.
The 'produces' and 'put' interfaces where removed from InputSource
and put into PuttableSourceBase. This change updates DQMProtobufReader
to follow the change.
@cmsbuild
Copy link
Contributor

@Dr15Jones
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 27, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/23279/console Started: 2017/09/27 23:38

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2698383
  • DQMHistoTests: Total failures: 254
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2697940
  • DQMHistoTests: Total skipped: 189
  • DQMHistoTests: Total Missing objects: 0
  • Checked 107 log files, 15 edm output root files, 26 DQM output files

@Dr15Jones
Copy link
Contributor Author

+1

@arunhep
Copy link
Contributor

arunhep commented Sep 28, 2017

+1

@civanch
Copy link
Contributor

civanch commented Sep 29, 2017

+1

@perrotta
Copy link
Contributor

perrotta commented Oct 2, 2017

+1

  • RecoLuminosity/LumiProducer/plugins/ExpressLumiProducer.cc its the only reco file touched. It chages from
    produces<std::string, edm::InLumi>("...")
    to
    produces<std::string, edm::Transition::BeginLuminosityBlock>("...")
    (which is the same as currently in LumiProducer.cc, by the way)
  • No differences in jenkins tests (none expected)

@davidlange6
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 984ed1f into cms-sw:master Oct 2, 2017
@Dr15Jones Dr15Jones deleted the speedUpPrincipalPut branch October 10, 2017 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment