| Message ID | 1396287436-12696-3-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
On Mon, Mar 31, 2014 at 7:37 PM, Andreas Färber <afaerber@suse.de> wrote: > CPU address spaces touching load and store helpers as well as the > movement of (almost) all fields from CPU_COMMON to CPUState have led to > a noticeable increase of CPU() usage in "hot" paths for both TCG and KVM. > > While CPU()'s OBJECT_CHECK() might help detect development errors, i.e. > in form of crashes due to QOM vs. non-QOM mismatches rather than QOM > type mismatches, it is not really needed at runtime since mostly used in > CPU-specific paths, coming from a target-specific CPU subtype. If that > pointer is damaged, other errors are highly likely to occur elsewhere > anyway. > > Keep the CPU() macro for a consistent developer experience and for > flexibility to exchange its implementation, but turn it into a pure, > unchecked C cast for now. > > Compare commit 6e42be7cd10260fd3a006d94f6c870692bf7a2c0. > > Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Andreas Färber <afaerber@suse.de> It works fine here. Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com> Thanks, Laurent > --- > include/qom/cpu.h | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index f99885a..df977c8 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -53,7 +53,12 @@ typedef uint64_t vaddr; > > #define TYPE_CPU "cpu" > > -#define CPU(obj) OBJECT_CHECK(CPUState, (obj), TYPE_CPU) > +/* Since this macro is used a lot in hot code paths and in conjunction with > + * FooCPU *foo_env_get_cpu(), we deviate from usual QOM practice by using > + * an unchecked cast. > + */ > +#define CPU(obj) ((CPUState *)(obj)) > + > #define CPU_CLASS(class) OBJECT_CLASS_CHECK(CPUClass, (class), TYPE_CPU) > #define CPU_GET_CLASS(obj) OBJECT_GET_CLASS(CPUClass, (obj), TYPE_CPU) > > -- > 1.8.4.5 > >
On Mon, Mar 31, 2014 at 7:37 PM, Andreas Färber <afaerber@suse.de> wrote: > CPU address spaces touching load and store helpers as well as the > movement of (almost) all fields from CPU_COMMON to CPUState have led to > a noticeable increase of CPU() usage in "hot" paths for both TCG and KVM. > > While CPU()'s OBJECT_CHECK() might help detect development errors, i.e. > in form of crashes due to QOM vs. non-QOM mismatches rather than QOM > type mismatches, it is not really needed at runtime since mostly used in > CPU-specific paths, coming from a target-specific CPU subtype. If that > pointer is damaged, other errors are highly likely to occur elsewhere > anyway. > > Keep the CPU() macro for a consistent developer experience and for > flexibility to exchange its implementation, but turn it into a pure, > unchecked C cast for now. I re-ran my image to measure the improvements. Intel 4770K (no HT, no OC) Fedora Core 19 QEMU 82c6f513735297ad76acaaf2e87f0c5a0b3647a7 Image: ARM kernel + Google V8 running Sunspider old: without this patch strong: default stack protection (strong in my case) noprot: no stack protection (--disable-stack-protector) disable: no QOM debug (--disable-qom-cast-debug) old+strong 16.7 old+strong+disable 15.9 old+noprot 16.6 old+noprot+disable 16.0 strong 15.8 strong+disable 15.7 noprot 15.6 noprot+disable 15.5 Thanks, Laurent > Compare commit 6e42be7cd10260fd3a006d94f6c870692bf7a2c0. > > Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Andreas Färber <afaerber@suse.de> > --- > include/qom/cpu.h | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index f99885a..df977c8 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -53,7 +53,12 @@ typedef uint64_t vaddr; > > #define TYPE_CPU "cpu" > > -#define CPU(obj) OBJECT_CHECK(CPUState, (obj), TYPE_CPU) > +/* Since this macro is used a lot in hot code paths and in conjunction with > + * FooCPU *foo_env_get_cpu(), we deviate from usual QOM practice by using > + * an unchecked cast. > + */ > +#define CPU(obj) ((CPUState *)(obj)) > + > #define CPU_CLASS(class) OBJECT_CLASS_CHECK(CPUClass, (class), TYPE_CPU) > #define CPU_GET_CLASS(obj) OBJECT_GET_CLASS(CPUClass, (obj), TYPE_CPU) > > -- > 1.8.4.5 > >
diff --git a/include/qom/cpu.h b/include/qom/cpu.h index f99885a..df977c8 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -53,7 +53,12 @@ typedef uint64_t vaddr; #define TYPE_CPU "cpu" -#define CPU(obj) OBJECT_CHECK(CPUState, (obj), TYPE_CPU) +/* Since this macro is used a lot in hot code paths and in conjunction with + * FooCPU *foo_env_get_cpu(), we deviate from usual QOM practice by using + * an unchecked cast. + */ +#define CPU(obj) ((CPUState *)(obj)) + #define CPU_CLASS(class) OBJECT_CLASS_CHECK(CPUClass, (class), TYPE_CPU) #define CPU_GET_CLASS(obj) OBJECT_GET_CLASS(CPUClass, (obj), TYPE_CPU)
CPU address spaces touching load and store helpers as well as the movement of (almost) all fields from CPU_COMMON to CPUState have led to a noticeable increase of CPU() usage in "hot" paths for both TCG and KVM. While CPU()'s OBJECT_CHECK() might help detect development errors, i.e. in form of crashes due to QOM vs. non-QOM mismatches rather than QOM type mismatches, it is not really needed at runtime since mostly used in CPU-specific paths, coming from a target-specific CPU subtype. If that pointer is damaged, other errors are highly likely to occur elsewhere anyway. Keep the CPU() macro for a consistent developer experience and for flexibility to exchange its implementation, but turn it into a pure, unchecked C cast for now. Compare commit 6e42be7cd10260fd3a006d94f6c870692bf7a2c0. Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com> Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de> --- include/qom/cpu.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)