change Tracer::dumpContextForLabel into a vstring

[fwyzard]

No description provided.

[cmsbuild]

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

change Tracer::dumpContextForLabel into a vstring

It involves the following packages:

FWCore/Services

@smuzaffar, @Dr15Jones, @ktf, @nclopezo can you please review it and eventually sign? Thanks.
@wmtan 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.
@ktf you are the release manager for this.

[Dr15Jones]

How about changing the parameter name to 'dumpContextForLabels' and changing the internal data structure from std::vectorstd::string to std::setstd::string so we don't have to do a linear lookup.

[fwyzard]

Will do.

.A

On 24 October 2013 18:09, Chris Jones notifications@github.com wrote:

How about changing the parameter name to 'dumpContextForLabels' and
changing the internal data structure from std::vectorstd::string to
std::setstd::string so we don't have to do a linear lookup.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1168#issuecomment-27005973
.

[fwyzard]

By the way, can someone confirm that the use of std::move on line 47 is correct ?

[cmsbuild]

Pull request #1168 was updated. @smuzaffar, @Dr15Jones, @ktf, @nclopezo can you please check and sign again.

[wmtan]

My non-expert opinion (I did not write this code)
std::move on line 42 is a performance optimization that allows the string to be moved rather than copied, since the source does not need its contents saved.
it is possible that, since the source is a temp, that the compiler would be smart enough to do the move anyway, but the explicit std::move can't hurt.

[Dr15Jones]

+1

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next IBs unless changes or unless it breaks tests. @ktf can you please take care of it?

[nclopezo]

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/1009/summary.html

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next IBs unless changes or unless it breaks tests. @ktf can you please take care of it?

[ktf]

I think this actually breaks a couple of unit tests:

https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/slc5_amd64_gcc481/CMSSW_7_0_X_2013-10-27-1400/unitTestLogs/FWCore/Framework

@fwyzard can you have a look?
@nclopezo how come your tests did not pick it up?

[fwyzard]

Hi Giulio, should be fixed with #1198 , sorry about that, I had only checked the FWCore/Services package itself.

.A

[nclopezo]

Hi All,

I checked and I noticed that when the tests ran, it only checked out FWCore/Services and FWCore/Version, so it didn't run the tests for FWCore/Framework and FWCore/Integration. So it interpreted it as without errors, because the unit test that it ran passed as you can see here:

https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/1009/unitTests.log