Eric Blake <ebl...@redhat.com> writes: > On 02/11/2018 03:36 AM, Markus Armbruster wrote: >> Our qapi-schema.json is composed of modules connected by include >> directives, but the generated code is monolithic all the same: one >> qapi-types.h with all the types, one qapi-visit.h with all the >> visitors, and so forth. These monolithic headers get included all >> over the place. In my "build everything" tree, adding a QAPI type >> recompiles about 4800 out of 5100 objects. >> >> We wouldn't write such monolithic headers by hand. It stands to >> reason that we shouldn't generate them, either. >> >> Split up generated qapi-types.h to mirror the schema's modular >> structure: one header per module. Name the main module's header >> qapi-types.h, and sub-module D/B.json's header D/qapi-types-B.h. >> >> Mirror the schema's includes in the headers, so that qapi-types.h gets >> you everything exactly as before. If you need less, you can include >> one or more of the sub-module headers. To be exploited shortly. >> >> Split up qapi-types.c, qapi-visit.h, qapi-visit.c, qmp-commands.h, >> qmp-commands.c, qapi-event.h, qapi-event.c the same way. >> qmp-introspect.h, qmp-introspect.c and qapi.texi remain monolithic. > > Make sense. > >> >> The split of qmp-commands.c duplicates static helper function >> qmp_marshal_output_str() in qapi-commands-char.c and >> qapi-commands-misc.c. This happens when commands returning the same >> type occur in multiple modules. Not worth avoiding. > > As long as it is static, and neither .c file includes the other, we're > fine (gdb may have a harder time figuring out which copy to put a > breakpoint on, but that's not too terrible). > >> >> Since I'm going to rename qapi-event.[ch] to qapi-events.[ch], and >> qmp-commands.[ch] to qapi-commands.[ch], name the shards that way >> already, to reduce churn. This requires temporary hacks in >> commands.py and events.py. They'll go away with the rename. > > The planned rename makes sense; prepping now is fine. > >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> .gitignore | 60 ++++++++++++++++++++++++ >> Makefile | 120 >> +++++++++++++++++++++++++++++++++++++++++++++++ >> Makefile.objs | 65 ++++++++++++++++++++++++- >> scripts/qapi/commands.py | 35 +++++++++----- >> scripts/qapi/common.py | 21 +++++++-- >> scripts/qapi/events.py | 19 ++++++-- >> 6 files changed, 300 insertions(+), 20 deletions(-) >> >> diff --git a/.gitignore b/.gitignore >> index 9477a08b6b..42c57998fd 100644 >> --- a/.gitignore >> +++ b/.gitignore >> @@ -31,7 +31,67 @@ >> /qapi-gen-timestamp >> /qapi-builtin-types.[ch] >> /qapi-builtin-visit.[ch] >> +/qapi/qapi-commands-block-core.[ch] >> +/qapi/qapi-commands-block.[ch] > > Is it worth using a glob instead of specific patterns, as in: > > /qapi/qapi-commands-*.[ch] > > Maybe being precise doesn't hurt, if we aren't adding sub-modules all > that often; but being generous with a glob makes it less likely that a > future module addition will forget to update .gitignore.
We should simply ditch building in-tree. I wanted to post patches for that for quite some time. >> +++ b/Makefile >> @@ -92,10 +92,70 @@ include $(SRC_PATH)/rules.mak >> GENERATED_FILES = qemu-version.h config-host.h qemu-options.def >> GENERATED_FILES += qapi-builtin-types.h qapi-builtin-types.c >> GENERATED_FILES += qapi-types.h qapi-types.c >> +GENERATED_FILES += qapi/qapi-types-block-core.h qapi/qapi-types-block-core.c > > For the makefile, I'd suggest using an intermediate list of module > names, and then using make string operations to expand it into larger > lists, to cut down on the repetition. Something like (untested): > > QAPI_MODULES = block-core block char common ... > GENERATED_FILES += $(addprefix qapi/qapi-types-,$(addsuffix > .c,$(QAPI_MODULES)) > GENERATED_FILES += $(addprefix qapi/qapi-types-,$(addsuffix > .h,$(QAPI_MODULES)) I did it the stupid way to save time. Improvements welcome :) >> @@ -525,10 +585,70 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json >> $(SRC_PATH)/qapi/common.json \ >> qapi-builtin-types.c qapi-builtin-types.h \ >> qapi-types.c qapi-types.h \ >> +qapi/qapi-types-block-core.c qapi/qapi-types-block-core.h \ >> +qapi/qapi-types-block.c qapi/qapi-types-block.h \ > > And repeating the list of filenames; again, an intermediate variable > would let you do: > > QAPI_GENERATED_FILES = ... > GENERATED_FILES += $(QAPI_GENERATED_FILES) > > $(QAPI_GENERATED_FILES): ... > >> qmp-introspect.h qmp-introspect.c \ >> qapi-doc.texi: \ >> qapi-gen-timestamp ; > > Not only does it cut down on the repetition, but it will make adding a > future module easier. > >> diff --git a/Makefile.objs b/Makefile.objs >> index 2813e984fd..7a55d45669 100644 >> --- a/Makefile.objs >> +++ b/Makefile.objs >> @@ -3,8 +3,56 @@ >> stub-obj-y = stubs/ crypto/ >> util-obj-y = util/ qobject/ qapi/ >> util-obj-y += qapi-builtin-types.o >> +util-obj-y += qapi-types.o >> +util-obj-y += qapi/qapi-types-block-core.o > > Perhaps this can also exploit make text manipulation? > > Otherwise looks good. Thanks!
