[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-bugs-dist
Subject:    [valgrind] [Bug 371916] execution tree xtree concept
From:       Philippe Waroquiers <bugzilla_noreply () kde ! org>
Date:       2016-11-09 20:26:44
Message-ID: bug-371916-17878-VkAegjykoP () http ! bugs ! kde ! org/
[Download RAW message or body]

https://bugs.kde.org/show_bug.cgi?id=371916

--- Comment #16 from Philippe Waroquiers <philippe.waroquiers@skynet.be> ---
(In reply to Josef Weidendorfer from comment #15)
> Hi Philippe, nice patch!
> 
> I think it is good to move functionalty from tools to the core, if it
> can make other tools better or easier to write. Attaching data to call
> trees built from stack traces should fit here.
> However, I did not check the large rewrite of massif.
> 
> General comments (I may be missing something here):
>  
> * is this bound to memory allocation, or is memory allocation just
>   an example use case of the API? It would be nice if tools could
>   attach arbitrary data to xtrees. Perhaps separate the memory allocation
>   stuff into different files (e.g. xtree_memalloc.h).
The API is done so that arbitrary data can be attached.
Of course, to be output as a callgrind or massif file, the data
must be translatable in (positive) integers values.

For what concerns separating the memory allocation stuff in another file:
that is a good suggestion, I will take a look at this. I think this
should be no problem.

> 
> * behavior of some functions in the XTree API depend on command line
>   options. Wouldn't it be better for a tool using this API to be in
>   full control here, ie. the tool parses command line
>   options and pass them as flags?
Basically, that is what the patch does:
command line options are parsed (specifically --xtree-memory=none|allocs|full)
and then the tool calls the memory support functions according to
the command line option (e.g. calling VG_(XTMemory_Full_alloc)).
The tool is in full control, e.g. it might implement 
   --xtree-memory=allocs
completely differently, if needed.

>  About the changes in the malloc
>   wrappers, I think it would be better if the tool can register handlers,
>   and these handlers then call e.g. VG_(XTMemory_Full_alloc).
I guess this could be done, but I think this would be less practical
than the current approach:  the malloc replacement is today
calling a tool replacement function.
So, these replacement functions are doing the job of e.g. recording
the allocated blocks in a tool specific way. In this replacement function,
the tool can (if desired) call e.g. VG_(XTMemory_Full_alloc)).
IMO, having another handler separated from the current handler function
would complexity things, as e.g. the tool would have to associate the
work/data done/prepared in the 'first handler' with the 'following' handler
calls related to xtree. So, in summary, there is now a handler that tells
the tool to e.g. execute a malloc, and this tool function can do the
needed work in one single invocation (e.g. re-using the execontext
done for tool purpose as data input for the xtree).


> 
> About the XTree API, why do you need these add/sub variants?
> Why not just have a function to get a pointer to the value to be updated?
> Any reduction operation may be useful, such as min/max.
Yes, any operation can be done in the add/sub : these functions
get a pointer to the value being updated.
It might be possible to use maybe a more generic name than add/sub
(maybe  Operate(...) ?), but then we will need more arguments for Operate
to indicate what this has to really do, and m_xtree.c itself need to do some
additions to e.g. produce some totals. So, one way or another, there is
an (somewhat flexible) notion of addition which is needed.
The current add/sub_data_fn are matching the current usage (xtree memory).
As a follow-up, I intend to work on a 'syscall xtree' which e.g. might
capture min/max (or maybe an histogram) of values. If needed, we might
revisit the interface of the *_data_fn (e.g. I suspect we might need
a destroy_data_fn if we want a more flexible/variable data set).

What I can do in any case is to add some comments
to indicate that the semantic of 'add' is quite flexible, and that e.g.
a 'max' or 'min' can be computed as part of the an 'add'.

-- 
You are receiving this mail because:
You are watching all bug changes.=
[prev in list] [next in list] [prev in thread] [next in thread]