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

Jump to bottom

Move from tinyxml to tinyxml2 in FWCore-MessageLogger #24503

Merged
merged 6 commits into from Oct 15, 2018
Merged

Move from tinyxml to tinyxml2 in FWCore-MessageLogger #24503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ba854c9
Move from tinyxml to tinyxml2 in FWCore-MessageLogger
mrodozov Sep 11, 2018
ddaef4b
Fix identation and use single printer object
mrodozov Sep 11, 2018
fdd57bf
Move also Rivet tinyxml->tinyxml2
mrodozov Sep 12, 2018
28775f1
Delete document nodes when the doc is used inside loops
mrodozov Sep 12, 2018
592f427
Fix indentation again
mrodozov Sep 12, 2018
7cd589d
Update test xml files to match exactly generated one
mrodozov Sep 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion FWCore/MessageLogger/BuildFile.xml
View file
Open in desktop
@@ -1,6 +1,6 @@
<use name="FWCore/Utilities"/>
<use name="boost"/>
<use name="tinyxml"/>
<use name="tinyxml2"/>
<use name="tbb"/>
<export>
<lib name="1"/>
Expand Down
106 changes: 64 additions & 42 deletions FWCore/MessageLogger/src/JobReport.cc
View file
Open in desktop
Expand Up @@ -23,7 +23,7 @@

// The part of tinyxml used in JobReport was reviewed and
// determined to be threadsafe.
#include "tinyxml.h"
#include "tinyxml2.h"
#include <fstream>
#include <iomanip>
#include <ostream>
Expand All @@ -35,23 +35,24 @@ namespace edm {
* If something outside these classes requires access to the
* same formatting then we need to refactor it into a common library
*/

template <typename S, typename T>
S& formatFile(T const& f, S& os) {

tinyxml2::XMLDocument doc;
if(f.fileHasBeenClosed) {
os << "\n<State Value=\"closed\"/>";
} else {
os << "\n<State Value=\"open\"/>";
}

os << "\n<LFN>" << TiXmlText(f.logicalFileName) << "</LFN>";
os << "\n<PFN>" << TiXmlText(f.physicalFileName) << "</PFN>";
os << "\n<Catalog>" << TiXmlText(f.catalog) << "</Catalog>";
os << "\n<ModuleLabel>" << TiXmlText(f.moduleLabel) << "</ModuleLabel>";
os << "\n<LFN>" << doc.NewText(f.logicalFileName.c_str())->Value() << "</LFN>";
os << "\n<PFN>" << doc.NewText(f.physicalFileName.c_str())->Value() << "</PFN>";
os << "\n<Catalog>" << doc.NewText(f.catalog.c_str())->Value() << "</Catalog>";
os << "\n<ModuleLabel>" << doc.NewText(f.moduleLabel.c_str())->Value() << "</ModuleLabel>";
os << "\n<GUID>" << f.guid << "</GUID>";
os << "\n<Branches>";
for(auto const& branch : f.branchNames) {
os << "\n <Branch>" << TiXmlText(branch) << "</Branch>";
os << "\n <Branch>" << doc.NewText(branch.c_str())->Value() << "</Branch>";
doc.DeleteChildren();
}
os << "\n</Branches>";
return os;
Expand All @@ -63,30 +64,31 @@ namespace edm {
*/
template <typename S>
S& print(S& os, JobReport::InputFile const& f) {

tinyxml2::XMLDocument doc;
os << "\n<InputFile>";
formatFile(f, os);
os << "\n<InputType>" << f.inputType << "</InputType>";
os << "\n<InputSourceClass>" << TiXmlText(f.inputSourceClassName)
os << "\n<InputSourceClass>" << doc.NewText(f.inputSourceClassName.c_str())->Value()
<< "</InputSourceClass>";
os << "\n<EventsRead>" << f.numEventsRead << "</EventsRead>";
return os;
}

template <typename S>
S& print(S& os, JobReport::OutputFile const& f) {
tinyxml2::XMLDocument doc;
formatFile(f, os);
os << "\n<OutputModuleClass>"
<< TiXmlText(f.outputModuleClassName)
<< doc.NewText(f.outputModuleClassName.c_str())->Value()
<< "</OutputModuleClass>";
os << "\n<TotalEvents>"
<< f.numEventsWritten
<< "</TotalEvents>\n";
os << "\n<DataType>"
<< TiXmlText(f.dataType)
<< doc.NewText(f.dataType.c_str())->Value()
<< "</DataType>\n";
os << "\n<BranchHash>"
<< TiXmlText(f.branchHash)
<< doc.NewText(f.branchHash.c_str())->Value()
<< "</BranchHash>\n";
return os;
}
Expand Down Expand Up @@ -222,6 +224,7 @@ namespace edm {
*
*/
void JobReport::JobReportImpl::writeOutputFile(JobReport::OutputFile const& f) {
tinyxml2::XMLDocument doc;
if(ost_) {
*ost_ << "\n<File>";
*ost_ << f;
Expand All @@ -236,18 +239,20 @@ namespace edm {
for(auto token : f.contributingInputs) {
JobReport::InputFile inpFile = inputFiles_.at(token);
*ost_ << "\n<Input>";
*ost_ << "\n <LFN>" << TiXmlText(inpFile.logicalFileName) << "</LFN>";
*ost_ << "\n <PFN>" << TiXmlText(inpFile.physicalFileName) << "</PFN>";
*ost_ << "\n <LFN>" << doc.NewText(inpFile.logicalFileName.c_str())->Value() << "</LFN>";
*ost_ << "\n <PFN>" << doc.NewText(inpFile.physicalFileName.c_str())->Value() << "</PFN>";
*ost_ << "\n <FastCopying>" << findOrDefault(f.fastCopyingInputs, inpFile.physicalFileName) << "</FastCopying>";
*ost_ << "\n</Input>";
doc.DeleteChildren();
}
for(auto token : f.contributingInputsSecSource) {
JobReport::InputFile inpFile = inputFilesSecSource_.at(token);
*ost_ << "\n<Input>";
*ost_ << "\n <LFN>" << TiXmlText(inpFile.logicalFileName) << "</LFN>";
*ost_ << "\n <PFN>" << TiXmlText(inpFile.physicalFileName) << "</PFN>";
*ost_ << "\n <LFN>" << doc.NewText(inpFile.logicalFileName.c_str())->Value() << "</LFN>";
*ost_ << "\n <PFN>" << doc.NewText(inpFile.physicalFileName.c_str())->Value() << "</PFN>";
*ost_ << "\n <FastCopying>" << findOrDefault(f.fastCopyingInputs, inpFile.physicalFileName) << "</FastCopying>";
*ost_ << "\n</Input>";
doc.DeleteChildren();
}
*ost_ << "\n</Inputs>";
*ost_ << "\n</File>\n";
Expand Down Expand Up @@ -505,12 +510,13 @@ namespace edm {

void
JobReport::reportAnalysisFile(std::string const& fileName, std::map<std::string, std::string> const& fileData) {
tinyxml2::XMLDocument doc;
if(impl_->ost_) {
std::ostream& msg = *(impl_->ost_);
{
std::lock_guard<std::mutex> lock(write_mutex);
msg << "<AnalysisFile>\n"
<< " <FileName>" << TiXmlText(fileName) << "</FileName>\n";
<< " <FileName>" << doc.NewText(fileName.c_str())->Value() << "</FileName>\n";

typedef std::map<std::string, std::string>::const_iterator const_iterator;
for(const_iterator pos = fileData.begin(), posEnd = fileData.end(); pos != posEnd; ++pos) {
Expand Down Expand Up @@ -546,12 +552,15 @@ namespace edm {
std::string const& lfn) {
if(impl_->ost_) {
std::ostream& msg = *(impl_->ost_);
TiXmlElement skipped("SkippedFile");
skipped.SetAttribute("Pfn", pfn);
skipped.SetAttribute("Lfn", lfn);
tinyxml2::XMLDocument doc;
tinyxml2::XMLPrinter printer;
tinyxml2::XMLElement * skipped = doc.NewElement("SkippedFile");
skipped->SetAttribute("Pfn", pfn.c_str());
skipped->SetAttribute("Lfn", lfn.c_str());
{
std::lock_guard<std::mutex> lock(write_mutex);
msg << skipped << "\n";
skipped->Accept(&printer);
msg << printer.CStr() ;
msg << std::flush;
}
}
Expand All @@ -561,12 +570,15 @@ namespace edm {
JobReport::reportFallbackAttempt(std::string const& pfn, std::string const& lfn, std::string const& err) {
if(impl_->ost_) {
std::ostream& msg = *(impl_->ost_);
TiXmlElement fallback("FallbackAttempt");
fallback.SetAttribute("Pfn", pfn);
fallback.SetAttribute("Lfn", lfn);
tinyxml2::XMLDocument doc;
tinyxml2::XMLPrinter printer;
tinyxml2::XMLElement * fallback = doc.NewElement("FallbackAttempt");
fallback->SetAttribute("Pfn", pfn.c_str());
fallback->SetAttribute("Lfn", lfn.c_str());
{
std::lock_guard<std::mutex> lock(write_mutex);
msg << fallback << "\n";
fallback->Accept(&printer);
msg << printer.CStr() ;
msg << "<![CDATA[\n" << err << "\n]]>\n";
msg << std::flush;
}
Expand Down Expand Up @@ -611,26 +623,34 @@ namespace edm {
if(impl_->ost_) {
std::ostream& ost = *(impl_->ost_);
ost << "<ReadBranches>\n";
tinyxml2::XMLDocument doc;
tinyxml2::XMLPrinter printer;
for(auto const& iBranch : impl_->readBranches_) {
TiXmlElement branch("Branch");
branch.SetAttribute("Name", iBranch.first);
branch.SetAttribute("ReadCount", iBranch.second);
ost << branch << "\n";
tinyxml2::XMLElement * branch = doc.NewElement("Branch");
branch->SetAttribute("Name", iBranch.first.c_str());
branch->SetAttribute("ReadCount", int64_t(iBranch.second));
branch->Accept(&printer);
ost << printer.CStr();
printer.ClearBuffer();
}
for(auto const& iBranch : impl_->readBranchesSecFile_) {
TiXmlElement branch("Branch");
branch.SetAttribute("Name", iBranch.first);
branch.SetAttribute("ReadCount", iBranch.second);
ost << branch << "\n";
tinyxml2::XMLElement * branch = doc.NewElement("Branch");
branch->SetAttribute("Name", iBranch.first.c_str());
branch->SetAttribute("ReadCount", int64_t(iBranch.second));
branch->Accept(&printer);
ost << printer.CStr();
printer.ClearBuffer();
}
ost << "</ReadBranches>\n";
if(!impl_->readBranchesSecSource_.empty()) {
ost << "<SecondarySourceReadBranches>\n";
for(auto const& iBranch : impl_->readBranchesSecSource_) {
TiXmlElement branch("Branch");
branch.SetAttribute("Name", iBranch.first);
branch.SetAttribute("ReadCount", iBranch.second.value().load());
ost << branch << "\n";
tinyxml2::XMLElement * branch = doc.NewElement("Branch");
branch->SetAttribute("Name", iBranch.first.c_str());
branch->SetAttribute("ReadCount", int64_t(iBranch.second.value().load()));
branch->Accept(&printer);
ost << printer.CStr();
printer.ClearBuffer();
}
ost << "</SecondarySourceReadBranches>\n";
}
Expand Down Expand Up @@ -665,12 +685,13 @@ namespace edm {
}

void JobReport::reportRandomStateFile(std::string const& name) {
tinyxml2::XMLDocument doc;
if(impl_->ost_) {
std::ostream& msg = *(impl_->ost_);
{
std::lock_guard<std::mutex> lock(write_mutex);
msg << "<RandomServiceStateFile>\n"
<< TiXmlText(name) << "\n"
<< doc.NewText(name.c_str())->Value() << "\n"
<< "</RandomServiceStateFile>\n";
msg << std::flush;
}
Expand Down Expand Up @@ -725,7 +746,7 @@ namespace edm {
JobReport::dumpFiles(void) {
std::ostringstream msg;


tinyxml2::XMLDocument doc;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will having the doc outside of the loop mean it will accumulate memory until the loop finishes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The last commit is using a method to delete the document nodes when the doc is used to create new text objects inside loops.

for(auto const& f :impl_->outputFiles_) {

msg << "\n<File>";
Expand All @@ -737,10 +758,11 @@ namespace edm {
for(auto const& iInput : f.contributingInputs) {
auto const& inpFile = impl_->inputFiles_[iInput];
msg << "\n<Input>";
msg << "\n <LFN>" << TiXmlText(inpFile.logicalFileName) << "</LFN>";
msg << "\n <PFN>" << TiXmlText(inpFile.physicalFileName) << "</PFN>";
msg << "\n <LFN>" << doc.NewText(inpFile.logicalFileName.c_str())->Value() << "</LFN>";
msg << "\n <PFN>" << doc.NewText(inpFile.physicalFileName.c_str())->Value() << "</PFN>";
msg << "\n <FastCopying>" << findOrDefault(f.fastCopyingInputs, inpFile.physicalFileName) << "</FastCopying>";
msg << "\n</Input>";
doc.DeleteChildren();
}
msg << "\n</Inputs>";
msg << "\n</File>";
Expand Down
1 change: 0 additions & 1 deletion GeneratorInterface/RivetInterface/BuildFile.xml
View file
Open in desktop
Expand Up @@ -5,7 +5,6 @@
<use name="boost"/>
<use name="gsl"/>
<use name="root"/>
<use name="tinyxml"/>
<use name="GeneratorInterface/Core"/>
<use name="GeneratorInterface/Pythia6Interface"/>
<flags RIVET_PLUGIN="1"/>
1 change: 1 addition & 0 deletions GeneratorInterface/RivetInterface/plugins/BuildFile.xml
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@
<use name="rivet"/>
<use name="yoda"/>
<use name="gsl"/>
<use name="tinyxml2"/>
<use name="DQMServices/Core"/>
<use name="SimGeneral/HepPDTRecord"/>
<use name="DataFormats/HepMCCandidate"/>
Expand Down
27 changes: 13 additions & 14 deletions GeneratorInterface/RivetInterface/plugins/RivetHarvesting.cc
View file
Open in desktop
Expand Up @@ -11,7 +11,7 @@
#include "Rivet/AnalysisHandler.hh"
#include "Rivet/Analysis.hh"
#include "Rivet/Tools/RivetYODA.hh"
#include "tinyxml.h"
#include "tinyxml2.h"

#include <string>
#include <vector>
Expand All @@ -22,6 +22,7 @@
using namespace Rivet;
using namespace edm;
using namespace std;
using namespace tinyxml2;

RivetHarvesting::RivetHarvesting(const edm::ParameterSet& pset) :
_analysisHandler(),
Expand Down Expand Up @@ -186,11 +187,12 @@ void RivetHarvesting::endJob(){
vector<YODA::Point2D> RivetHarvesting::getPoint2DValsErrs(std::string filename, std::string path, std::string name) {

// Open YODA XML file
TiXmlDocument doc(filename);
doc.LoadFile();
XMLDocument doc;
doc.LoadFile(filename.c_str());
//doc.LoadFile();
if (doc.Error()) {
string err = "Error in " + string(doc.Value());
err += ": " + string(doc.ErrorDesc());
string err = "Error in " + filename;
err += ": " + string(doc.ErrorStr());
cerr << err << endl;
throw cms::Exception("RivetHarvesting") << "Cannot open " << filename;
}
Expand All @@ -200,10 +202,9 @@ vector<YODA::Point2D> RivetHarvesting::getPoint2DValsErrs(std::string filename,

try {
// Walk down tree to get to the <paper> element
const TiXmlNode* yodaN = doc.FirstChild("yoda");
const XMLElement* yodaN = doc.FirstChildElement("yoda");
if (!yodaN) throw cms::Exception("RivetHarvesting") << "Couldn't get <yoda> root element";
for (const TiXmlNode* dpsN = yodaN->FirstChild("dataPointSet"); dpsN; dpsN = dpsN->NextSibling()) {
const TiXmlElement* dpsE = dpsN->ToElement();
for (const XMLElement* dpsE = yodaN->FirstChildElement("dataPointSet"); dpsE; dpsE = dpsE->NextSiblingElement()) {
const string plotname = dpsE->Attribute("name");
const string plotpath = dpsE->Attribute("path");
if (plotpath != path && plotname != name)
Expand All @@ -216,12 +217,10 @@ vector<YODA::Point2D> RivetHarvesting::getPoint2DValsErrs(std::string filename,

/// @todo Check that "path" matches filename
vector<Point2D> points;
for (const TiXmlNode* dpN = dpsN->FirstChild("dataPoint"); dpN; dpN = dpN->NextSibling()) {
const TiXmlNode* xMeasN = dpN->FirstChild("measurement");
const TiXmlNode* yMeasN = xMeasN->NextSibling();
if (xMeasN && yMeasN) {
const TiXmlElement* xMeasE = xMeasN->ToElement();
const TiXmlElement* yMeasE = yMeasN->ToElement();
for (const XMLElement* dpE = dpsE->FirstChildElement("dataPoint"); dpE; dpE = dpE->NextSiblingElement()) {
const XMLElement* xMeasE = dpE->FirstChildElement("measurement");
const XMLElement* yMeasE = xMeasE->NextSiblingElement();
if (xMeasE && yMeasE) {
const string xcentreStr = xMeasE->Attribute("value");
const string xerrplusStr = xMeasE->Attribute("errorPlus");
const string xerrminusStr = xMeasE->Attribute("errorMinus");
Expand Down
4 changes: 2 additions & 2 deletions IOPool/Common/test/proper_fjr_output
View file
Open in desktop
Expand Up @@ -79,8 +79,8 @@
</Inputs>
</File>
<ReadBranches>
<Branch Name="edmTriggerResults_TriggerResults__TESTPROD." ReadCount="35" />
<Branch Name="edmtestThings_Thing__TESTPROD." ReadCount="35" />
<Branch Name="edmTriggerResults_TriggerResults__TESTPROD." ReadCount="35"/>
<Branch Name="edmtestThings_Thing__TESTPROD." ReadCount="35"/>
</ReadBranches>

</FrameworkJobReport>
10 changes: 5 additions & 5 deletions IOPool/Common/test/proper_fjrx_output
View file
Open in desktop
Expand Up @@ -85,11 +85,11 @@
</Inputs>
</File>
<ReadBranches>
<Branch Name="edmTriggerResults_TriggerResults__TESTPROD." ReadCount="35" />
<Branch Name="edmtestOtherThings_AOtherThing_testUserTag_TESTPROD." ReadCount="10" />
<Branch Name="edmtestOtherThings_OtherThing_testUserTag_TESTPROD." ReadCount="10" />
<Branch Name="edmtestOtherThings_ZOtherThing_testUserTag_TESTPROD." ReadCount="10" />
<Branch Name="edmtestThings_Thing__TESTPROD." ReadCount="35" />
<Branch Name="edmTriggerResults_TriggerResults__TESTPROD." ReadCount="35"/>
<Branch Name="edmtestOtherThings_AOtherThing_testUserTag_TESTPROD." ReadCount="10"/>
<Branch Name="edmtestOtherThings_OtherThing_testUserTag_TESTPROD." ReadCount="10"/>
<Branch Name="edmtestOtherThings_ZOtherThing_testUserTag_TESTPROD." ReadCount="10"/>
<Branch Name="edmtestThings_Thing__TESTPROD." ReadCount="35"/>
</ReadBranches>

</FrameworkJobReport>