[vsipl++] [patch] Built-in function profiling

[Jules Bergmann]

Don McCoy wrote:

>>> @@ -171,6 +179,7 @@
>>>  Profiler::dump(char* filename, char /*mode*/)
>>>  {
>>>    std::ofstream    file;
>>> +  const char delim[] = " : ";
>>
>> While the spaces improve the human readability, they are more 
>> difficult to post-process in a languages like perl and python 
>> (although the extra difficulty is pretty minor).  Let's leave the 
>> space in, but in general we should "error" on the side of easier 
>> post-processing rather than readability since the profiling output 
>> will have a lot of raw information that will be difficult to digest 
>> without some reduction.
>>
> I'll leave it as is, but you can see I was thinking the same thing by 
> making it easy to change in the future.  It might even be nice to allow 
> it to be overridden by the user.  This will help people who would find 
> it easier or preferable to use X over whatever default we settle on.

Sounds good!


>>> +      // clear log
>>>        cur->second.total = TP::zero();
>>>        cur->second.count = 0;
>>
>> [1] I think this should also clear 'value' too.
>>
> I ran into some strange behavior (incorrect results, missing ops value) 
> when I did this initially.

That is strange.

>  Looking at the method used for 'data_' (not 
> shown, but it is a vector<> rather than a map<>) so I changed it to:
> 
>     accum_.clear();  (after the for loop)
> 
> Is there a reason to avoid this?  It seems to be the right thing, but 
> there may have been a reason not to that is not obvious at first glance.

The reason I avoided using clear is because it deletes both the keys and 
the values.  If you are using measuring the same thing over and over in 
accumulate mode and every so often want to dump out your results, 
clear() will introduce a extra overhead because the map's B-tree (or 
whatever structure it uses internally) has to be recreated after the clear.

An example of this is using accumulate mode to profile a benchmark.  It 
makes sense to zero out the accumulated data when switching from one 
problem size to the next.  However, since the next problem size will be 
doing the same events, keeping the keys around will avoids recreating 
the map's internal search tree.

However, all that said, I think we should also be more explicit in 
naming the events (adding the data type, size, etc).  That takes away my 
original reason for avoiding clear().  In fact, it makes using clear() a 
good idea since it will remove crufty old entries.

Bottom line: let's use clear().

				-- Jules


-- 
Jules Bergmann
CodeSourcery
jules at codesourcery.com
(650) 331-3385 x705