Cache the mapping from type_info to TypeWithDict for use in ObjectWithDict::dynamicType()

[dan131riley]

In "lazy" evaluation mode (which @Dr15Jones wishes people wouldn't use), the expression evaluator calls ObjectWithDict::dynamicType() on every evaluation to check for virtual function overrides. That call creates a new TypeWithDict for the real type of the variable using its std::type_info and TClass::GetClass(). GetClass() traverses the ROOT global class list, taking at least a read lock on the list, leading to stack traces like this:

#11 0x00007fefeb52120a in ROOT::TReadLockGuard::TReadLockGuard (this=0x7fefb4738c20, mutex=0x7fefcf9619e0) at include/TVirtualRWMutex.h:107
#12 0x00007fefeacb1efc in TClass::GetClass (typeinfo=..., load=true) at core/meta/src/TClass.cxx:3111
#13 0x00007fefeba1886c in edm::TypeWithDict::TypeWithDict(std::type_info const&, long) () from libFWCoreUtilities.so
#14 0x00007fefeb9f5ee0 in edm::ObjectWithDict::dynamicType() const () from libFWCoreUtilities.so
#15 0x00007fefcc05aa0f in reco::parser::LazyInvoker::invokeLast(edm::ObjectWithDict const&, std::vector<edm::ObjectWithDict, std::allocator<edm::ObjectWithDict> >&) const () from libCommonToolsUtils.so
#16 0x00007fefcc03cfe7 in reco::parser::ExpressionLazyVar::value(edm::ObjectWithDict const&) const () from libCommonToolsUtils.so
#17 0x00007fefcc05dd19 in reco::parser::BinarySelector::operator()(edm::ObjectWithDict const&) const () from libCommonToolsUtils.so

In random stack samples and in the profiling tool "vtune", these calls are one of the leading causes of ROOT lock contention in a standard RAW2DIGI,L1Reco,RECO,EI,PAT job. This PR adds a map from std::type_index to TypeWithDict, which is used to cache the results of these lookups. A new function TypeWithDict::byTypeInfo() populates and interrogates the map.

Currently only ObjectWithDict::dynamicType() has been modified to use this cache. While other uses of the TypeWithDict constructors could be modified to use this, in practice this PR plus #23058 are, in my test workflow, sufficient to eliminate all per-event calls to GetClass due to TypeWithDict construction.

By removing potential lock contention for "lazy" uses of the cut parser, this PR improves the throughput and efficiency of my 24-core RECO test job by a modest but consistent 3%.

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23059/4464

[cmsbuild]

A new Pull Request was created by @dan131riley (Dan Riley) for master.

It involves the following packages:

FWCore/Utilities

@cmsbuild, @smuzaffar, @Dr15Jones can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @wddgit, @Martin-Grunewald this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[Dr15Jones]

@dan131riley Just checking, this pull request alone will not see a speedup since the speedup also requires a change to the parser to call the new function?

Answered myself: the parser calls ObjectWithDict::dynamicType() and that function was updated to use the new cache. Therefore this change is sufficient to gain the speed up.

[Dr15Jones]

How about switching to the more modern, using construct?

using TypeIndexMap = tbb::concurrent_unordered_map<std::type_index, TypeWithDict, Hasher>;

[Dr15Jones]

While there, please go ahead and change the other typedefs as well. :).

[dan131riley]

@Dr15Jones the LazyInvoker calls ObjectWithDict::dynamicType(), which now calls the new function, so no change to the parser is necessary.

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23059/4465

[cmsbuild]

Pull request #23059 was updated. @cmsbuild, @smuzaffar, @Dr15Jones can you please check and sign again.

[Dr15Jones]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/27657/console Started: 2018/04/25 22:41

[cmsbuild]

+1
Tested at: f0f5f9a
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23059/27657/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 29
• DQMHistoTests: Total histograms compared: 2494144
• DQMHistoTests: Total failures: 11
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2493957
• DQMHistoTests: Total skipped: 176
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 28 files compared)
• Checked 119 log files, 9 edm output root files, 29 DQM output files

[Dr15Jones]

+1

[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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

[fabiocos]

+1