From: Richard P. <ri...@op...> - 2007-05-09 16:43:01
|
Add callgraph XML output ChangeLog | 10 ++ libpp/format_output.cpp | 178 +++++++++++++++++++++++++++++++++++++++++------ libpp/format_output.h | 40 ++++++++-- libpp/xml_utils.cpp | 17 +--- libutil++/xml_output.cpp | 3 libutil++/xml_output.h | 3 pp/opreport.cpp | 37 +++++++-- pp/opreport_options.cpp | 5 - 8 files changed, 238 insertions(+), 55 deletions(-) Index: oprofile/libpp/format_output.cpp =================================================================== --- oprofile.orig/libpp/format_output.cpp +++ oprofile/libpp/format_output.cpp @@ -562,6 +562,20 @@ ostringstream bytes_out; map<string, size_t> symbol_data_table; size_t symbol_data_index = 0; +/* Return any existing index or add to the table */ +size_t xml_get_symbol_index(string const name) +{ + size_t index = symbol_data_index; + map<string, size_t>::iterator it = symbol_data_table.find(name); + + if (it == symbol_data_table.end()) { + symbol_data_table[name] = symbol_data_index++; + return index; + } + + return it->second; +} + class symbol_details_t { public: @@ -577,14 +591,15 @@ symbol_details_array_t symbol_details; size_t detail_table_index = 0; xml_formatter:: -xml_formatter(profile_container const & p, +xml_formatter(profile_container const *p, symbol_collection & s) : profile(p), symbols(s), need_details(false) { - counts.total = profile.samples_count(); + if (profile) + counts.total = profile->samples_count(); } @@ -640,12 +655,11 @@ void xml_formatter::output_symbol_data(o string const image = get_image_name(symb->image_name, true); string const qname = image + ":" + name; map<string, size_t>::iterator sd_it = symbol_data_table.find(qname); - size_t si = xml_support->get_symbol_index(it); - if (sd_it->second == si) { + if (sd_it != symbol_data_table.end()) { // first time we've seen this symbol out << open_element(SYMBOL_DATA, true); - out << init_attr(TABLE_ID, si); + out << init_attr(TABLE_ID, sd_it->second); field_datum datum(*symb, symb->sample, 0, counts); @@ -660,9 +674,12 @@ void xml_formatter::output_symbol_data(o output_attribute(out, datum, ff_vma, STARTING_ADDR); if (need_details) - xml_support->output_symbol_bytes(bytes_out, symb, si); + xml_support->output_symbol_bytes(bytes_out, symb, sd_it->second); } out << close_element(); + + // seen so remove (otherwise get several "no symbols") + symbol_data_table.erase(qname); } } out << close_element(SYMBOL_TABLE); @@ -675,8 +692,8 @@ output_symbol_details(symbol_entry const if (!has_sample_counts(symb->sample.counts, lo, hi)) return ""; - sample_container::samples_iterator it = profile.begin(symb); - sample_container::samples_iterator end = profile.end(symb); + sample_container::samples_iterator it = profile->begin(symb); + sample_container::samples_iterator end = profile->end(symb); ostringstream str; for (; it != end; ++it) { @@ -725,10 +742,11 @@ output_symbol_details(symbol_entry const void xml_formatter:: output_symbol(ostream & out, - symbol_collection::const_iterator const it, size_t lo, size_t hi) + symbol_entry const * symb, size_t lo, size_t hi, bool is_module) { - symbol_entry const * symb = *it; ostringstream str; + // pointless reference to is_module, remove insane compiler warning + size_t indx = is_module ? 0 : 1; // output symbol's summary data for each profile class bool got_samples = false; @@ -752,27 +770,21 @@ output_symbol(ostream & out, string const image = get_image_name(symb->image_name, true); string const qname = image + ":" + name; - map<string, size_t>::iterator sd_it = symbol_data_table.find(qname); - size_t si = xml_support->get_symbol_index(it); - // if this is the first time we've seen this symbol, save it's index - if (sd_it == symbol_data_table.end()) - symbol_data_table[qname] = si; - else - si = sd_it->second; + indx = xml_get_symbol_index(qname); - out << init_attr(ID_REF, si); + out << init_attr(ID_REF, indx); if (need_details) { ostringstream details; - symbol_details_t & sd = symbol_details[si]; + symbol_details_t & sd = symbol_details[indx]; size_t const detail_lo = sd.index; string detail_str = output_symbol_details(symb, sd.index, lo, hi); if (detail_str.size() > 0) { if (sd.id < 0) - sd.id = si; + sd.id = indx; details << detail_str; } @@ -828,5 +840,131 @@ output_attribute(ostream & out, field_da } } +xml_cg_formatter:: +xml_cg_formatter(callgraph_container const * cg, symbol_collection & s) + : + xml_formatter(NULL, s), + callgraph(cg) +{ + counts.total = callgraph->samples_count(); +} + +void xml_cg_formatter::output(ostream & out) +{ + xml_support->build_subclasses(out); + + xml_support->output_program_structure(out); + output_symbol_data(out); + + out << close_element(PROFILE); +} + +void xml_cg_formatter:: +output_symbol_core(ostream & out, cg_symbol::children const cg_symb, + string const selfname, string const qname, + size_t lo, size_t hi, bool is_module, tag_t tag) +{ + + cg_symbol::children::const_iterator cit; + cg_symbol::children::const_iterator cend = cg_symb.end(); + + for (cit = cg_symb.begin(); cit != cend; ++cit) { + string binary = get_image_name((cit)->app_name, true); + string module = get_image_name((cit)->image_name, true); + bool got_samples = false, self = false; + ostringstream str; + size_t indx; + + for (size_t p = lo; p <= hi; ++p) + got_samples |= xml_support->output_summary_data(str, cit->sample.counts, p); + + if (!got_samples) + continue; + + cverb << vxml << " <!-- symbol_ref=" << symbol_names.name(cit->name) << " -->" << endl; + + if (is_module) { + out << open_element(MODULE, true); + out << init_attr(NAME, module) << close_element(NONE, true); + } + + out << open_element(SYMBOL, true); + + string const symname = symbol_names.name(cit->name); + assert(symname.size() > 0); + + string const symqname = module + ":" + symname; + + // Find any self references and handle + if ((symname == selfname) && (tag == CALLEES)) { + self = true; + indx = xml_get_symbol_index(qname); + } else + indx = xml_get_symbol_index(symqname); + + out << init_attr(ID_REF, indx); + + if (self) + out << init_attr(SELFREF, "true"); + + out << close_element(NONE, true); + out << str.str(); + out << close_element(SYMBOL); + + if (is_module) + out << close_element(MODULE); + } +} + + +void xml_cg_formatter:: +output_symbol(ostream & out, + symbol_entry const * symb, size_t lo, size_t hi, bool is_module) +{ + cg_symbol const * cg_symb = dynamic_cast<const cg_symbol *>(symb); + ostringstream str; + + // output symbol's summary data for each profile class + bool got_samples = false; + + for (size_t p = lo; p <= hi; ++p) { + got_samples |= xml_support->output_summary_data(str, + symb->sample.counts, p); + } + + if (!got_samples) + return; + + cverb << vxml << " <!-- symbol_ref=" << symbol_names.name(symb->name) << " -->" << endl; + + out << open_element(SYMBOL, true); + + string const name = symbol_names.name(symb->name); + assert(name.size() > 0); + + string const image = get_image_name(symb->image_name, true); + string const qname = image + ":" + name; + + string const selfname = symbol_names.demangle(symb->name) + " [self]"; + + out << init_attr(ID_REF, xml_get_symbol_index(qname)); + + out << close_element(NONE, true); + + out << open_element(CALLERS); + if (cg_symb) + output_symbol_core(out, cg_symb->callers, selfname, qname, lo, hi, is_module, CALLERS); + out << close_element(CALLERS); + + out << open_element(CALLEES); + if (cg_symb) + output_symbol_core(out, cg_symb->callees, selfname, qname, lo, hi, is_module, CALLEES); + + out << close_element(CALLEES); + + // output summary + out << str.str(); + out << close_element(SYMBOL); +} } // namespace format_output Index: oprofile/libpp/format_output.h =================================================================== --- oprofile.orig/libpp/format_output.h +++ oprofile/libpp/format_output.h @@ -227,7 +227,7 @@ private: class xml_formatter : public formatter { public: /// build a ready to use formatter - xml_formatter(profile_container const & profile, + xml_formatter(profile_container const * profile, symbol_collection & symbols); // output body of XML output @@ -235,9 +235,9 @@ public: /** output one symbol symb to out according to the output format * specifier previously set by call(s) to add_format() */ - void output_symbol(std::ostream & out, - symbol_collection::const_iterator const it, - size_t lo, size_t hi); + virtual void output_symbol(std::ostream & out, + symbol_entry const * symb, size_t lo, size_t hi, + bool is_module); /// output details for the symbol std::string output_symbol_details(symbol_entry const * symb, @@ -246,9 +246,12 @@ public: /// set the output_details boolean void show_details(bool); + // output SymbolData XML elements + void output_symbol_data(std::ostream & out); + private: /// container we work from - profile_container const & profile; + profile_container const * profile; // ordered collection of symbols associated with this profile symbol_collection & symbols; @@ -256,9 +259,6 @@ private: /// true if we need to show details for each symbols bool need_details; - // output SymbolData XML elements - void output_symbol_data(std::ostream & out); - // count of DetailData items output so far size_t detail_count; @@ -270,6 +270,30 @@ private: format_flags fl, tag_t tag); }; +// callgraph XML output version +class xml_cg_formatter : public xml_formatter { +public: + /// build a ready to use formatter + xml_cg_formatter(callgraph_container const * callgraph, + symbol_collection & symbols); + + // output body of XML output + void output(std::ostream & out); + + /** output one symbol symb to out according to the output format + * specifier previously set by call(s) to add_format() */ + virtual void output_symbol(std::ostream & out, + symbol_entry const * symb, size_t lo, size_t hi, bool is_module); + +private: + /// container we work from + callgraph_container const * callgraph; + + void output_symbol_core(std::ostream & out, + cg_symbol::children const cg_symb, + std::string const selfname, std::string const qname, + size_t lo, size_t hi, bool is_module, tag_t tag); +}; } // namespace format_output Index: oprofile/libpp/xml_utils.cpp =================================================================== --- oprofile.orig/libpp/xml_utils.cpp +++ oprofile/libpp/xml_utils.cpp @@ -268,13 +268,6 @@ void xml_utils::output_xml_header(string cout << close_element(SETUP) << endl; } -size_t xml_utils::get_symbol_index(sym_iterator const it) -{ - return it - symbols_begin; -} - - - class subclass_info_t { public: string unitmask; @@ -443,7 +436,7 @@ public: bool is_closed(string const & n); protected: void output_summary(ostream & out); - void output_symbols(ostream & out); + void output_symbols(ostream & out, bool is_module); string name; sym_iterator begin; @@ -593,7 +586,7 @@ void module_info::output(ostream & out) out << open_element(MODULE, true); out << init_attr(NAME, name) << close_element(NONE, true); output_summary(out); - output_symbols(out); + output_symbols(out, true); out << close_element(MODULE); } @@ -605,13 +598,13 @@ void module_info::output_summary(ostream } -void module_info::output_symbols(ostream & out) +void module_info::output_symbols(ostream & out, bool is_module) { if (begin == (sym_iterator)0) return; for (sym_iterator it = begin; it != end; ++it) - xml_out->output_symbol(out, it, lo, hi); + xml_out->output_symbol(out, *it, lo, hi, is_module); } @@ -791,7 +784,7 @@ void binary_info::output(ostream & out) out << init_attr(NAME, name) << close_element(NONE, true); output_summary(out); - output_symbols(out); + output_symbols(out, false); for (size_t a = 0; a < nr_modules; ++a) my_modules[a].output(out); Index: oprofile/libutil++/xml_output.cpp =================================================================== --- oprofile.orig/libutil++/xml_output.cpp +++ oprofile/libutil++/xml_output.cpp @@ -47,8 +47,11 @@ string const xml_tag_map[] = { "binary", "module", "name", + "callers", + "callees", "symbol", "idref", + "self", "detaillo", "detailhi", "symboltable", Index: oprofile/libutil++/xml_output.h =================================================================== --- oprofile.orig/libutil++/xml_output.h +++ oprofile/libutil++/xml_output.h @@ -28,7 +28,8 @@ typedef enum { THREAD, THREAD_ID, BINARY, MODULE, NAME, - SYMBOL, ID_REF, DETAIL_LO, DETAIL_HI, + CALLERS, CALLEES, + SYMBOL, ID_REF, SELFREF, DETAIL_LO, DETAIL_HI, SYMBOL_TABLE, SYMBOL_DATA, STARTING_ADDR, SOURCE_FILE, SOURCE_LINE, CODE_LENGTH, Index: oprofile/pp/opreport.cpp =================================================================== --- oprofile.orig/pp/opreport.cpp +++ oprofile/pp/opreport.cpp @@ -378,7 +378,7 @@ void output_symbols(profile_container co format_output::opreport_formatter * text_out = 0; if (options::xml) { - xml_out = new format_output::xml_formatter(pc, symbols); + xml_out = new format_output::xml_formatter(&pc, symbols); xml_out->show_details(options::details); out = xml_out; // for XML always output long filenames @@ -450,21 +450,40 @@ void output_cg_symbols(callgraph_contain options::sort_by.sort(symbols, options::reverse_sort, options::long_filenames); - format_output::cg_formatter out(cg); + format_output::formatter * out; + format_output::xml_cg_formatter * xml_out = 0; + format_output::cg_formatter * text_out = 0; - out.set_nr_classes(nr_classes); - out.show_long_filenames(options::long_filenames); - out.show_header(options::show_header); - out.vma_format_64bit(output_hints & cf_64bit_vma); - out.show_global_percent(options::global_percent); + if (options::xml) { + xml_out = new format_output::xml_cg_formatter(&cg, symbols); + out = xml_out; + // for XML always output long filenames + out->show_long_filenames(true); + } else { + text_out = new format_output::cg_formatter(cg); + out = text_out; + out->show_long_filenames(options::long_filenames); + } + + out->set_nr_classes(nr_classes); + out->show_header(options::show_header); + out->vma_format_64bit(output_hints & cf_64bit_vma); + out->show_global_percent(options::global_percent); format_flags flags = get_format_flags(output_hints); if (multiple_apps) flags = format_flags(flags | ff_app_name); - out.add_format(flags); + out->add_format(flags); + + if (options::xml) { + xml_support = new xml_utils(xml_out, symbols, nr_classes, + &options::symbol_filter, options::archive_path); + xml_out->output(cout); + } else { + text_out->output(cout, symbols); + } - out.output(cout, symbols); } Index: oprofile/pp/opreport_options.cpp =================================================================== --- oprofile.orig/pp/opreport_options.cpp +++ oprofile/pp/opreport_options.cpp @@ -177,11 +177,6 @@ void check_options(bool diff) } if (xml) { - if (callgraph) { - cerr << "--callgraph is incompatible with --xml" << endl; - do_exit = true; - } - if (accumulated) { cerr << "--accumulated is incompatible with --xml" << endl; do_exit = true; Index: oprofile/ChangeLog =================================================================== --- oprofile.orig/ChangeLog +++ oprofile/ChangeLog @@ -1,5 +1,15 @@ 2007-05-09 Richard Purdie <rp...@op...> + * libpp/format_output.cpp: + * libpp/format_output.h: + * libpp/xml_utils.cpp: + * libutil++/xml_output.cpp: + * libutil++/xml_output.h: + * pp/opreport.cpp: + * pp/opreport_options.cpp: Add callgraph XML output + +2007-05-09 Richard Purdie <rp...@op...> + * libpp/callgraph_container.cpp: * libpp/callgraph_container.h: * libpp/format_output.cpp: |
From: John L. <le...@mo...> - 2007-05-21 00:55:44
|
On Wed, May 09, 2007 at 05:42:49PM +0100, Richard Purdie wrote: Add callgraph XML output Could we see an example output? You'll need to change opreport.xsd in the patch too. +/* Return any existing index or add to the table */ +size_t xml_get_symbol_index(string const name) string const & name +xml_formatter(profile_container const *p, const * p + // seen so remove (otherwise get several "no symbols") + symbol_data_table.erase(qname); I don't understand why we're doing this? Can you explain the issue... } } +xml_cg_formatter:: +xml_cg_formatter(callgraph_container const * cg, symbol_collection & s) + : + xml_formatter(NULL, s), Use 0 not NULL. Is it definitely the right thing for xml_cg_formatter to derive from xml_formatter rather than just 'formatter'? Do we share that much code? The fact that you have to change this code indicates that a inheritance relationship is not necessarily the best choice. +void xml_cg_formatter:: +output_symbol_core(ostream & out, cg_symbol::children const cg_symb, const & cg_symb + string const selfname, string const qname, const & for both + size_t lo, size_t hi, bool is_module, tag_t tag) +{ + Spurious blank line. + cg_symbol::children::const_iterator cit; + cg_symbol::children::const_iterator cend = cg_symb.end(); + + for (cit = cg_symb.begin(); cit != cend; ++cit) { + string binary = get_image_name((cit)->app_name, true); + string module = get_image_name((cit)->image_name, true); string const & wherever possible. + bool got_samples = false, self = false; One variable declaration per line please. + ostringstream str; + size_t indx; + + for (size_t p = lo; p <= hi; ++p) + got_samples |= xml_support->output_summary_data(str, cit->sample.counts, p); + + if (!got_samples) + continue; + + cverb << vxml << " <!-- symbol_ref=" << symbol_names.name(cit->name) << " -->" << endl; This line same comment as other place. + if (is_module) { + out << open_element(MODULE, true); + out << init_attr(NAME, module) << close_element(NONE, true); + } + + out << open_element(SYMBOL, true); + + string const symname = symbol_names.name(cit->name); + assert(symname.size() > 0); + + string const symqname = module + ":" + symname; + + // Find any self references and handle + if ((symname == selfname) && (tag == CALLEES)) { + self = true; + indx = xml_get_symbol_index(qname); + } else + indx = xml_get_symbol_index(symqname); } else { ... } in this case. +void xml_cg_formatter:: +output_symbol(ostream & out, + symbol_entry const * symb, size_t lo, size_t hi, bool is_module) +{ + cg_symbol const * cg_symb = dynamic_cast<const cg_symbol *>(symb); Consistent 'const' placement please. + cverb << vxml << " <!-- symbol_ref=" << symbol_names.name(symb->name) << " -->" << endl; This differs slightly from xml_formatter::output_symbol. +// callgraph XML output version +class xml_cg_formatter : public xml_formatter { +public: + /// build a ready to use formatter + xml_cg_formatter(callgraph_container const * callgraph, + symbol_collection & symbols); + + // output body of XML output + void output(std::ostream & out); + + /** output one symbol symb to out according to the output format + * specifier previously set by call(s) to add_format() */ + virtual void output_symbol(std::ostream & out, + symbol_entry const * symb, size_t lo, size_t hi, bool is_module); + +private: + /// container we work from + callgraph_container const * callgraph; + + void output_symbol_core(std::ostream & out, + cg_symbol::children const cg_symb, + std::string const selfname, std::string const qname, + size_t lo, size_t hi, bool is_module, tag_t tag); +}; } // namespace format_output Index: oprofile/libpp/xml_utils.cpp =================================================================== --- oprofile.orig/libpp/xml_utils.cpp +++ oprofile/libpp/xml_utils.cpp @@ -268,13 +268,6 @@ void xml_utils::output_xml_header(string cout << close_element(SETUP) << endl; } -size_t xml_utils::get_symbol_index(sym_iterator const it) -{ - return it - symbols_begin; -} - - - class subclass_info_t { public: string unitmask; @@ -443,7 +436,7 @@ public: bool is_closed(string const & n); protected: void output_summary(ostream & out); - void output_symbols(ostream & out); + void output_symbols(ostream & out, bool is_module); string name; sym_iterator begin; @@ -593,7 +586,7 @@ void module_info::output(ostream & out) out << open_element(MODULE, true); out << init_attr(NAME, name) << close_element(NONE, true); output_summary(out); - output_symbols(out); + output_symbols(out, true); out << close_element(MODULE); } @@ -605,13 +598,13 @@ void module_info::output_summary(ostream } -void module_info::output_symbols(ostream & out) +void module_info::output_symbols(ostream & out, bool is_module) { if (begin == (sym_iterator)0) return; for (sym_iterator it = begin; it != end; ++it) - xml_out->output_symbol(out, it, lo, hi); + xml_out->output_symbol(out, *it, lo, hi, is_module); } @@ -791,7 +784,7 @@ void binary_info::output(ostream & out) out << init_attr(NAME, name) << close_element(NONE, true); output_summary(out); - output_symbols(out); + output_symbols(out, false); for (size_t a = 0; a < nr_modules; ++a) my_modules[a].output(out); |
From: Richard P. <ri...@op...> - 2007-05-21 23:16:41
|
I've sent an updated patch and have responded to comments below. Where I've not referred to anything below, it should be addressed in the new patch. On Mon, 2007-05-21 at 02:00 +0100, John Levon wrote: > On Wed, May 09, 2007 at 05:42:49PM +0100, Richard Purdie wrote: > > Add callgraph XML output > > Could we see an example output? http://folks.o-hand.com/richard/oprofile_cg.log.gz Let me know if you want anything specific. > + // seen so remove (otherwise get several "no symbols") > + symbol_data_table.erase(qname); > > I don't understand why we're doing this? Can you explain the issue... We only want each symbol of a particular ID printed once in the symbol table. If we have a library with no symbols, several symbols can all end up using the same ID for a given libraries "(no symbols)" entry. You can't just iterate through the symbol table as some symbols don't get listed, only the ones referenced elsewhere in the xml are. The easiest solution ended up being to remove them from the table when printed. If you don't do this, the result is the same symbol being printed multiple times. > +xml_cg_formatter:: > +xml_cg_formatter(callgraph_container const * cg, symbol_collection & s) > + : > + xml_formatter(NULL, s), > > Use 0 not NULL. Is it definitely the right thing for xml_cg_formatter to > derive from xml_formatter rather than just 'formatter'? Do we share that > much code? The fact that you have to change this code indicates that a > inheritance relationship is not necessarily the best choice. I do think it makes sense. In order to prove that, I experimented a bit with adding detail support to the callgraph xml. I can't quite get it 100% working but I've included a proof of concept at the end of the patch series. That patch reuses almost all the code. You'll also note the NULL/0 above is replaced by something better in that patch although I don't like the fact I had to keep profile_container around really. Any tips on how to improve that patch and fix its bug welcome... Cheers, Richard |