Re: [RFC] Make opreport use <cur_dir>/oprofile data for default session-dir
Maynard Johnson <maynardj <at> us.ibm.com>
2012-07-03 19:30:30 GMT
On 06/29/2012 11:23 AM, Maynard Johnson wrote:
> On 06/26/2012 02:47 PM, Maynard Johnson wrote:
>> As those of you who have played with operf already know, when the profiling
>> session ends, operf prints the following message:
>>
>> -----------------
>> Use '--session-dir=/home/≤user_name>/oprofile_data'
>> with opreport and other post-processing tools to view your profile data.
>> -----------------
>>
>> This is eventually going to be very annoying to users, and I hate to start
>> a precedent that will be hard to get rid of in the future. So, in light
>> of the new "operf" reality that will soon be the normal oprofile usage
>> model, I have attached below a proposal to change how opreport decides
>> where to look for sample data. Since the proposed implementation below
>> performs a bit of magic behind the curtain, I think one possible change
>> that could be helpful to the user is for the oprofile post-processing
>> tool to print a message indicating the session-dir it ends up using.
>>
>> I would very much appreciate comments from the community.
>
> I'll give this topic a few more days to ruminate and then I'll commit the patch to the perf-events branch.
Speak up now if you have any concerns.
I committed this patch to the perf-events branch with the following minor changes:
- Updated the man pages for opreport, opannotate, and oparchive to reflect the change in default session-dir
- Changed operf's "Profiling started" and "Profiling done" messages to go to stderr vs stdout
Review comments are still welcome.
-Maynard
>
> -Maynard
>
>>
>>
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> [RFC] Make opreport use <cur_dir>/oprofile data for default session-dir
>>
>> Since operf will be the recommended profiling tool in the future
>> (instead of opcontrol) and since operf stores the sample data by
>> default in <cur_dir>/oprofile_data, it makes sense for opreport
>> and other oprofile post-processing tools to look first in
>> <cur_dir/oprofile_data; then, if no samples are found there, the
>> tool should fall back to the "standard" /var/lib/oprofile session-dir.
>>
>> Signed-off-by: Maynard Johnson <maynardj <at> us.ibm.com>
>> ---
>> doc/operf.1.in | 3 +-
>> libpp/profile_spec.cpp | 2 +-
>> libutil++/op_exception.cpp | 9 ++++++++
>> libutil++/op_exception.h | 5 ++++
>> pe_profiling/operf.cpp | 4 +--
>> pp/common_option.cpp | 45 ++++++++++++++++++++++++++++++++++++++++++-
>> pp/common_option.h | 6 +++++
>> pp/opannotate_options.cpp | 11 +++++++++-
>> pp/oparchive_options.cpp | 19 +++++++++++++++++-
>> pp/opreport_options.cpp | 14 +++++++++++-
>> 10 files changed, 107 insertions(+), 11 deletions(-)
>>
>> diff --git a/doc/operf.1.in b/doc/operf.1.in
>> index 0de46b1..e3a1d7f 100644
>> --- a/doc/operf.1.in
>> +++ b/doc/operf.1.in
>> <at> <at> -97,7 +97,8 <at> <at> be added to it, and the 'previous' profile (if one existed) will remain untouche
>> To access the 'previous' profile, simply add a session specification to the normal
>> invocation of oprofile post-processing tools. For example:
>> .br
>> -.I " opreport --session-dir=`pwd`/oprofile_data session:previous"
>> +.BI " opreport"
>> +.BI session:previous
>> .br
>> .TP
>> .BI "--events / -e " event1[,event2[,...]]
>> diff --git a/libpp/profile_spec.cpp b/libpp/profile_spec.cpp
>> index 0a77a74..b699671 100644
>> --- a/libpp/profile_spec.cpp
>> +++ b/libpp/profile_spec.cpp
>> <at> <at> -563,7 +563,7 <at> <at> list<string> profile_spec::generate_file_list(bool exclude_dependent,
>> ostringstream os;
>> os << "No sample file found: try running opcontrol --dump\n"
>> << "or specify a session containing sample files\n";
>> - throw op_fatal_error(os.str());
>> + throw op_no_samples_exception(os.str());
>> }
>>
>> list<string> result;
>> diff --git a/libutil++/op_exception.cpp b/libutil++/op_exception.cpp
>> index 235b9bc..c78ef66 100644
>> --- a/libutil++/op_exception.cpp
>> +++ b/libutil++/op_exception.cpp
>> <at> <at> -30,6 +30,15 <at> <at> char const * op_exception::what() const throw()
>> return message.c_str();
>> }
>>
>> +op_no_samples_exception::op_no_samples_exception(string const & msg)
>> + :
>> + op_exception(msg)
>> +{
>> +}
>> +
>> +op_no_samples_exception::~op_no_samples_exception(void) throw()
>> +{
>> +}
>>
>> op_fatal_error::op_fatal_error(string const & msg)
>> :
>> diff --git a/libutil++/op_exception.h b/libutil++/op_exception.h
>> index af344ed..f51ff4e 100644
>> --- a/libutil++/op_exception.h
>> +++ b/libutil++/op_exception.h
>> <at> <at> -35,6 +35,11 <at> <at> private:
>> std::string message;
>> };
>>
>> +class op_no_samples_exception : public op_exception {
>> +public:
>> + explicit op_no_samples_exception(std::string const & msg);
>> + ~op_no_samples_exception() throw();
>> +};
>>
>> /**
>> * fatal exception, never catch it except at top level (likely main or
>> diff --git a/pe_profiling/operf.cpp b/pe_profiling/operf.cpp
>> index ff9334b..41a8a36 100644
>> --- a/pe_profiling/operf.cpp
>> +++ b/pe_profiling/operf.cpp
>> <at> <at> -1595,9 +1595,7 <at> <at> int main(int argc, char * const argv[])
>> cerr << "WARNING: Profile results may be incomplete due to to abend of profiled app." << endl;
>> }
>> } else {
>> - cout << endl << "Use '--session-dir=" << operf_options::session_dir << "'" << endl
>> - << "with opreport and other post-processing tools to view your profile data."
>> - << endl;
>> + cout << endl << "Profiling done." << endl;
>> }
>> cleanup();
>> return run_result;;
>> diff --git a/pp/common_option.cpp b/pp/common_option.cpp
>> index 99e0e96..6b398e6 100644
>> --- a/pp/common_option.cpp
>> +++ b/pp/common_option.cpp
>> <at> <at> -24,12 +24,17 <at> <at>
>> #include "common_option.h"
>> #include "file_manip.h"
>>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +#include <unistd.h>
>> +#include <limits.h>
>> +
>> using namespace std;
>>
>> namespace options {
>> double threshold = 0.0;
>> string threshold_opt;
>> - string session_dir = OP_SESSION_DIR_DEFAULT;
>> + string session_dir;
>> string command_options;
>> vector<string> image_path;
>> string root_path;
>> <at> <at> -37,6 +42,7 <at> <at> namespace options {
>>
>> namespace {
>>
>> +
>> vector<string> verbose_strings;
>>
>> popt::option common_options_array[] = {
>> <at> <at> -51,6 +57,8 <at> <at> popt::option common_options_array[] = {
>> "path to filesystem to search for missing binaries", "path"),
>> };
>>
>> +int session_dir_supplied;
>> +int trying_default_session_dir;
>>
>> double handle_threshold(string threshold)
>> {
>> <at> <at> -163,13 +171,34 <at> <at> fail:
>> exit(EXIT_FAILURE);
>> }
>>
>> -
>> options::spec get_options(int argc, char const * argv[])
>> {
>> vector<string> non_options;
>> popt::parse_options(argc, argv, non_options);
>>
>> // initialize paths in op_config.h
>> + if (options::session_dir.empty()) {
>> + char * cwd;
>> + struct stat sb;
>> + // First try <curr_dir>/oprofile_data session-dir (i.e., used by operf)
>> + cwd = new char[PATH_MAX];
>> + options::session_dir = getcwd(cwd, PATH_MAX);
>> + delete cwd;
>> + options::session_dir +="/oprofile_data";
>> + if ((stat(options::session_dir.c_str(), &sb) < 0) ||
>> + ((sb.st_mode & S_IFMT) != S_IFDIR)) {
>> + // Try the standard default session dir instead
>> + options::session_dir = "/var/lib/oprofile";
>> + trying_default_session_dir = 1;
>> + } else {
>> + // OK, we'll try <cur_dir>/oprofile_data first; then, if we don't find
>> + // any samples there, we'll also try the default session dir /var/lib/oprofile.
>> + trying_default_session_dir = 0;
>> + }
>> + session_dir_supplied = 0;
>> + } else {
>> + session_dir_supplied = 1;
>> + }
>> init_op_config_dirs(options::session_dir.c_str());
>>
>> if (!options::threshold_opt.empty())
>> <at> <at> -290,3 +319,15 <at> <at> merge_option handle_merge_option(vector<string> const & mergespec,
>>
>> return merge_by;
>> }
>> +
>> +int try_another_session_dir(void)
>> +{
>> + if (!session_dir_supplied && !trying_default_session_dir) {
>> + trying_default_session_dir = 1;
>> + options::session_dir = "/var/lib/oprofile";
>> + init_op_config_dirs(options::session_dir.c_str());
>> + return 1;
>> + } else {
>> + return 0;
>> + }
>> +}
>> diff --git a/pp/common_option.h b/pp/common_option.h
>> index b54ef7c..f054612 100644
>> --- a/pp/common_option.h
>> +++ b/pp/common_option.h
>> <at> <at> -69,4 +69,10 <at> <at> demangle_type handle_demangle_option(std::string const & option);
>> merge_option handle_merge_option(std::vector<std::string> const & mergespec,
>> bool allow_lib, bool exclude_dependent);
>>
>> +/**
>> + * Determine if tool should look at any other potential locations for
>> + * a session dir. Returns '0' for "no" and '1' for "yes".
>> + */
>> +int try_another_session_dir(void);
>> +
>> #endif /* !COMMON_OPTION_H */
>> diff --git a/pp/opannotate_options.cpp b/pp/opannotate_options.cpp
>> index 57a5924..9b6f6db 100644
>> --- a/pp/opannotate_options.cpp
>> +++ b/pp/opannotate_options.cpp
>> <at> <at> -125,7 +125,16 <at> <at> void handle_options(options::spec const & spec)
>> profile_spec::create(spec.common, options::image_path,
>> options::root_path);
>>
>> - list<string> sample_files = pspec.generate_file_list(exclude_dependent, true);
>> + list<string> sample_files;
>> +again:
>> + try {
>> + sample_files = pspec.generate_file_list(exclude_dependent, true);
>> + } catch (op_no_samples_exception & e) {
>> + if (try_another_session_dir())
>> + goto again;
>> + else
>> + throw op_fatal_error(e.what());
>> + }
>>
>> cverb << vsfile << "Archive: " << pspec.get_archive_path() << endl;
>>
>> diff --git a/pp/oparchive_options.cpp b/pp/oparchive_options.cpp
>> index 4b33aec..011fc3d 100644
>> --- a/pp/oparchive_options.cpp
>> +++ b/pp/oparchive_options.cpp
>> <at> <at> -15,7 +15,9 <at> <at>
>> #include <algorithm>
>> #include <iterator>
>> #include <fstream>
>> +#include <string.h>
>>
>> +#include "op_config.h"
>> #include "profile_spec.h"
>> #include "arrange_profiles.h"
>> #include "oparchive_options.h"
>> <at> <at> -23,6 +25,7 <at> <at>
>> #include "string_filter.h"
>> #include "file_manip.h"
>> #include "cverb.h"
>> +#include "op_exception.h"
>>
>>
>> using namespace std;
>> <at> <at> -99,7 +102,15 <at> <at> void handle_options(options::spec const & spec)
>> profile_spec const pspec =
>> profile_spec::create(spec.common, image_path, root_path);
>>
>> - sample_files = pspec.generate_file_list(exclude_dependent, false);
>> +again:
>> + try {
>> + sample_files = pspec.generate_file_list(exclude_dependent, false);
>> + } catch (op_no_samples_exception & e) {
>> + if (try_another_session_dir())
>> + goto again;
>> + else
>> + throw op_fatal_error(e.what());
>> + }
>>
>> cverb << vsfile << "Matched sample files: " << sample_files.size()
>> << endl;
>> <at> <at> -116,4 +127,10 <at> <at> void handle_options(options::spec const & spec)
>> "too strict ?" << endl;
>> exit(EXIT_FAILURE);
>> }
>> +
>> + if (strncmp(op_session_dir, "/var/lib/oprofile", strlen("/var/lib/oprofile")))
>> + cerr << "NOTE: The sample data in this archive is located at " << op_session_dir << endl
>> + << "instead of the standard location of /var/lib/oprofile. Hence, when using opreport" << endl
>> + << "and other post-processing tools on this archive, you must pass the following option:" << endl
>> + << "\t--session-dir=" << op_session_dir << endl;
>> }
>> diff --git a/pp/opreport_options.cpp b/pp/opreport_options.cpp
>> index 95d219c..08d5e8c 100644
>> --- a/pp/opreport_options.cpp
>> +++ b/pp/opreport_options.cpp
>> <at> <at> -25,6 +25,7 <at> <at>
>> #include "xml_output.h"
>> #include "xml_utils.h"
>> #include "cverb.h"
>> +#include "op_exception.h"
>>
>> using namespace std;
>>
>> <at> <at> -253,8 +254,17 <at> <at> void process_spec(profile_classes & classes, list<string> const & spec)
>> profile_spec::create(spec, options::image_path,
>> options::root_path);
>>
>> - list<string> sample_files = pspec.generate_file_list(exclude_dependent,
>> - !options::callgraph);
>> + list<string> sample_files;
>> +again:
>> + try {
>> + sample_files = pspec.generate_file_list(exclude_dependent,
>> + !options::callgraph);
>> + } catch (op_no_samples_exception e) {
>> + if (try_another_session_dir())
>> + goto again;
>> + else
>> + throw op_fatal_error(e.what());
>> + }
>>
>> cverb << vsfile << "Archive: " << pspec.get_archive_path() << endl;
>>
>
>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> oprofile-list mailing list
> oprofile-list <at> lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/oprofile-list
>
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
RSS Feed