stephane eranian <eran...@googlemail.com> wrote on 06/10/2009 02:31:16 AM:
> Corey, > On Wed, Jun 10, 2009 at 12:42 AM, Corey J Ashford <cjash...@us.ibm.com> wrote: > This looks great, Stephane. > > stephane eranian <eran...@googlemail.com> wrote on 06/09/2009 02:54:16 PM: > > > > > > - the listing and event info API is still returning integer, but > > they are opaque identifiers. It is > > not possible to do for(i=0; i< num_events;i++). Instead, you > > can use a helper macros with > > accessor functions: for_each_event(). The same kind of opaque > > identifiers is used to list and > > query attributes. > This is a great idea. It makes it so the OS-specific layer doesn't have > to create a unified/monotonic event numbering... it just has to be able to > enumerate all of the events and give them unique ids which it is free to > concoct. > > You might also want to provide these functions: > > int pfm_first_event(void); /* returns first opaque event identifier > and implicity starts the sequence over again */ > int pfm_next_event(int opaque_event_id); /* returns -1 when you've > gone past the end of the event list */ > > They are there: > #define for_each_event(x) \ > for((x)=pfm_get_first_event(); (x) != -1; (x) = > pfm_get_next_event((x))) > > #define for_each_event_attr(x, z) \ > for((x)=pfm_get_first_attr((z)); (x) != -1; (x) = > pfm_get_next_attr((z), (x))) > Nice. That's what I get for not looking at the .h file very carefully :) > > - to make things simpler, I have modified all calls which return > > a string, e.g, description of an event or > > attribute, to allocate the string internally. Thus, no need to > > worry about the buffer size but you need > > to free the string whenever you are done. > I have mixed feelings about this because of the possibility of long-term > memory fragmentation. However, I think the vast majority of apps will not > be calling the interface very often, so the problem should be fairly > minimal. > > Yes, that could indeed be a problem but you are right that it is not > anticipated that > libpfm would be used in critical paths, but rather during the setup > phase of a > measurement. The alternatives are: > > 1- the function return value is the string, e.g., const char > *pfm_get_event_desc(int idx) This would be a good solution if it were possible for all arches and chips. That would be possible on all of the Power chips I'm aware of, but I don't know about the others. > > 2- pass a pre-allocated buffer and its size, like we have it > today. But then if the buffer > is too small? The function could return PFMLIB_ERR_TOOSMALL. > The caller would have > to iterate until success. Or we could provide a call to > return the maximum string length > possible. That requires a prior scan for everything that > could return a string which is: > > extern pfm_err_t pfm_get_pmu_name(char **name); > > extern pfm_err_t pfm_get_event_name(int idx, char **name); > extern pfm_err_t pfm_get_event_attr_name(int idx, int > attr_idx, char **name); > > extern pfm_err_t pfm_get_event_desc(int idx, char **str); > extern pfm_err_t pfm_get_event_attr_desc(int idx, int > attr_idx, char **desc); > > extern pfm_err_t pfm_get_cycle_event(char **event); > extern pfm_err_t pfm_get_inst_retired_event(char **event); > > The good thing about all those functions is that the string > length is known in advance, i.e., > the input parameters will not change the maximum size > possible. The max size can be known > in advance for a PMU name, event name, event/attr descriptions. Yep, for Power, I use a Perl script to do the conversion from events and groups files to the libpfm C event and group tables. So the script could easily determine the max string lengths and hard-code them into the header. > > Going back to fragmentation, it is not clear to me that > pushing the allocation in the application > (rather than the library) will make this problem go away. You > will still have to allocate a buffer > based on maximum size, and once you're done you'll have to free it. Except that the user would be free to determine how they were going allocate and dispose of the memory. For example, they could use a static buffer, or allocate the buffer on the stack. > > > > > - pfm_get_event_encoding() to return raw event encoding (not PCL) > So no change here, except the rename from pfm_get_event_code to > pfm_get_event_encoding? > > That is a good point. The 2 functions are different. pfm_get_event_code() does > not take a event description string as input but rather an opaque > identifier. Its > purpose is mostly for event enumeration. Here is an example with pfmon: > > $ pfmon -iretired_instructions > Name : RETIRED_INSTRUCTIONS > Code : 0xc0 > Counters : [ 0 1 2 3 ] > Desc : Retired Instructions > > The function returns 0xc0. You could also use the > pfm_get_event_encoding() function > but then you'd have to form a complete event description with at > least one unit mask. I see, thanks. > The current logic in libpfm is that if an event has unit masks, then > at least one must be > provided. Phil and I discussed the ability to designate a default > unit masks. In fact, > the logic is implemented in the current libpfm but it is not used, > except maybe on MIPS. We are using that feature of libpfm in an internal project right now, actually. > The problem with this is is you have an event A, which supports unit > mask E,F,G, if you > simply pass A, and libpfm picks up A:E (as default), then you don't > really know what you > are measuring with A. Unless we document it somewhere or provide a > call to return the > default such that tool can provide the info. If I recall OProfile > uses default unit masks. That's a good point. In the case where we are using it, there is one obvious choice for the default of each event type, and is effectively, "give me what I want for this event". Right now, the user doesn't know which specific attributes he's getting, but has already stated, "give me what I want, I don't care about the details." You're right, though, it would be good to have a some way to expose that info if it was requested. I have one other question. When I first read this proposal, it seemed like a massive redesign of libpfm. Are you going to maintain backward compatibility and still provide, for example, pfm_dispatch_events? Regards, - Corey Corey Ashford Software Engineer IBM Linux Technology Center, Linux Toolchain cjash...@us.ibm.com ------------------------------------------------------------------------------ Crystal Reports - New Free Runtime and 30 Day Trial Check out the new simplified licensing option that enables unlimited royalty-free distribution of the report engine for externally facing server and web deployment. http://p.sf.net/sfu/businessobjects _______________________________________________ perfmon2-devel mailing list perfmon2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
