From: Yeh, J. <jas...@am...> - 2005-12-21 21:30:12
|
CodeAnalyst interfaces with Oprofile in the ways similar to how opreport uses libpp. Raw data is not used by CodeAnalyst. Instead it uses processed data through linking with libpp. Here are two ways that CodeAnalyst uses libpp to access data of current profile. The snippets of the code are at the end of this email for less cluttering: I. Use Oprofile's post processing code to generates summary in the following manner: 1. Create profile_spec. 2. Generate a list of sample files. 3. Arrange sample files into profile classes. 4. Create summary_container from profile classes. 5. CodeAnalyst saves the summary. II. Use Oprofile's post processing code to retrieve the results for each binary file in terms of symbols: 1. Create profile_spec. 2. Generate a list of sample files. 3. Arrange sample files into profile classes. 4. Invert profile classes. 5. For each inverted profile class add symbols into one profile_container. 6. Select all symbols from profile_container 7. Sort the symbols by file name and by reverse sort.=20 8. For each symbol, CodeAnalyst saves the data. If this level of API is believed to be practical and viable, I can go through headers in libpp in the same fashion Maynard Johnson has done for libdb/odb.h. Regards, Jason /****** Code segment for case 1 *****/ profile_spec const spec =3D profile_spec::create (non_options, extra_found_images); list < string > sample_files; try { sample_files =3D spec.generate_file_list (exclude_dependent, EXCLUDE_CG_DATA); m_mod_classes =3D arrange_profiles (sample_files, merge_by); } catch (...) { ... } summary_container summaries (m_mod_classes.v); // Store the count into the app_sample_map vector < app_summary >::const_iterator it =3D summaries.apps.begin = (); for (; it !=3D summaries.apps.end (); it++) { .... } /***** Code segment of case 2 *****/ profile_spec const spec =3D profile_spec::create (non_options, extra_found_images); list < string > sample_files; try { sample_files =3D spec.generate_file_list (exclude_dependent, EXCLUDE_CG_DATA); } catch (...) { ... } addr_classes =3D arrange_profiles (sample_files, merge_by); // Don't need debug info, but need details profile_container samples (false, true); list < inverted_profile > iprofiles =3D invert_profiles (options::archive_path, addr_classes, options::extra_found_images); list < inverted_profile >::iterator it =3D iprofiles.begin (); list < inverted_profile >::iterator const end =3D iprofiles.end (); for (; it !=3D end; ++it) { try { populate_for_image (options::archive_path, samples, *it, options::symbol_filter); } catch(...) { ... } } profile_container::symbol_choice choice; choice.threshold =3D 0; symbol_collection symbols =3D samples.select_symbols (choice); options::sort_by.sort (symbols, options::reverse_sort, options::long_filenames); symbol_collection::const_iterator sit =3D symbols.begin (); symbol_collection::const_iterator send =3D symbols.end (); for (; sit !=3D send; sit++) { .... } |
From: Yeh, J. <jas...@am...> - 2006-01-19 19:31:12
|
Hi, At the end of the message, there is the header of a prototype API library I have created. The prototype is written with the following goal: 1. Minimal changes to any existing code in various libraries of current code. 2. Decoupling the internal steps from API as mentioned by John. There are changes to the Makefile.am files not shown in this email. Specifically, I modified relevant Makefile.am to use libtool to create the archives. The share object of the API library is also build and installed with libtool. =20 I would like to receive any opinion, especially on the following points: 1. Uses of libtool and glib. 2. Ways specified which data should be return. The prototype does not do much other than return all data. 3. Needs to provide API for diffing archives? regards, Jason // Contents of oprofile.h in libopai // options to select sesseions and how data are merged struct op_pp_options { unsigned int pp_flags; char * session; }; /// flags control how data are merged by libpp enum op_pp_flags { op_merge_none =3D 0, op_merge_by_cpu =3D 1 << 0, op_merge_by_lib =3D 1 << 1, op_merge_by_unitmask =3D 1 << 2, op_merge_by_tid =3D 1 << 3, op_merge_by_tgid =3D 1 << 4, }; /// structure of summary passed to user of the API struct op_app_summary { GString * image; GSList * deps; GArray * count; }; /// structure of symbol struct op_symbol_entry { GString * name; GArray * count size_t size; }; unsigned int init(struct op_pp_options * pp_options); -----Original Message----- Most of these steps are essentially internal. We should take a step back from the internal headers and ask what is a sensible, simple API that: o) provides a way to specify what data we want o) provides a way to iterate through that data. In particular, this should be decoupled as much as possible from internal things like inverting the profiles. Needless to say, I would encourage discussion of what exactly such an API looks like. regards john |
From: John L. <le...@mo...> - 2006-01-19 19:39:59
|
On Thu, Jan 19, 2006 at 01:30:39PM -0600, Yeh, Jason wrote: > 1. Uses of libtool This is fine, if it works OK. > glib. I also don't have any major issue with this. > 3. Needs to provide API for diffing archives? Eventually. Also, gprof data and annotations. > // options to select sessions and how data are merged > struct op_pp_options { > unsigned int pp_flags; > char * session; > }; > > > /// flags control how data are merged by libpp > enum op_pp_flags { > op_merge_none = 0, > op_merge_by_cpu = 1 << 0, > op_merge_by_lib = 1 << 1, > op_merge_by_unitmask = 1 << 2, > op_merge_by_tid = 1 << 3, > op_merge_by_tgid = 1 << 4, > }; > > > /// structure of summary passed to user of the API > struct op_app_summary { > GString * image; > GSList * deps; > GArray * count; > }; > > > /// structure of symbol > struct op_symbol_entry { > GString * name; > GArray * count > size_t size; > }; > > unsigned int init(struct op_pp_options * pp_options); Can you expand a little bit on how this works? I'm looking for an API that's somewhat similar to how people use opreport command line. Probably we should reverse the "merge" idea: only *not* merge if the user asks for "per cpu". thanks, john |
From: Yeh, J. <jas...@am...> - 2006-01-19 20:11:28
|
I apologize for not realizing that part of the header was cut off when I copied to Outlook. The functions prototypes are here. =20 /** * Returns app summary stored in returned GSList of struct * op_app_summary */ GSList * get_profile_summary(); /** * Returns all symbols from current session in returned GSList * of op_symbol_entry */ GSList * get_all_symbols(); /// Returns symbols of particular image GSList * get_symbols_for_image(const char * image_name); get_profile_summary is similar to running "opreport" without specifying any image and filter. =20 get_all_symbols is similar to running "opreport -l". get_all_symbols_for_image is similar to running "opreport -l" with a image path. I agree that in general that merging the data is a good idea except for CPU. My intuition is that users on multi cpu system would want to see per-cpu data by default. The options would not have impact on single CPU system. I have not looked opannotate data in depth yet. I will delve into this part once the opreport data API gets into a shape. Jason -----Original Message----- From: opr...@li... [mailto:opr...@li...] On Behalf Of John Levon Sent: Thursday, January 19, 2006 1:40 PM To: Yeh, Jason Cc: opr...@li... Subject: Re: OProfile API discussion -- CodeAnalyst On Thu, Jan 19, 2006 at 01:30:39PM -0600, Yeh, Jason wrote: > 1. Uses of libtool This is fine, if it works OK. > glib. I also don't have any major issue with this. > 3. Needs to provide API for diffing archives? Eventually. Also, gprof data and annotations. > // options to select sessions and how data are merged > struct op_pp_options { > unsigned int pp_flags; > char * session; > }; >=20 >=20 > /// flags control how data are merged by libpp > enum op_pp_flags { > op_merge_none =3D 0, > op_merge_by_cpu =3D 1 << 0, > op_merge_by_lib =3D 1 << 1, > op_merge_by_unitmask =3D 1 << 2, > op_merge_by_tid =3D 1 << 3, > op_merge_by_tgid =3D 1 << 4, > }; >=20 >=20 > /// structure of summary passed to user of the API > struct op_app_summary { > GString * image; > GSList * deps; > GArray * count; > }; >=20 >=20 > /// structure of symbol > struct op_symbol_entry { > GString * name; > GArray * count > size_t size; > }; >=20 > unsigned int init(struct op_pp_options * pp_options); Can you expand a little bit on how this works? I'm looking for an API that's somewhat similar to how people use opreport command line. Probably we should reverse the "merge" idea: only *not* merge if the user asks for "per cpu". thanks, john ------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! http://sel.as-us.falkag.net/sel?cmd=3Dlnk&kid=3D103432&bid=3D230486&dat=3D= 121642 _______________________________________________ oprofile-list mailing list opr...@li... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
From: John L. <le...@mo...> - 2006-01-19 20:38:03
|
On Thu, Jan 19, 2006 at 02:10:18PM -0600, Yeh, Jason wrote: > /** > * Returns app summary stored in returned GSList of struct > * op_app_summary > */ > GSList * get_profile_summary(); > > > /** > * Returns all symbols from current session in returned GSList > * of op_symbol_entry > */ > GSList * get_all_symbols(); > > /// Returns symbols of particular image > GSList * get_symbols_for_image(const char * image_name); They seem a little specific. What if I want *two* apps side-by-side? Shouldn't we be passing in some kind of template saying what we want, with lists of stuff? struct template { list image_names; list lib_image_names; list cpus; list pids; ... } etc? > I agree that in general that merging the data is a good idea except for > CPU. My intuition is that users on multi cpu system would want to see > per-cpu data by default. The options would not have impact on single > CPU system. I don't agree. If I'm running my_single_threaded_app, I rarely care that it got moved about on CPUs, I just want to see the normal profile. regards john |
From: John K. <jk...@ca...> - 2006-01-20 15:36:41
|
On Thu, 2006-01-19 at 20:38 +0000, John Levon wrote: > On Thu, Jan 19, 2006 at 02:10:18PM -0600, Yeh, Jason wrote: SNIP ---------------------------------------------------------- > Shouldn't we be passing in some kind of template saying what we want, > with lists of stuff? > > struct template { > list image_names; > list lib_image_names; > list cpus; > list pids; > ... > } > > etc? > > > I agree that in general that merging the data is a good idea except for > > CPU. My intuition is that users on multi cpu system would want to see > > per-cpu data by default. The options would not have impact on single > > CPU system. > > I don't agree. If I'm running my_single_threaded_app, I rarely care that > it got moved about on CPUs, I just want to see the normal profile. > > regards > john > But if you are running a multi-threaded app say on a NUMA machine, you will care if it gets moved around. Isn't it easier to plan for all the information a user might want - and then filter out what's not wanted? |
From: John L. <le...@mo...> - 2006-01-20 17:08:56
|
On Fri, Jan 20, 2006 at 10:34:37AM -0500, John Kacur wrote: > > I don't agree. If I'm running my_single_threaded_app, I rarely care that > > it got moved about on CPUs, I just want to see the normal profile. > > But if you are running a multi-threaded app say on a NUMA machine, you > will care if it gets moved around. Isn't it easier to plan for all the > information a user might want - and then filter out what's not wanted? No, it's best to provide the simplest interface by default, then allow the rarer cases to get the data if they need it. regards, john |
From: Yeh, J. <jas...@am...> - 2006-01-27 21:29:56
Attachments:
oprofile-api.patch
output.xml
|
My original was send to the admin list instead of this list. I apologies if you receive this email twice.=20 Here is a patch for proof-of-concept API patch (against 0.9.1) and a simple test program that outputs XML to stdout. The API source codes reside in a newly added "libopai" directory. The test program is under "libopapi/tests". =20 An output of the test program is also included. The simple test takes either zero or one command line argument. When run without any argument, the program outputs all information it can gather from libpp to stdout. A saved output is also attached to this mail. If run with a path of a binary image that had generated results in current profiling session, the test program will only output data of that specific binary image. The test app is just to illustrate and test the API written in a short time. Many features are still missing. =20 The patch is by no means ready. The patch still has the following short coming: 1. missing call graph data. 2. missing opannotate data. 3. missing exclude/include symbol selection ability 4. The implementation uses glib2 data structures. However, using it is rather tedious. I will most likely not use it in the later revision. As always, comments are always appreciated. regards,=20 Jason |
From: Ralf W. <Ral...@gm...> - 2006-01-28 08:09:11
|
Hi Jason, * Yeh, Jason wrote on Fri, Jan 27, 2006 at 10:28:45PM CET: > > Here is a patch for proof-of-concept API patch (against 0.9.1) and a > simple test program that outputs XML to stdout. The API source codes > reside in a newly added "libopai" directory. The test program is under > "libopapi/tests". Some comments inline (much abridged) about the build structure. Disclaimer: I know very little about oprofile internals. | diff -Naur oprofile-0.9.1/daemon/liblegacy/Makefile.am oprofile-0.9.1-api/daemon/liblegacy/Makefile.am | --- oprofile-0.9.1/daemon/liblegacy/Makefile.am 2004-02-22 23:57:30.000000000 -0600 | +++ oprofile-0.9.1-api/daemon/liblegacy/Makefile.am 2006-01-27 14:02:15.000000000 -0600 | @@ -1,4 +1,5 @@ | -noinst_LIBRARIES = liblegacy.a | +lib_LTLIBRARIES = liblegacy.la You will install a library named liblegacy. IMVHO that's much too generic a name for an installed library. | + | | # -fno-omit-frame-pointer needed for daemon build: see ChangeLog 2004-02-23 | AM_CFLAGS = @OP_CFLAGS@ -fno-omit-frame-pointer | @@ -9,7 +10,7 @@ | -I ${top_srcdir}/libdb \ | -I ${top_srcdir}/daemon | | -liblegacy_a_SOURCES = \ | +liblegacy_la_SOURCES = \ | opd_24_stats.c \ | opd_24_stats.h \ | opd_kernel.c \ | diff -Naur oprofile-0.9.1/daemon/Makefile.am oprofile-0.9.1-api/daemon/Makefile.am | --- oprofile-0.9.1/daemon/Makefile.am 2005-05-02 10:06:57.000000000 -0500 | +++ oprofile-0.9.1-api/daemon/Makefile.am 2006-01-27 14:01:57.000000000 -0600 | @@ -40,19 +40,19 @@ | | if enable_abi | oprofiled_LDADD = \ | - liblegacy/liblegacy.a \ | - ../libabi/libabi.a \ | - ../libdb/libodb.a \ | - ../libop/libop.a \ | - ../libutil/libutil.a | + liblegacy/liblegacy.la \ | + ../libabi/libabi.la \ | + ../libdb/libodb.la \ | + ../libop/libop.la \ | + ../libutil/libutil.la Why don't you kill all these dependencies and link oprofiled against libopapi instead? (Also see below.) | LINKER=$(CXX) | else | oprofiled_LDADD = \ | - liblegacy/liblegacy.a \ | - ../libdb/libodb.a \ | - ../libop/libop.a \ | - ../libutil/libutil.a | + liblegacy/liblegacy.la \ | + ../libdb/libodb.la \ | + ../libop/libop.la \ | + ../libutil/libutil.la | LINKER=$(CC) | endif | | -oprofiled_LINK = $(LINKER) $(AM_CFLAGS) $(CFLAGS) $(AM_LDFLAGS) $(LDFLAGS) -o $@ | +oprofiled_LINK = libtool --mode=link $(LINKER) $(AM_CFLAGS) $(CFLAGS) $(AM_LDFLAGS) $(LDFLAGS) -o $@ This will possibly use a different `libtool' script from the one `configure' created. Use `$(LIBTOOL)' instead. You should also note that using `--tag=CC' or `--tag=CXX' as well is bound to give more predictable results. | diff -Naur oprofile-0.9.1/libdb/Makefile.am oprofile-0.9.1-api/libdb/Makefile.am | --- oprofile-0.9.1/libdb/Makefile.am 2004-05-29 11:29:41.000000000 -0500 | +++ oprofile-0.9.1-api/libdb/Makefile.am 2006-01-14 11:30:41.000000000 -0600 | @@ -6,9 +6,9 @@ | | AM_CFLAGS = @OP_CFLAGS@ | | -noinst_LIBRARIES = libodb.a | +noinst_LTLIBRARIES = libodb.la | | -libodb_a_SOURCES = \ | +libodb_la_SOURCES = \ | db_manage.c \ | db_insert.c \ | db_travel.c \ | diff -Naur oprofile-0.9.1/libop/Makefile.am oprofile-0.9.1-api/libop/Makefile.am | --- oprofile-0.9.1/libop/Makefile.am 2003-10-01 17:34:00.000000000 -0500 | +++ oprofile-0.9.1-api/libop/Makefile.am 2006-01-14 11:07:50.000000000 -0600 | @@ -3,8 +3,8 @@ | AM_CPPFLAGS=-I${top_srcdir}/libutil | AM_CFLAGS = @OP_CFLAGS@ | | -noinst_LIBRARIES = libop.a | -libop_a_SOURCES = \ | +noinst_LTLIBRARIES = libop.la | +libop_la_SOURCES = \ | op_events.c \ | op_events.h \ | op_parse_event.c \ | diff -Naur oprofile-0.9.1/libopapi/libadd.loT oprofile-0.9.1-api/libopapi/libadd.loT This is a leftover temp file and should not be part of source. | diff -Naur oprofile-0.9.1/libopapi/Makefile.am oprofile-0.9.1-api/libopapi/Makefile.am | --- oprofile-0.9.1/libopapi/Makefile.am 1969-12-31 18:00:00.000000000 -0600 | +++ oprofile-0.9.1-api/libopapi/Makefile.am 2006-01-27 10:16:12.000000000 -0600 | @@ -0,0 +1,40 @@ | +SUBDIRS = . tests | + | +AM_CPPFLAGS = \ | + -I ${top_srcdir}/libpp \ | + -I ${top_srcdir}/libutil++ \ | + -I ${top_srcdir}/libutil \ | + -I ${top_srcdir}/libregex \ | + -I ${top_srcdir}/libdb I don't think it's portable to put a space between `-I' and the path. (Several instances of this.) | + | + | + | +common_libs = \ | + ../libpp/libpp.la \ | + ../libopt++/libopt++.la \ | + ../libregex/libop_regex.la \ | + ../libutil++/libutil++.la \ | + ../libop/libop.la \ | + ../libutil/libutil.la \ | + ../libdb/libodb.la | + | +LIBS=@BFD_LIBS@ | +AM_CXXFLAGS = \ | + @OP_CXXFLAGS@\ | + `pkg-config --cflags glib-2.0` \ | + `xml2-config --cflags --libs` | + | +lib_LTLIBRARIES = libopapi.la | + | +libopapi_la_SOURCES = \ | + summary.cpp \ | + summary.h \ | + oprofile.cpp \ | + oprofile.h | + | +libopapi_la_LIBADD = $(common_libs) I don't know if that is intended, but: this will effectively include all the code from all the $(common_libs) which are noinst_LTLIBRARIES, onto libopapi. noinst_LTLIBRARIES are convenience archives, their sole function is to exist as intermediate steps for being merged into larger archives (for projects using a recursive Makefile structure, for example). [ One should also note that it's not completely portable to link programs against convenience archives; I'm pretty sure though that oprofile systems scope is much narrower than that, so this is nothing to worry about. ] Also, I really wonder why you don't just name this baby liboprofile. It would seem to me the canonical name, given that nothing else uses this name yet. | +libopapi_la_LDFLAGS = `pkg-config --libs glib-2.0` | + | +pkginclude_HEADERS = \ | + oprofile.h | + | diff -Naur oprofile-0.9.1/libopapi/tests/Makefile.am oprofile-0.9.1-api/libopapi/tests/Makefile.am | --- oprofile-0.9.1/libopapi/tests/Makefile.am 1969-12-31 18:00:00.000000000 -0600 | +++ oprofile-0.9.1-api/libopapi/tests/Makefile.am 2006-01-27 10:20:47.000000000 -0600 | @@ -0,0 +1,21 @@ | +AM_CPPFLAGS = \ | + -I/usr/local/include/oprofile | + | +AM_CFLAGS = @OP_CFLAGS@ `pkg-config --cflags glib-2.0` | +AM_CXXFLAGS = @OP_CXXFLAGS@ \ | + `pkg-config --cflags glib-2.0`\ | + `xml2-config --cflags` | + | +LIBS = @BFD_LIBS@ @POPT_LIBS@ `pkg-config --libs glib-2.0` `xml2-config --libs` | + | + | + | +check_PROGRAMS = api_tests | +#bin_PROGRAMS = api_tests | + | +api_tests_SOURCES = \ | + api_xml_test.cpp | + | +api_tests_LDADD = -lopapi | + | +TESTS = ${check_PROGRAMS} | diff -Naur oprofile-0.9.1/libopt++/Makefile.am oprofile-0.9.1-api/libopt++/Makefile.am | --- oprofile-0.9.1/libopt++/Makefile.am 2003-05-21 14:36:17.000000000 -0500 | +++ oprofile-0.9.1-api/libopt++/Makefile.am 2006-01-14 11:01:42.000000000 -0600 | @@ -1,5 +1,5 @@ | AM_CPPFLAGS=-I ${top_srcdir}/libutil++ -I ${top_srcdir}/libutil | AM_CXXFLAGS = @OP_CXXFLAGS@ | | -noinst_LIBRARIES = libopt++.a | -libopt___a_SOURCES = popt_options.cpp popt_options.h | +noinst_LTLIBRARIES = libopt++.la | +libopt___la_SOURCES = popt_options.cpp popt_options.h | diff -Naur oprofile-0.9.1/libpp/Makefile.am oprofile-0.9.1-api/libpp/Makefile.am | --- oprofile-0.9.1/libpp/Makefile.am 2005-04-11 22:14:09.000000000 -0500 | +++ oprofile-0.9.1-api/libpp/Makefile.am 2006-01-27 14:03:05.000000000 -0600 | @@ -9,8 +9,9 @@ | | AM_CXXFLAGS = @OP_CXXFLAGS@ | | -noinst_LIBRARIES = libpp.a | -libpp_a_SOURCES = \ | +noinst_LTLIBRARIES = libpp.la | + | +libpp_la_SOURCES = \ | arrange_profiles.cpp \ | arrange_profiles.h \ | callgraph_container.h \ | @@ -50,3 +51,13 @@ | symbol_functors.h \ | symbol_sort.cpp \ | symbol_sort.h | + | +pkginclude_HEADERS = \ | + arrange_profiles.h \ | + symbol.h \ | + profile.h \ | + profile_spec.h \ | + locate_images.h \ | + populate.h \ | + profile_container.h \ | + symbol_sort.h | diff -Naur oprofile-0.9.1/libregex/Makefile.am oprofile-0.9.1-api/libregex/Makefile.am | --- oprofile-0.9.1/libregex/Makefile.am 2003-09-25 21:16:23.000000000 -0500 | +++ oprofile-0.9.1-api/libregex/Makefile.am 2006-01-15 07:49:31.000000000 -0600 | @@ -3,13 +3,16 @@ | AM_CPPFLAGS = -I ${top_srcdir}/libutil++ | AM_CXXFLAGS = @OP_CXXFLAGS@ | | -noinst_LIBRARIES = libop_regex.a | +noinst_LTLIBRARIES = libop_regex.la | | -libop_regex_a_SOURCES = \ | +libop_regex_la_SOURCES = \ | op_regex.cpp \ | op_regex.h \ | demangle_symbol.h \ | demangle_symbol.cpp | | +pkginclude_HEADERS = demangle_symbol.h | + | datadir = $(prefix)/share/oprofile | nodist_data_DATA = stl.pat | + | diff -Naur oprofile-0.9.1/libtool oprofile-0.9.1-api/libtool This file is generated by `configure' and should not be part of the source. | diff -Naur oprofile-0.9.1/libutil/Makefile.am oprofile-0.9.1-api/libutil/Makefile.am | --- oprofile-0.9.1/libutil/Makefile.am 2003-09-26 08:23:32.000000000 -0500 | +++ oprofile-0.9.1-api/libutil/Makefile.am 2006-01-15 05:53:54.000000000 -0600 | @@ -2,8 +2,8 @@ | | AM_CFLAGS = @OP_CFLAGS@ | | -noinst_LIBRARIES = libutil.a | -libutil_a_SOURCES = \ | +noinst_LTLIBRARIES = libutil.la | +libutil_la_SOURCES = \ | op_deviceio.c \ | op_lockfile.c \ | op_file.c \ | @@ -26,3 +26,5 @@ | op_cpufreq.h \ | op_version.c \ | op_version.h | + | +pkginclude_HEADERS = op_types.h | diff -Naur oprofile-0.9.1/libutil++/Makefile.am oprofile-0.9.1-api/libutil++/Makefile.am | --- oprofile-0.9.1/libutil++/Makefile.am 2005-04-22 14:29:21.000000000 -0500 | +++ oprofile-0.9.1-api/libutil++/Makefile.am 2006-01-15 08:09:02.000000000 -0600 | @@ -5,8 +5,8 @@ | -I ${top_srcdir}/libop | AM_CXXFLAGS = @OP_CXXFLAGS@ | | -noinst_LIBRARIES = libutil++.a | -libutil___a_SOURCES = \ | +noinst_LTLIBRARIES = libutil++.la | +libutil___la_SOURCES = \ | op_bfd.cpp \ | op_bfd.h \ | bfd_support.cpp \ | @@ -35,3 +35,9 @@ | utility.h \ | cached_value.h \ | comma_list.h | + | +pkginclude_HEADERS = \ | + growable_vector.h \ | + string_filter.h \ | + string_manip.h \ | + utility.h | diff -Naur oprofile-0.9.1/ltmain.sh oprofile-0.9.1-api/ltmain.sh This file is installed as well (by libtoolize). If the oprofile project does not store autotool-installed files, it should not be part of CVS. (It will be part of the distribution tarball though). | diff -Naur oprofile-0.9.1/Makefile.am oprofile-0.9.1-api/Makefile.am | --- oprofile-0.9.1/Makefile.am 2005-03-25 19:10:24.000000000 -0600 | +++ oprofile-0.9.1-api/Makefile.am 2006-01-27 13:03:54.000000000 -0600 | @@ -14,8 +14,17 @@ | pp \ | events \ | doc \ | + libopapi \ | gui | | + | + | + | +libtool: @LIBTOOL_DEPS@ | + cd $(srcdir) && \ | + $(SHELL) ./config.status --recheck I don't think you want the recheck to happen in $(srcdir): config.status lives in $(top_builddir) which is `.' for this Makefile. | + | + | ACLOCAL_AMFLAGS = -I m4 | | # The module will not build under distcheck | diff -Naur oprofile-0.9.1/pp/Makefile.am oprofile-0.9.1-api/pp/Makefile.am | --- oprofile-0.9.1/pp/Makefile.am 2005-04-04 17:34:41.000000000 -0500 | +++ oprofile-0.9.1-api/pp/Makefile.am 2006-01-14 11:48:42.000000000 -0600 | @@ -16,13 +16,13 @@ | pp_common = common_option.cpp common_option.h | | common_libs = \ | - ../libpp/libpp.a \ | - ../libopt++/libopt++.a \ | - ../libregex/libop_regex.a \ | - ../libutil++/libutil++.a \ | - ../libop/libop.a \ | - ../libutil/libutil.a \ | - ../libdb/libodb.a | + ../libpp/libpp.la \ | + ../libopt++/libopt++.la \ | + ../libregex/libop_regex.la \ | + ../libutil++/libutil++.la \ | + ../libop/libop.la \ | + ../libutil/libutil.la \ | + ../libdb/libodb.la | | opreport_SOURCES = opreport.cpp \ | opreport_options.h opreport_options.cpp \ | diff -Naur oprofile-0.9.1/utils/Makefile.am oprofile-0.9.1-api/utils/Makefile.am | --- oprofile-0.9.1/utils/Makefile.am 2005-03-31 11:20:41.000000000 -0600 | +++ oprofile-0.9.1-api/utils/Makefile.am 2006-01-27 14:02:43.000000000 -0600 | @@ -7,4 +7,5 @@ | dist_bin_SCRIPTS = opcontrol | | ophelp_SOURCES = ophelp.c | -ophelp_LDADD = ../libop/libop.a ../libutil/libutil.a | +ophelp_LDADD = ../libop/libop.la ../libutil/libutil.la | +ophelp_LINK = libtool --mode=link $(CC) $(AM_LDFLAGS) $(LDFLAGS) -o $@ Same as above. |
From: John L. <le...@mo...> - 2006-01-30 00:54:05
|
On Sat, Jan 28, 2006 at 09:08:58AM +0100, Ralf Wildenhues wrote: > Some comments inline (much abridged) about the build structure. Thanks for these comments, Ralf. I'm not much of a libtool user so I really appreciate your input. regards john |
From: John L. <le...@mo...> - 2006-01-30 01:09:37
|
On Fri, Jan 27, 2006 at 03:28:45PM -0600, Yeh, Jason wrote: > Here is a patch for proof-of-concept API patch (against 0.9.1) and a Thanks for starting this work. Some initial comments. +/** + * options to specify how samples should be separated. + * By default no separation is done. + */ +struct separate_option { + unsigned char cpu; + unsigned char lib; + unsigned char tid; + unsigned char tgid; + unsigned char unitmask; +}; I'd much prefer flags, they are easier to extend. +/// options to select sesseions and how data are merged +struct op_pp_options { + GSList * image_names; // GSList of GString + GSList * image_paths; // GSList of GString + GSList * pids; Incomplete. What about tgids vs. tids. + char * session; It would be really nice if instead of code a specification document could be written. For example, does this have to be a full path, or does it work like opreport? This document could eventually form the API documentation. Note: we need a utility function to enumerate the available sessions. How do I find out the events available, and how do I restrict to just those I'm interested in? + // Only applicable for call-graph + unsigned char details; This comment isn't true. + // Only applicable for symbols + unsigned char debug_info; + unsigned char address; + double threshold; All of these should be flags too. We need to version any structs we pass in so we can extend them and keep backwards-compatibility. +/// structure of summary passed to user of the API +struct op_app_summary { + GString * image; + GSList * deps; + GArray * counts; +}; + +struct op_profile_classes { + GString * cpuinfo; + GSList * profile_templates; +}; + +enum { + TEMPLATE_COUNT = 0, + TEMPLATE_UNITMASK, + TEMPLATE_CPU, + TEMPLATE_TGID, + TEMPLATE_TID +}; + +#define TEMPLATE_INFO_SIZE TEMPLATE_TID+1 +/** + * Information of the current query. Each entry in + * the list corresponds to each entry of the counts array + * of op_app_summary + */ +struct op_profile_template { + GString * event; + guint info[TEMPLATE_INFO_SIZE]; +}; + + +struct op_symbol_sample { + GString * filename; + GArray * counts; + guint vma; + guint linenr; +}; + +struct op_symbol { + GString * image_name; + GString * app_name; + GString * name; + struct op_symbol_sample * samples; + guint size; +}; I'm unhappy about passing structures back. We should be passing *iterators* back (an opaque handle), and letting a user iterate through via callbacks. +bool create_profile_summary(); + +/** + * Returns app summary stored in returned GSList of struct + * op_app_summary. + */ +GSList * get_profile_summary(); Why separate steps? What user cares? +void op_create_symbols(); + +/** + * Returns all symbols from current session in returned GSList + * of op_symbol_entry + */ +GSList * op_get_symbols(); Ditto. +/// Returns symbols of particular image +GSList * get_symbol_for_image(const char * image_name); Seems specific? +/** + * Delete memroy allocated. + */ +void op_cleanup(); We should return an opaque handle and take this handle for cleaning up internally. How do I specify a path to search for extra images? For every opreport etc. option we need to investigate whether we need it in the API, and how it might look. regards john |
From: Yeh, J. <jas...@am...> - 2006-01-30 16:20:59
|
As John suggested, I listed every options of opreport, opannotate here to start the discussion on whether or not it is necessary in the API. We can discuss how it might work in the following emails once we decide on the necessity of the option to be provided by the API: Opreport: --verbose: Necessary, for debugging purposes. --image-path: Necessary. --callgraph: Necessary. --details: Necessary. --symbols: Necessary --output-file. Not applicable. --sort: Not necessary; --merge: By default everything is merged, separations is done upon request. Not necessary, but an option to separate is necessary. --exclude-dependent: Necessary. --exclude-symbols: Necessary. --include-symbols: Necessary. --threshold: Necessary. --demangle: Necessary. --debug-info: Necessary. --no-header: Not necessary. --show-address: Necessary. --long-filenames: Not necessary. I think always returning the long name is a safe bet. --accumulated: Debatable. I don't think it's necessary for the API to return the stats. =20 --global-percent: Not necessary. Non-options options: Opannotate (repeated options are omitted): --image-path: Necessary. --output-dir: Not necessary. --search-dirs: Necessary --base-dirs: Necessary. --include-file: Necessary. --exclude-file: Necessary. --objdump-params: Necessary. --source: Necessary. --assembly: Necessary. Jason -----Original Message----- From: John Levon [mailto:le...@mo...]=20 Sent: Sunday, January 29, 2006 7:10 PM To: Yeh, Jason Cc: opr...@li... Subject: Re: OProfile API discussion -- CodeAnalyst On Fri, Jan 27, 2006 at 03:28:45PM -0600, Yeh, Jason wrote: > Here is a patch for proof-of-concept API patch (against 0.9.1) and a Thanks for starting this work. Some initial comments. +/** + * options to specify how samples should be separated. + * By default no separation is done. + */ +struct separate_option { + unsigned char cpu; + unsigned char lib; + unsigned char tid; + unsigned char tgid; + unsigned char unitmask; +}; I'd much prefer flags, they are easier to extend. +/// options to select sesseions and how data are merged +struct op_pp_options { + GSList * image_names; // GSList of GString + GSList * image_paths; // GSList of GString + GSList * pids; Incomplete. What about tgids vs. tids. + char * session; It would be really nice if instead of code a specification document could be written. For example, does this have to be a full path, or does it work like opreport? This document could eventually form the API documentation. Note: we need a utility function to enumerate the available sessions. How do I find out the events available, and how do I restrict to just those I'm interested in? + // Only applicable for call-graph + unsigned char details; This comment isn't true. + // Only applicable for symbols + unsigned char debug_info; + unsigned char address; + double threshold; All of these should be flags too. We need to version any structs we pass in so we can extend them and keep backwards-compatibility. +/// structure of summary passed to user of the API +struct op_app_summary { + GString * image; + GSList * deps; + GArray * counts; +}; + +struct op_profile_classes { + GString * cpuinfo; + GSList * profile_templates; +}; + +enum { + TEMPLATE_COUNT =3D 0, + TEMPLATE_UNITMASK, + TEMPLATE_CPU, + TEMPLATE_TGID, + TEMPLATE_TID +}; + +#define TEMPLATE_INFO_SIZE TEMPLATE_TID+1 +/** + * Information of the current query. Each entry in + * the list corresponds to each entry of the counts array + * of op_app_summary + */ +struct op_profile_template { + GString * event; + guint info[TEMPLATE_INFO_SIZE]; +}; + + +struct op_symbol_sample { + GString * filename; + GArray * counts; + guint vma; + guint linenr; +}; + +struct op_symbol { + GString * image_name; + GString * app_name; + GString * name; + struct op_symbol_sample * samples; + guint size; +}; I'm unhappy about passing structures back. We should be passing *iterators* back (an opaque handle), and letting a user iterate through via callbacks. +bool create_profile_summary(); + +/** + * Returns app summary stored in returned GSList of struct + * op_app_summary. + */ +GSList * get_profile_summary(); Why separate steps? What user cares? +void op_create_symbols(); + +/** + * Returns all symbols from current session in returned GSList + * of op_symbol_entry + */ +GSList * op_get_symbols(); Ditto. +/// Returns symbols of particular image +GSList * get_symbol_for_image(const char * image_name); Seems specific? +/** + * Delete memroy allocated. =20 + */ +void op_cleanup(); We should return an opaque handle and take this handle for cleaning up internally. How do I specify a path to search for extra images? For every opreport etc. option we need to investigate whether we need it in the API, and how it might look. regards john |
From: Maynard J. <may...@us...> - 2006-01-31 22:10:18
|
Yeh, Jason wrote: >As John suggested, I listed every options of opreport, opannotate here >to start the discussion on whether or not it is necessary in the API. > > Yes, I think a high-level discussion like this is a great way to get consensus on what's needed in the API. >We can discuss how it might work in the following emails once we decide >on the necessity of the option to be provided by the API: > > One thing missing from the list of options below is 'profile specification'. I suppose that the API should support the same profile specification parameters as the existing tools. Further comments below . . . >Opreport: >--verbose: Necessary, for debugging purposes. > >--image-path: Necessary. > >--callgraph: Necessary. > > Callgraph is not yet supported on all platforms. >--details: Necessary. > >--symbols: Necessary > >--output-file. Not applicable. > >--sort: Not necessary; > > and --reverse-sort , also not necessary. >--merge: By default everything is merged, separations is done upon >request. Not necessary, but an option to separate is necessary. > >--exclude-dependent: Necessary. > >--exclude-symbols: Necessary. > >--include-symbols: Necessary. > >--threshold: Necessary. > > I don't think this is necessary. This kind of filtering of data should be left to the tools that use the API. >--demangle: Necessary. > >--debug-info: Necessary. > > Yes, this is necessary, but this is implied with opannotate; so it's not necessary to list here. >--no-header: Not necessary. > >--show-address: Necessary. > >--long-filenames: Not necessary. I think always returning the long name >is a safe bet. > >--accumulated: Debatable. I don't think it's necessary for the API to >return the stats. > I agree -- not necessary. > > >--global-percent: Not necessary. > >Non-options options: > >Opannotate (repeated options are omitted): >--image-path: Necessary. > >--output-dir: Not necessary. > >--search-dirs: Necessary > >--base-dirs: Necessary. > >--include-file: Necessary. > >--exclude-file: Necessary. > >--objdump-params: Necessary. > >--source: Necessary. > >--assembly: Necessary. > >Jason > > > [snip] Regards, -Maynard |
From: John L. <le...@mo...> - 2006-01-31 22:17:58
|
On Tue, Jan 31, 2006 at 04:10:15PM -0600, Maynard Johnson wrote: > >--merge: By default everything is merged, separations is done upon > >request. Not necessary, but an option to separate is necessary. > > > >--exclude-dependent: Necessary. > > > >--exclude-symbols: Necessary. > > > >--include-symbols: Necessary. > > > >--threshold: Necessary. > > > > > I don't think this is necessary. This kind of filtering of data should > be left to the tools that use the API. Iagree, with the possible exception of --exclude-dependent - it all depends on how we're presenting the data. > >--demangle: Necessary. > > > >--debug-info: Necessary. > > > > > Yes, this is necessary, but this is implied with opannotate; so it's not > necessary to list here. --debug-info is an opreport option - it says whether we should provide file:line information for symbols (and also details). We should consider whether its our business to provide demangling support or not. I'm inclined towards not having it as an option to generating the data, but possible providing an after-the-fact utility function. > >--long-filenames: Not necessary. I think always returning the long name > >is a safe bet. Agreed. > >--accumulated: Debatable. I don't think it's necessary for the API to > >return the stats. > > > I agree -- not necessary. Yes. > >--output-dir: Not necessary. > > > >--search-dirs: Necessary > > > >--base-dirs: Necessary. > > > >--include-file: Necessary. > > > >--exclude-file: Necessary. > > > >--objdump-params: Necessary. > > > >--source: Necessary. > > > >--assembly: Necessary. I'm not sure all of these make sense. Think about what a tool will likely need: it will probably have a source file, and need the data for each line. regards john |
From: Maynard J. <may...@us...> - 2006-01-31 23:22:03
|
John Levon wrote: >On Tue, Jan 31, 2006 at 04:10:15PM -0600, Maynard Johnson wrote: > > > >>>--output-dir: Not necessary. >>> >>>--search-dirs: Necessary >>> >>>--base-dirs: Necessary. >>> >>>--include-file: Necessary. >>> >>>--exclude-file: Necessary. >>> >>>--objdump-params: Necessary. >>> >>>--source: Necessary. >>> >>>--assembly: Necessary. >>> >>> > >I'm not sure all of these make sense. Think about what a tool will >likely need: it will probably have a source file, and need the data for >each line. > > Right . . . A typical scenario would be a GUI profiler tool that allows the user to select a particular sample for which they would like to see the source (if compiled with debug info). The returned data should include all the samples taken for the selected source file so the user can scroll through the file and see everything immediately. So the returned data would simply be a list of line numbers and their associated event(s) and count(s). The same sort of thing could be done if the user wanted to see samples at the assembly level -- i.e., just provide a list of VMAs (for a specified binary), along with event(s) and counts, and let the tool do the objdump and plug in the returned sample info when its displayed. (Substitute 'ticks' for 'event(s)' if appropriate.) -Maynard >regards >john > > >------------------------------------------------------- >This SF.net email is sponsored by: Splunk Inc. Do you grep through log files >for problems? Stop! Download the new AJAX search engine that makes >searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! >http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642 >_______________________________________________ >oprofile-list mailing list >opr...@li... >https://lists.sourceforge.net/lists/listinfo/oprofile-list > > > |
From: Yeh, J. <jas...@am...> - 2006-01-31 22:53:42
|
On Tue, Jan 31, 2006 at 04:19:15PM -0600, John Levon wrote: <snip> After looking at these options again, I have second thoughts on some. > >--output-dir: Not necessary. > > > >--search-dirs: Necessary This option would be useful for binaries build in a different machine, in a temp directory, or when source files are moved around, etc. > >--base-dirs: Necessary. Not necessary. > >--include-file: Necessary. > >--exclude-file: Necessary. Not necessary. Similar to other filtering options, we can leave the filter to the app using the API. > >--objdump-params: Necessary. Not necessary. I think default options should be fine for most of the cases. =20 > >--source: Necessary. > > > >--assembly: Necessary. > >I'm not sure all of these make sense. Think about what a tool will >likely need: it will probably have a source file, and need the data for >each line. That's exactly the kind of tool I have in mind. Something that will display source file/disassembly with stats attached to each line, and possibly graphic display of samples. I will create a new comprehensive list once no new "votes" are casted. regards, Jason |
From: John L. <le...@mo...> - 2006-01-31 23:06:51
|
On Tue, Jan 31, 2006 at 04:53:25PM -0600, Yeh, Jason wrote: > > >--objdump-params: Necessary. > Not necessary. I think default options should be fine for most of the > cases. I forget why we have this option in fact, but in the wider case of an API, the whole objdump thing is a disaster. We don't have any great options at the moement though. But we probably do need an ability to specify objdump stuff. regards john |
From: Maynard J. <may...@us...> - 2006-01-31 23:33:06
|
Yeh, Jason wrote: >On Tue, Jan 31, 2006 at 04:19:15PM -0600, John Levon wrote: ><snip> > >After looking at these options again, I have second thoughts on some. > > > >>>--output-dir: Not necessary. >>> >>>--search-dirs: Necessary >>> >>> >This option would be useful for binaries build in a different machine, >in a temp directory, or when source files are moved around, etc. > > These different locations will be specified by the user in some manner to the tool. The tool can make multiple calls to the API if there are multiple locations to check for a binary or source file. I think it would be an acceptable simplification of the API to leave this out, and does not put too much burden on the tool developer. -Maynard > > >Jason > > > >------------------------------------------------------- >This SF.net email is sponsored by: Splunk Inc. Do you grep through log files >for problems? Stop! Download the new AJAX search engine that makes >searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! >http://sel.as-us.falkag.net/sel?cmd=k&kid3432&bid#0486&dat1642 >_______________________________________________ >oprofile-list mailing list >opr...@li... >https://lists.sourceforge.net/lists/listinfo/oprofile-list > > > |
From: Yeh, J. <jas...@am...> - 2006-02-01 00:14:55
|
Right. The line number, source file name, and vma are suffice for the GUI. It does not require the post process library to do any search. =20 In this case, the data provided by opreport covers most of the needs of the API. --debug-info will get the source line information and --show-address will get the vma. Jason -----Original Message----- From: opr...@li... [mailto:opr...@li...] On Behalf Of Maynard Johnson > >>>--output-dir: Not necessary. >>> >>>--search-dirs: Necessary >>> =20 >>> >This option would be useful for binaries build in a different machine, >in a temp directory, or when source files are moved around, etc. > =20 > These different locations will be specified by the user in some manner=20 to the tool. The tool can make multiple calls to the API if there are=20 multiple locations to check for a binary or source file. I think it=20 would be an acceptable simplification of the API to leave this out, and=20 does not put too much burden on the tool developer. -Maynard |
From: Maynard J. <may...@us...> - 2006-02-01 16:37:53
|
Yeh, Jason wrote: > Right. The line number, source file name, and vma are suffice for the > GUI. It does not require the post process library to do any search. > > In this case, the data provided by opreport covers most of the needs of > the API. --debug-info will get the source line information and > --show-address will get the vma. I agree. -Maynard > > Jason |
From: Yeh, J. <jas...@am...> - 2006-02-01 16:49:31
|
Here is an updated list of the needed options and the consensus so far: --image-path: Necessary. --callgraph: Necessary. --details: Necessary. --symbols: Necessary --exclude-dependent: Necessary. --demangle: Not as an options, but a possible utility function. --debug-info: Necessary. and the profile specifications. Jason |
From: Yeh, J. <jas...@am...> - 2006-02-01 20:08:10
|
John,=20 Some questions arise after re-reading your comment on the prototype of the API library. This email is referring to John's email with the link below: http://marc.theaimsgroup.com/?l=3Doprofile-list&m=3D113858341228548&w=3D2= John's original comments are in quotes. 1. "We need a utility function to enumerate the available sessions". opreport uses user specified sessions if specified, and the current session if none is specified. Do you suggest that the API look for the session in addition to the current capability of opreport? 2. "How do I find out the events available, how do I restrict to just those I'm intereste in?" The API can generate a dummy spec file to sweep all the sample files when users of API ask for available events, and return iterators t o the event. Should we provide similar function for cpu, tgid, tid, image or should we keep it simple and limit this function to just event? On a separate but related issue, what would be a better way to set profile specifications? Pass one structure with version number and all specification, or have many set_profile_specification() functions. regards, Jason |
From: John L. <le...@mo...> - 2006-02-01 20:12:06
|
On Wed, Feb 01, 2006 at 02:07:44PM -0600, Yeh, Jason wrote: > 1. "We need a utility function to enumerate the available sessions". > opreport uses user specified sessions if specified, and the current > session if none is specified. Do you suggest that the API look for the > session in addition to the current capability of opreport? Yes. Think about a GUI - it would want to have a combo box that lists the available sessions, for example. > 2. "How do I find out the events available, how do I restrict to just > those I'm intereste in?" > The API can generate a dummy spec file to sweep all the sample files > when users of API ask for available events, and return iterators t o the > event. Should we provide similar function for cpu, tgid, tid, image or > should we keep it simple and limit this function to just event? This is a difficult question. We need to think some more about this. > On a separate but related issue, what would be a better way to set > profile specifications? Pass one structure with version number and all > specification, or have many set_profile_specification() functions. I think one structure makes sense for this. regards john |
From: Yeh, J. <jas...@am...> - 2006-02-01 22:37:05
Attachments:
op_api.h
|
I attached an updated API header that reflects the discussions made in previous few days. A non-code documentation is still being work on. regards, Jason |
From: Yeh, J. <jas...@am...> - 2006-02-13 21:00:24
Attachments:
oprofile-api-2.patch
|
This mail includes the first revision of the previous api patch. I try to incorporate as much suggestions from John and Ralf as possible. However, there are still few things not completed: 1. What's the preferable way to handle errors within Oprofile library? Would throwing exceptions be preferred than returning error code/failure or vice versa?=20 2. A more sophisticated way to generate the derived object (the actual implementation) from the abstract library interface. Since there is only one derived class now, I don't think this is a big deal. 3. Documentation. I am still in the middle of completing it. 4. Iterators to retrieves individual samples. To access the samples, one must go through symbol structure. The next revision will provide the iterator to access samples directly. Please take a look at the patch. As always, suggestions are welcome. regards, Jason |