Xen devel - Oct 2008 - [RFC][PATCH] eliminate extra tb_init_done check

Hi,

As c/s 18441 does the following changes:
-    ASSERT(tb_init_done);
+    if( !tb_init_done )
+        return;
in the function __trace_var in the file xen/common/trace.c.
We don't need to check the variable tb_init_done before we invoke
__trace_var.

This patch simplies such logic:
        if ( tb_init_done )
                __trace_var(...)

to
        __trace_var(...)
.

Two corner conditions are left untouched. One is the assembly in entry.S,
the other is the check of tb_init_done not immediately followed by __trace_var.

Or more aggressively, we can eliminate all the extra checks, make tb_init_done
a static variable, and rename __trace_var to trace_var which looks more like
a right interface name.


--
Guanqun


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 15/10/08 07:20, "Lu, Guanqun" <guanqun.lu@intel.com> wrote:
> Two corner conditions are left untouched. One is the assembly in entry.S,
> the other is the check of tb_init_done not immediately followed by
> __trace_var.
> 
> Or more aggressively, we can eliminate all the extra checks, make
tb_init_done
> a static variable, and rename __trace_var to trace_var which looks more
like
> a right interface name.
The macros check tb_init_done before calling __trace_var() to try and reduce
the cost of the common case (tracing disabled) as far as possible. Hence we
avoid a function call and computation of some arguments to that function.

I don''t know if we''ve actually measured teh performance win
from this. If we
have, George would know about it.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Well, I just tested ddk-build running inside a w2k3 box.  This normally
touches a lot of shadow code, which has a lot of tracing in it.

For each configuration, I ran the build once to warm up the disk cache,
then three more times for actual testing.


No patch: 156s, 161s, 156s
Patch:    161s, 156s, 156s

So I''m not seeing a big impact from this change.

Personally, I think I''d leave it as-is, because I don''t like
the idea of
marshalling all that code and then not doing anything, but I think
that''s just a taste thing at this point. :-)

 -George

Keir Fraser wrote:
> On 15/10/08 07:20, "Lu, Guanqun" <guanqun.lu@intel.com>
wrote:
> 
>> Two corner conditions are left untouched. One is the assembly in
entry.S,
>> the other is the check of tb_init_done not immediately followed by
>> __trace_var.
>>
>> Or more aggressively, we can eliminate all the extra checks, make
tb_init_done
>> a static variable, and rename __trace_var to trace_var which looks more
like
>> a right interface name.
> 
> The macros check tb_init_done before calling __trace_var() to try and
reduce
> the cost of the common case (tracing disabled) as far as possible. Hence we
> avoid a function call and computation of some arguments to that function.
> 
> I don''t know if we''ve actually measured teh performance
win from this. If we
> have, George would know about it.
> 
>  -- Keir
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel