From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, kwolf@redhat.com, hreitz@redhat.com,
peter.maydell@linaro.org
Subject: Re: [RFC PATCH] qobject: Rewrite implementation of QDict for in-order traversal
Date: Fri, 8 Jul 2022 12:01:24 +0100 [thread overview]
Message-ID: <YsgOhJLpbyODJCGG@redhat.com> (raw)
In-Reply-To: <87wncqmq2t.fsf@pond.sub.org>
On Wed, Jul 06, 2022 at 01:35:22PM +0200, Markus Armbruster wrote:
> Markus Armbruster <armbru@redhat.com> writes:
>
> > QDict is implemented as a simple hash table of fixed size. Observe:
> >
> > * Slow for large n. Not sure this matters.
> >
> > * A QDict with n entries takes 4120 + n * 32 bytes on my box. Wastes
> > space for small n, which is a common case.
> >
> > * Order of traversal depends on the hash function and on insertion
> > order, because it iterates first over buckets, then collision
> > chains.
> >
> > * Special code ensures qdict_size() takes constant time.
> >
> > Replace the hash table by a linked list. Observe:
> >
> > * Even slower for large n. Might be bad enough to matter.
> >
> > * A QDict with n entries takes 32 + n * 24 bytes.
> >
> > * Traversal is in insertion order.
> >
> > * qdict_size() is linear in the number of entries.
> >
> > This is an experiment. Do not commit to master as is.
>
> Forgot to mention: see also
>
> Subject: Re: [PULL 14/15] qdev: Base object creation on QDict rather than QemuOpts
> Message-ID: <87wnctzdl9.fsf@pond.sub.org>
> https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg00358.html
What alternative options do we have for addressing this scenario.
I can think of
- Auto-create array elements, if seeing an element set before length.
This is based on the theory that 'len-PROP' field is largely
redundant. It is only needed if you want to create a sparse
array, with empty elements /after/ the last one explicitly
set, or if you want to get error reporting for an app setting
element 3 after saying it wanted a 2 element list. IMHO the
error reporting benefit is dubious, because the error scenario
only exists because we made the app set this redundant 'len-PROP'
attribute. Does anything actually need the 'sparse array'
facility ?
- Special case array properties
Modify object_set_properties_from_qdict, so that it has a special
case first iterating over any properties with 'len-' prefix in
their name, then iterating over everything else.
Assuming this 'len-' property is the only case where we genuinely
have ordering dependancies between properties, this is quite a
simple fix, and avoid imposes ordering requirements on either
clients or QEMU in general.
- Insertion order preserving QDict
What you've done here, pushing the ordering problem off to be
the caller's responsibility to get right. The caller could
easily have the same problem though. For example, for CLI args
these days, libvirt will populate a data structure based on
QAPI, and then serialize that to CLI args. I don't know offhand
if our code is insertion order preserving, or will hit this
exact same problem. Luckily we don't support the 'rocker'
object so havent hit this precise issue.
- Any other options ?
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
next prev parent reply other threads:[~2022-07-08 11:19 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-05 9:54 [RFC PATCH] qobject: Rewrite implementation of QDict for in-order traversal Markus Armbruster
2022-07-06 11:35 ` Markus Armbruster
2022-07-06 11:49 ` Mark Cave-Ayland
2022-07-08 11:01 ` Daniel P. Berrangé [this message]
2022-07-11 10:32 ` Peter Maydell
2022-07-11 11:09 ` Daniel P. Berrangé
2022-07-11 11:15 ` Peter Maydell
2022-07-18 10:45 ` Markus Armbruster
2022-07-07 12:57 ` Peter Maydell
2022-07-07 14:27 ` Markus Armbruster
2022-07-07 15:07 ` Daniel P. Berrangé
2022-07-07 14:43 ` Daniel P. Berrangé
2022-07-07 15:37 ` Daniel P. Berrangé
2022-07-07 15:52 ` Alex Bennée
2022-07-08 10:19 ` Markus Armbruster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YsgOhJLpbyODJCGG@redhat.com \
--to=berrange@redhat.com \
--cc=armbru@redhat.com \
--cc=hreitz@redhat.com \
--cc=kwolf@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).