[Mono-dev] [PATCH] MONO_IOMAP reporting option - string allocation locations

[Marek Habersack]

On 11/27/2009 06:48 PM, Rodrigo Kumpera wrote:
>
>
> On Fri, Nov 27, 2009 at 1:40 PM, Marek Habersack
> <grendel at twistedcode.net <mailto:grendel at twistedcode.net>> wrote:
>
>     On 11/27/2009 02:29 PM, Rodrigo Kumpera wrote:
>
>         I agree with Zoltan, we better figure out how to extend the
>         profilling
>         interface to support such tool than
>
>     it would defy the purpose of the tool, but it seems I can remove the
>     code from mono_string_new_utf16 without harming functionality -
>     would that be ok?
>
>
> Why using the profilling interface defy the purpose of the tool? I can't
> see how passing an extra argument to the mono runtime be a problem.
The utility should be as seamless to use. With it consisting of two 
parts - one in IO portability code and another as a profiler module 
('profiler' being a misnomer here too, as the tool is by no means a 
profiler) it would require setting both MONO_IOMAP and passing 
--profile=iomap (or somesuch) to the runtime as they both are required 
for the code to be useful. This can be a problem for MonoVS or 
MonoDevelop, which would have to call the runtime with --profile for 
every app whether or not it is necessary, and also it introduces a 
complexity for users not familiar with Mono command line. To get rid of 
the disparity, one would have to switch IOMAP on behind the scenes if 
--profile=iomap was used and this is not a nice thing to do, imho. From 
the other end, the user would have to know that they need to specify 
'all:report' and pass --profile=iomap to the runtime - also not a nice 
solution.

> My main issue is about bloat, if we can make it work as an external tool
There's not much bloat really. The only thing that can be considered as 
such is the addition to the mini_redirect_call function which checks if 
IOMAP reporting is active. I agree it's not a nice thing to do at this 
point, but it's not very costly in terms of execution time and code size 
(especially if related to what mini_redirect_call does). I've just 
removed the code from mono_string_new_utf16 without any harm to 
functionality, so the only part that remains in the string allocation 
functions is the one in InternalAllocateStr icall - which is NOT used by 
default anyway (mini_redirect_call overrides it with emitted code for 
managed string allocator) and so the code won't affect usual 
performance. I really think the impact on the runtime is kept to minimum.

> just fine, I don't see a reason to add it to the runtime.
> As I mentioned, we can extend the profiling API to fit your needs.
The profiling API has all that's needed - but if the code was there, it 
would have to hook up to the object allocation function and examine 
every single object whether it's a string and then store the string - it 
would cost more time it does with the current patch.

Find attached the new version of the patch.

marek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: iomap-report.diff
Type: text/x-patch
Size: 26045 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20091127/03bf708f/attachment-0001.bin