[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access |
Date: |
Tue, 18 Jun 2013 12:45:45 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 |
Am 18.06.2013 12:39, schrieb Michael S. Tsirkin:
> On Tue, Jun 18, 2013 at 08:35:22PM +1000, Peter Crosthwaite wrote:
>> Hi Michael,
>>
>> On Tue, Jun 18, 2013 at 8:23 PM, Michael S. Tsirkin <address@hidden> wrote:
>>> On Tue, Jun 18, 2013 at 07:43:11PM +1000, address@hidden wrote:
>>>> From: Peter Crosthwaite <address@hidden>
>>>>
>>>>
>>>> This series enables QOM super class access and demostrates some usages.
>>>> Replaces the save->override->call via FooClass technique, to reduce
>>>> some of the boiler plate in recently fully QOMified devices.
>>>>
>>>> Applied the change to ARM CPU, MB CPU and some of Andreas's recently
>>>> QOMified i386 devices, all which have the save->override->call issue.
>>>> ARMCPU I've done a brief test on and seems to work.
>>>>
>>>> ARM CPU was particularly difficult, as it has 3 layers of heirachy,
>>>> where a non-concrete class (TYPE_ARM_CPU) need to super class itself
>>>> (to TYPE_CPU). This sees the need for super-classers to specify their
>>>> expected base class level. See patches for illustration.
>>>>
>>>> The main future work to the series is to apply the change pattern to
>>>> the reset of the tree
>>>
>>> Looks good to me overall.
>>> Some nits:
>>> - Super is an immediate parent in java and python.
>>
>> s/Super/parent might be the go. But it is designed to work like
>> py/java. Its the immediate parent of the specified level, and it is
>> analogous to the java super.
>>
>>> - One of the design points of QOM is that it let
>>> you ignore which class is a parent and which is a child.
>>> All casts look the same.
>>>
>>> So, why do we need the new APIs with _SUPER?
>>
>> The SUPER APIs have a nice consistent appearance with the GET_CLASS
>> and FOO_CLASS APIs and they are likely to live alongside each other in
>> QOM fns.
>>
>>> What's wrong with simple
>>> object_class_by_name()
>>> and casting to that?
>>>
>>
>> There a performance consequence - object_class_by_name is a lookup,
>> whereas this approach is able to just walk the pointer through the
>> inheritance heirachy till it hits.
>
> None of these uses has a chance to be at all performance
> sensitive.
>
>
>> Tying it to an object also brings
>> into play the possibility of a cast cache should that be needed.
>
> Doesn't make sense to me. This is object construction,
> how many classes do you expect there to be so
> this would affect qemu startup speed noticeably?
> 1000000?
It's not just about startup, also about CPU reset for instance.
But still not that performance-sensitive.
If we want to performance-optimize it, sharing code with
object_class_by_name() or any other class-comparing function would be nice.
Andreas
>
>> Thanks for the review.
>>
>> Regards,
>> Peter
>>
>>>
>>>>
>>>> Peter Crosthwaite (7):
>>>> target-arm/cpu.c: delete un-needed instance/class sizes
>>>> qom: Add super class accessor
>>>> qdev-core: Introduce DEVICE super class cast macros
>>>> qom/cpu: Introduce CPU super class cast macros
>>>> target-arm: Remove ARMCPUClass
>>>> target-microblaze: Remove MicroblazeCPUClass
>>>> i8254: Remove [KVM]PITClass
>>>>
>>>> hw/i386/kvm/i8254.c | 17 ++---------------
>>>> hw/timer/i8254.c | 16 ++--------------
>>>> include/hw/qdev-core.h | 4 ++++
>>>> include/qom/cpu.h | 4 ++++
>>>> include/qom/object.h | 18 ++++++++++++++++++
>>>> qom/object.c | 15 +++++++++++++++
>>>> target-arm/cpu-qom.h | 20 --------------------
>>>> target-arm/cpu.c | 16 +++++-----------
>>>> target-microblaze/cpu-qom.h | 20 --------------------
>>>> target-microblaze/cpu.c | 13 ++++---------
>>>> 10 files changed, 54 insertions(+), 89 deletions(-)
>>>>
>>>> --
>>>> 1.8.3.rc1.44.gb387c77.dirty
>>>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
- [Qemu-devel] [RFC PATCH v1 5/7] target-arm: Remove ARMCPUClass, (continued)
- [Qemu-devel] [RFC PATCH v1 5/7] target-arm: Remove ARMCPUClass, peter . crosthwaite, 2013/06/18
- [Qemu-devel] [RFC PATCH v1 6/7] target-microblaze: Remove MicroblazeCPUClass, peter . crosthwaite, 2013/06/18
- [Qemu-devel] [RFC PATCH v1 7/7] i8254: Remove [KVM]PITClass, peter . crosthwaite, 2013/06/18
- Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access, Andreas Färber, 2013/06/18
- Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access, Michael S. Tsirkin, 2013/06/18
- Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access, Andreas Färber, 2013/06/18