All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: John Snow <jsnow@redhat.com>
Cc: Cleber Rosa <crosa@redhat.com>,
	qemu-devel@nongnu.org, Eduardo Habkost <ehabkost@redhat.com>
Subject: introspect.py output representation (was: [PATCH v2 00/11] qapi: static typing conversion, pt2)
Date: Mon, 16 Nov 2020 14:17:32 +0100	[thread overview]
Message-ID: <87y2j1xwyb.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <20201026194251.11075-1-jsnow@redhat.com> (John Snow's message of "Mon, 26 Oct 2020 15:42:40 -0400")

Warning: losely related brain dump ahead.

introspect.py's purpose is providing query-qmp-schema with the data it
needs to built its response, which is a JSON object conforming to
['SchemaInfo'].

Stupidest solution that could possibly work: have this module generate a
C string holding the (JSON text) response.

Since a QMP command handler returns a QAPI object, not the response
string, this becomes:

     schema
        |
        |                                       qapi-gen.py
        v
    C string        --------------------------------------------------
        |
        |           qobject_from_json()
        v
     QObject                                    qmp_query_qmp_schema()
        |
        |           QObject input visitor
        v
  SchemaInfoList    --------------------------------------------------
        |
        |           QObject output visitor      generated wrapper
        v
     QObject        --------------------------------------------------
        |
        |           qobject_to_json()           QMP core
        v
    C string

Meh.  So many pointless conversions.

Shortcut: 'gen' false lets us cut out two:

     schema
        |
        |                                       qapi-gen.py
        v
    C string        --------------------------------------------------
        |
        |           qobject_from_json()         qmp_query_qmp_schema()
        v
     QObject        --------------------------------------------------
        |
        |           qobject_to_json()           QMP core
        v
    C string

Less work for handwritten qmp_query_qmp_schema(); it's now a one-liner.
This is the initial version (commit 39a1815816).

Commit 7d0f982bfb replaced the generated C string by a QLitObject:

     schema
        |
        |                                       qapi-gen.py
        v
     QLitObj        --------------------------------------------------
        |
        |           qobject_from_qlit()         qmp_query_qmp_schema()
        v
     QObject        --------------------------------------------------
        |
        |           qobject_to_json()           QMP core
        v
    C string

The commit message claims the QLitObj is "easier to deal with".  I doubt
it.  The conversion to QObject is a one-liner before and after.  Neither
form is hard to generate (example appended).

QLitObj takes more memory: ~360KiB, mostly .data (unnecessarily?),
wheras the C string is ~150KiB .text.  Of course, both are dwarved many
times over by QObject: 12.4MiB.  Gross.

However, to actually work with the introspection data in C, we'd want it
as SchemaInfoList.  I have a few uses for introspection data in mind,
and I definitely won't code them taking QObject input.

SchemaInfoList is one visitor away from QObject, so no big deal.

But what if we generated SchemaInfoList directly?  We'd have:

     schema
        |
        |                                       qapi-gen.py
        v
  SchemaInfoList    --------------------------------------------------
        |
        |           QObject output visitor      generated wrapper
        v
     QObject        --------------------------------------------------
        |
        |           qobject_to_json()           QMP core
        v
    C string

At ~480KiB, SchemaInfoList is less compact than QLitObj (let alone the C
string).  It should go entirely into .text, though.

I don't think generating SchemaInfoList would be particularly hard.
Just work.  Bigger fish to fry, I guess.  But getting the idea out can't
hurt.



Example: BlockdevOptionsFile

    { 'struct': 'BlockdevOptionsFile',
      'data': { 'filename': 'str',
                '*pr-manager': 'str',
                '*locking': 'OnOffAuto',
                '*aio': 'BlockdevAioOptions',
                '*drop-cache': {'type': 'bool',
                                'if': 'defined(CONFIG_LINUX)'},
                '*x-check-cache-dropped': 'bool' },
      'features': [ { 'name': 'dynamic-auto-read-only',
                      'if': 'defined(CONFIG_POSIX)' } ] }

Generated QLitObj:

        /* "267" = BlockdevOptionsFile */
        QLIT_QDICT(((QLitDictEntry[]) {
            { "features", QLIT_QLIST(((QLitObject[]) {
    #if defined(CONFIG_POSIX)
                QLIT_QSTR("dynamic-auto-read-only"),
    #endif /* defined(CONFIG_POSIX) */
                {}
            })), },
            { "members", QLIT_QLIST(((QLitObject[]) {
                QLIT_QDICT(((QLitDictEntry[]) {
                    { "name", QLIT_QSTR("filename"), },
                    { "type", QLIT_QSTR("str"), },
                    {}
                })),
                QLIT_QDICT(((QLitDictEntry[]) {
                    { "default", QLIT_QNULL, },
                    { "name", QLIT_QSTR("pr-manager"), },
                    { "type", QLIT_QSTR("str"), },
                    {}
                })),
                QLIT_QDICT(((QLitDictEntry[]) {
                    { "default", QLIT_QNULL, },
                    { "name", QLIT_QSTR("locking"), },
                    { "type", QLIT_QSTR("412"), },
                    {}
                })),
                QLIT_QDICT(((QLitDictEntry[]) {
                    { "default", QLIT_QNULL, },
                    { "name", QLIT_QSTR("aio"), },
                    { "type", QLIT_QSTR("413"), },
                    {}
                })),
    #if defined(CONFIG_LINUX)
                QLIT_QDICT(((QLitDictEntry[]) {
                    { "default", QLIT_QNULL, },
                    { "name", QLIT_QSTR("drop-cache"), },
                    { "type", QLIT_QSTR("bool"), },
                    {}
                })),
    #endif /* defined(CONFIG_LINUX) */
                QLIT_QDICT(((QLitDictEntry[]) {
                    { "default", QLIT_QNULL, },
                    { "name", QLIT_QSTR("x-check-cache-dropped"), },
                    { "type", QLIT_QSTR("bool"), },
                    {}
                })),
                {}
            })), },
            { "meta-type", QLIT_QSTR("object"), },
            { "name", QLIT_QSTR("267"), },
            {}
        })),

Generated C string would look like this:

        "{\"features\": ["
    #if defined(CONFIG_POSIX)
        "\"dynamic-auto-read-only\""
    #endif /* defined(CONFIG_POSIX) */
        "], "
        "\"members\": ["
        "{\"name\": \"filename\", \"type\": \"str\"}, "
        "{\"name\": \"pr-manager\", \"default\": null, \"type\": \"str\"}, "
        "{\"name\": \"locking\", \"default\": null, \"type\": \"412\"}, "
        "{\"name\": \"aio\", \"default\": null, \"type\": \"413\"}, "
    #if defined(CONFIG_LINUX)
        "{\"name\": \"drop-cache\", \"default\": null, \"type\": \"bool\"}, "
    #endif /* defined(CONFIG_LINUX) */
        "{\"name\": \"x-check-cache-dropped\", \"default\": null, \"type\": \"bool\"}"
        "], "
        "\"meta-type\": \"object\", "
        "\"name\": \"267\""
        "}"



      parent reply	other threads:[~2020-11-16 13:23 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-26 19:42 [PATCH v2 00/11] qapi: static typing conversion, pt2 John Snow
2020-10-26 19:42 ` [PATCH v2 01/11] [DO-NOT-MERGE] docs: replace single backtick (`) with double-backtick (``) John Snow
2020-10-26 19:42 ` [PATCH v2 02/11] [DO-NOT-MERGE] docs/sphinx: change default role to "any" John Snow
2020-10-26 19:42 ` [PATCH v2 03/11] [DO-NOT-MERGE] docs: enable sphinx-autodoc for scripts/qapi John Snow
2020-10-26 19:42 ` [PATCH v2 04/11] qapi/introspect.py: add assertions and casts John Snow
2020-11-06 18:59   ` Cleber Rosa
2020-10-26 19:42 ` [PATCH v2 05/11] qapi/introspect.py: add preliminary type hint annotations John Snow
2020-11-07  2:12   ` Cleber Rosa
2020-12-07 21:29     ` John Snow
2020-11-13 16:48   ` Markus Armbruster
2020-12-07 23:48     ` John Snow
2020-12-16  7:51       ` Markus Armbruster
2020-12-16 17:49         ` Eduardo Habkost
2020-12-17  6:51           ` Markus Armbruster
2020-12-17  1:35         ` John Snow
2020-12-17  7:00           ` Markus Armbruster
2020-10-26 19:42 ` [PATCH v2 06/11] qapi/introspect.py: add _gen_features helper John Snow
2020-11-07  4:23   ` Cleber Rosa
2020-11-16  8:47   ` Markus Armbruster
2020-12-07 23:57     ` John Snow
2020-12-15 16:55       ` Markus Armbruster
2020-12-15 18:49         ` John Snow
2020-10-26 19:42 ` [PATCH v2 07/11] qapi/introspect.py: Unify return type of _make_tree() John Snow
2020-11-07  5:08   ` Cleber Rosa
2020-12-15  0:22     ` John Snow
2020-11-16  9:46   ` Markus Armbruster
2020-12-08  0:06     ` John Snow
2020-12-16  6:35       ` Markus Armbruster
2020-10-26 19:42 ` [PATCH v2 08/11] qapi/introspect.py: replace 'extra' dict with 'comment' argument John Snow
2020-11-07  5:10   ` Cleber Rosa
2020-11-16  9:55   ` Markus Armbruster
2020-12-08  0:12     ` John Snow
2020-10-26 19:42 ` [PATCH v2 09/11] qapi/introspect.py: create a typed 'Annotated' data strutcure John Snow
2020-11-07  5:45   ` Cleber Rosa
2020-11-16 10:12   ` Markus Armbruster
2020-12-08  0:21     ` John Snow
2020-12-16  7:08       ` Markus Armbruster
2020-12-17  1:30         ` John Snow
2020-10-26 19:42 ` [PATCH v2 10/11] qapi/introspect.py: improve readability of _tree_to_qlit John Snow
2020-11-07  5:54   ` Cleber Rosa
2020-11-16 10:17   ` Markus Armbruster
2020-12-15 15:25     ` John Snow
2020-10-26 19:42 ` [PATCH v2 11/11] qapi/introspect.py: Add docstring to _tree_to_qlit John Snow
2020-11-07  5:57   ` Cleber Rosa
2020-11-02 15:40 ` [PATCH v2 00/11] qapi: static typing conversion, pt2 John Snow
2020-11-04  9:51   ` Marc-André Lureau
2020-12-15 15:52     ` John Snow
2020-11-16 13:17 ` Markus Armbruster [this message]

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=87y2j1xwyb.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=crosa@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=jsnow@redhat.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.