Re: [PATCH] dtrace probes for memory manager

Frank Ch. Eigler píše v čt 10. 12. 2009 v 14:11 -0500:
> Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
> 
> > [...]
> > +    header = (StandardChunkHeader *)
> > +        ((char *) ret - STANDARDCHUNKHEADERSIZE);
> > +
> > +//    TRACE_POSTGRESQL_MCXT_ALLOC(context->name, context, size, header->size, true);
> > +
> > [...]
> 
> If the dormant overhead of these probes is measured or suspected to be
> excessive, consider using the dtrace-generated per-probe foo_ENABLED()
> conditional, or a postgres configuration global thusly:

TRACE_POSTGRESQL_MCXT_ALLOC and  TRACE_POSTGRESQL_ASET_ALLOC are
duplicated probes. Have them both make sense but from performance point
of view to have one of them is acceptable.

foo_enable() is good to use when number of argument and their evaluation
cost too much. In this case it does no seem to be much useful. See ASM
code:

AllocSetAlloc+0x17:             xorq   %rax,%rax
AllocSetAlloc+0x1a:             nop    
AllocSetAlloc+0x1b:             nop    
AllocSetAlloc+0x1c:             testl  %eax,%eax
AllocSetAlloc+0x1e:             je     +0xb     <AllocSetAlloc+0x2b>
AllocSetAlloc+0x20:             movq   %r13,%rdi
AllocSetAlloc+0x23:             movq   %r14,%rsi
AllocSetAlloc+0x26:             nop    
AllocSetAlloc+0x27:             nop    
AllocSetAlloc+0x28:             nop    
AllocSetAlloc+0x29:             nop    
AllocSetAlloc+0x2a:             nop    

>    if (__builtin_expect(TRACE_POSTGRESQL_MCXT_ALLOC_ENABLED(), 0))
>       TRACE_POSTGRESQL_MCXT_ALLOC(...);
> 
> so that the whole instrumentation parameter setup/call can be placed
> out of the hot line with gcc -freorder-blocks.

compiler specific construct is not good way. Do not forget that also
other compiler exists.
Zdenek