Re: [PATCH] Fix callgraph XML output so that all cg symbols are in the report

[Maynard J. <may...@us...>]

Richard,
Please review this patch as soon as possible for inclusion in the next oprofile 
release.

Thank you.
-Maynard

Maynard Johnson wrote:
> Under certain circumstances, symbols that show up as callee or caller in 
> a text-based callgraph report do not show up in an XML-based callgraph 
> report. For example, for the call chain of b()->bb()->bbb->bbbb()->c(), 
> where there were zero samples taken for bb, bbb, and bbbb, the bbb() 
> symbol does not show up in the XML-based report.  This gives GUI tools 
> problems when trying to visualize the call tree using the XML data.
> 
> This problem was reported in oprofile bug 2727485 
> (https://sourceforge.net/tracker/?func=detail&atid=116191&aid=2727485&group_id=16191). 
>  The submitter has tested the attached patch and given it a thumbs-up.
> 
> The attached patch makes two changes:
>   1) xml_cg_formatter::output_symbol is modified so that all symbols are 
> recorded in the symbol data table, even if they were only a callee or 
> caller and had zero samples.  This filtering had been done by iterating 
> through the profile classes (lo and hi) passed in, calling 
> xml_support->output_summary_data to check if any samples were collected 
> for the given symbol and profile class.  This filter is now removed.
>   2) xml_cg_formatter::output_symbol_core is called only for symbols in 
> the callee and caller lists.  This function, too, did profile 
> class-based filtering of symbols by calling 
> xml_support->output_summary_data (although in this case, the summary is 
> the number of times the symbol appears in the callee or caller lists of 
> other symbols).  Since we removed profile class-based filtering on the 
> "main" symbol, it did not make sense to allow such filtering of their 
> callee/caller children, so I removed it.  This entailed removal of the 
> "lo" and "hi" profile class parameters from output_symbol_core().  But 
> then, the "lo" and "hi" parameters of xml_cg_formatter::output_symbol 
> were giving build problems as being unused.  Since this is an 
> implementation of the virtual function xml_formatter:output_symbol, I 
> couldn't remove the parameters, so I just marked them "__attribute__ 
> ((unused))".
> 
> 
> Comments welcome.  I would especially like *Richard* to review this and 
> test it out with the OProfile-GUI.
> 
> Thanks.
> -Maynard
> 
> 
> 
> diff -paur oprof-orig/ChangeLog oprof-fix/ChangeLog
> --- oprof-orig/ChangeLog	2009-05-29 18:39:22.000000000 -0500
> +++ oprof-fix/ChangeLog	2009-05-29 18:41:50.000000000 -0500
> @@ -1,3 +1,9 @@
> +2009-05-29  Maynard Johnson  <may...@us...>
> +
> +	* libpp/format_output.h:
> +	* libpp/format_output.cpp: Make sure that all callgraph symbols are
> +	  reported, even if 0 samples
> +
>  2009-05-27 Suravee Suthikulpanit <sur...@am...>
> 
>  	* utils/opcontrol: Fix IBS initialization
> diff -paur oprof-orig/libpp/format_output.cpp oprof-fix/libpp/format_output.cpp
> --- oprof-orig/libpp/format_output.cpp	2009-05-29 18:37:59.000000000 -0500
> +++ oprof-fix/libpp/format_output.cpp	2009-05-29 18:44:01.000000000 -0500
> @@ -942,7 +942,7 @@ xml_cg_formatter(callgraph_container con
>  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)
> +	bool is_module, tag_t tag)
>  {
>  	cg_symbol::children::const_iterator cit;
>  	cg_symbol::children::const_iterator cend = cg_symb.end();
> @@ -950,17 +950,10 @@ output_symbol_core(ostream & out, cg_sym
>  	for (cit = cg_symb.begin(); cit != cend; ++cit) {
>  		string const & module = get_image_name((cit)->image_name,
>  			image_name_storage::int_filename, extra_found_images);
> -		bool got_samples = false;
>  		bool 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;
> -
>  		if (cverb << vxml)
>  			out << "<!-- symbol_ref=" << symbol_names.name(cit->name) <<
>  				" -->" << endl;
> @@ -1002,23 +995,13 @@ output_symbol_core(ostream & out, cg_sym
> 
>  void xml_cg_formatter::
>  output_symbol(ostream & out,
> -	symbol_entry const * symb, size_t lo, size_t hi, bool is_module)
> +	symbol_entry const * symb, size_t lo __attribute__ ((unused)),
> +	size_t hi __attribute__ ((unused)), bool is_module)
>  {
>  	cg_symbol const * cg_symb = dynamic_cast<cg_symbol const *>(symb);
>  	ostringstream str;
>  	size_t indx;
> 
> -	// 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;
> -
>  	if (cverb << vxml)
>  		out << "<!-- symbol_ref=" << symbol_names.name(symb->name) <<
>  			" -->" << endl;
> @@ -1042,12 +1025,12 @@ output_symbol(ostream & out,
> 
>  	out << open_element(CALLERS);
>  	if (cg_symb)
> -		output_symbol_core(out, cg_symb->callers, selfname, qname, lo, hi, is_module, CALLERS);
> +		output_symbol_core(out, cg_symb->callers, selfname, qname, 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);
> +		output_symbol_core(out, cg_symb->callees, selfname, qname, is_module, CALLEES);
> 
>  	out << close_element(CALLEES);
> 
> diff -paur oprof-orig/libpp/format_output.h oprof-fix/libpp/format_output.h
> --- oprof-orig/libpp/format_output.h	2009-05-29 18:37:59.000000000 -0500
> +++ oprof-fix/libpp/format_output.h	2009-05-29 18:38:40.000000000 -0500
> @@ -315,7 +315,7 @@ private:
>  	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);
> +		bool is_module, tag_t tag);
>  };
> 
>  } // namespace format_output 
> 
>