diff mbox series

[RFC,PATCH-for-9.1] qapi: Do not generate commands/events/introspect code for user emulation

Message ID 20240404195543.9804-1-philmd@linaro.org
State New
Headers show
Series [RFC,PATCH-for-9.1] qapi: Do not generate commands/events/introspect code for user emulation | expand

Commit Message

Philippe Mathieu-Daudé April 4, 2024, 7:55 p.m. UTC
User emulation requires the QAPI types. Due to the command
line processing, some visitor code is also used. The rest
is irrelevant (no QMP socket).

Add an option to the qapi-gen script to allow generating
the minimum when only user emulation is being built.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
RFC: Quick PoC for Markus. It is useful for user-only builds.
---
 qapi/meson.build     |  6 +++++-
 scripts/qapi/main.py | 16 +++++++++++-----
 2 files changed, 16 insertions(+), 6 deletions(-)

Comments

Markus Armbruster April 5, 2024, 5:35 a.m. UTC | #1
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> User emulation requires the QAPI types. Due to the command
> line processing, some visitor code is also used. The rest
> is irrelevant (no QMP socket).
>
> Add an option to the qapi-gen script to allow generating
> the minimum when only user emulation is being built.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> RFC: Quick PoC for Markus. It is useful for user-only builds.
> ---
>  qapi/meson.build     |  6 +++++-
>  scripts/qapi/main.py | 16 +++++++++++-----
>  2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/qapi/meson.build b/qapi/meson.build
> index 375d564277..5e02621145 100644
> --- a/qapi/meson.build
> +++ b/qapi/meson.build
> @@ -115,10 +115,14 @@ foreach module : qapi_all_modules
>    endif
>  endforeach
>  
> +qapi_gen_cmd = [ qapi_gen, '-o', 'qapi', '-b', '@INPUT0@' ]
> +if not (have_system or have_tools)
> +  qapi_gen_cmd += [ '--types-only' ]
> +endif
>  qapi_files = custom_target('shared QAPI source files',
>    output: qapi_util_outputs + qapi_specific_outputs + qapi_nonmodule_outputs,
>    input: [ files('qapi-schema.json') ],
> -  command: [ qapi_gen, '-o', 'qapi', '-b', '@INPUT0@' ],
> +  command: qapi_gen_cmd,
>    depend_files: [ qapi_inputs, qapi_gen_depends ])
>  
>  # Now go through all the outputs and add them to the right sourceset.
> diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
> index 316736b6a2..925af5841b 100644
> --- a/scripts/qapi/main.py
> +++ b/scripts/qapi/main.py
> @@ -33,7 +33,8 @@ def generate(schema_file: str,
>               prefix: str,
>               unmask: bool = False,
>               builtins: bool = False,
> -             gen_tracing: bool = False) -> None:
> +             gen_tracing: bool = False,
> +             gen_types_only: bool = False) -> None:
>      """
>      Generate C code for the given schema into the target directory.
>  
> @@ -50,9 +51,10 @@ def generate(schema_file: str,
>      schema = QAPISchema(schema_file)
>      gen_types(schema, output_dir, prefix, builtins)
>      gen_visit(schema, output_dir, prefix, builtins)
> -    gen_commands(schema, output_dir, prefix, gen_tracing)
> -    gen_events(schema, output_dir, prefix)
> -    gen_introspect(schema, output_dir, prefix, unmask)
> +    if not gen_types_only:
> +        gen_commands(schema, output_dir, prefix, gen_tracing)
> +        gen_events(schema, output_dir, prefix)
> +        gen_introspect(schema, output_dir, prefix, unmask)

This is the behavior change, everything else is plumbing.  You suppress
generation of source code for commands, events, and introspection, i.e.

    qapi-commands*.[ch]
    qapi-init-commands.[ch]
    qapi-events*[ch]
    qapi-introspect.[ch]

and the associated .trace-events.

But none of these .c get compiled for a user-only build.

So, all we save is a bit of build time and disk space: less than 0.1s on
my machine, ~1.6MiB in ~220 files.  My linux-user-only build tree clocks
in at 317MiB in ~4900 files, a full build takes me around 30s (real
time, -j 14 with ccache), so we're talking about 0.5% in disk space and
0.3% in build time.

Moreover, the patch needs work:

    FAILED: tests/unit/test-qobject-input-visitor.p/test-qobject-input-visitor.c.o 
    cc [...] -c ../tests/unit/test-qobject-input-visitor.c
    ../tests/unit/test-qobject-input-visitor.c:27:10: fatal error: qapi/qapi-introspect.h: No such file or directory
       27 | #include "qapi/qapi-introspect.h"
          |          ^~~~~~~~~~~~~~~~~~~~~~~~
    FAILED: libqemuutil.a.p/stubs_monitor-core.c.o 
    cc [...] -c ../stubs/monitor-core.c
    ../stubs/monitor-core.c:3:10: fatal error: qapi/qapi-emit-events.h: No such file or directory
        3 | #include "qapi/qapi-emit-events.h"
          |          ^~~~~~~~~~~~~~~~~~~~~~~~~

I don't think it's worth the bother.

>  
>  
>  def main() -> int:
> @@ -75,6 +77,9 @@ def main() -> int:
>      parser.add_argument('-u', '--unmask-non-abi-names', action='store_true',
>                          dest='unmask',
>                          help="expose non-ABI names in introspection")
> +    parser.add_argument('-t', '--types-only', action='store_true',
> +                        dest='gen_types_only',
> +                        help="Only generate QAPI types")
>  
>      # Option --suppress-tracing exists so we can avoid solving build system
>      # problems.  TODO Drop it when we no longer need it.
> @@ -96,7 +101,8 @@ def main() -> int:
>                   prefix=args.prefix,
>                   unmask=args.unmask,
>                   builtins=args.builtins,
> -                 gen_tracing=not args.suppress_tracing)
> +                 gen_tracing=not args.suppress_tracing,
> +                 gen_types_only=args.gen_types_only)
>      except QAPIError as err:
>          print(err, file=sys.stderr)
>          return 1
Philippe Mathieu-Daudé April 5, 2024, 8:43 a.m. UTC | #2
Hi Markus,

On 5/4/24 07:35, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> User emulation requires the QAPI types. Due to the command
>> line processing, some visitor code is also used. The rest
>> is irrelevant (no QMP socket).
>>
>> Add an option to the qapi-gen script to allow generating
>> the minimum when only user emulation is being built.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> RFC: Quick PoC for Markus. It is useful for user-only builds.
>> ---
>>   qapi/meson.build     |  6 +++++-
>>   scripts/qapi/main.py | 16 +++++++++++-----
>>   2 files changed, 16 insertions(+), 6 deletions(-)


>> @@ -50,9 +51,10 @@ def generate(schema_file: str,
>>       schema = QAPISchema(schema_file)
>>       gen_types(schema, output_dir, prefix, builtins)
>>       gen_visit(schema, output_dir, prefix, builtins)
>> -    gen_commands(schema, output_dir, prefix, gen_tracing)
>> -    gen_events(schema, output_dir, prefix)
>> -    gen_introspect(schema, output_dir, prefix, unmask)
>> +    if not gen_types_only:
>> +        gen_commands(schema, output_dir, prefix, gen_tracing)
>> +        gen_events(schema, output_dir, prefix)
>> +        gen_introspect(schema, output_dir, prefix, unmask)
> 
> This is the behavior change, everything else is plumbing.  You suppress
> generation of source code for commands, events, and introspection, i.e.
> 
>      qapi-commands*.[ch]
>      qapi-init-commands.[ch]
>      qapi-events*[ch]
>      qapi-introspect.[ch]
> 
> and the associated .trace-events.
> 
> But none of these .c get compiled for a user-only build.
> 
> So, all we save is a bit of build time and disk space: less than 0.1s on
> my machine, ~1.6MiB in ~220 files.  My linux-user-only build tree clocks
> in at 317MiB in ~4900 files, a full build takes me around 30s (real
> time, -j 14 with ccache), so we're talking about 0.5% in disk space and
> 0.3% in build time.

What I want to catch is invalid uses of these headers in user-only
units.

See for example:
https://lore.kernel.org/qemu-devel/20240404194757.9343-5-philmd@linaro.org/

(Actually I have this patch based on that series).

> Moreover, the patch needs work:
> 
>      FAILED: tests/unit/test-qobject-input-visitor.p/test-qobject-input-visitor.c.o
>      cc [...] -c ../tests/unit/test-qobject-input-visitor.c
>      ../tests/unit/test-qobject-input-visitor.c:27:10: fatal error: qapi/qapi-introspect.h: No such file or directory
>         27 | #include "qapi/qapi-introspect.h"
>            |          ^~~~~~~~~~~~~~~~~~~~~~~~

I'd simply skip these tests on user-only builds.

>      FAILED: libqemuutil.a.p/stubs_monitor-core.c.o
>      cc [...] -c ../stubs/monitor-core.c
>      ../stubs/monitor-core.c:3:10: fatal error: qapi/qapi-emit-events.h: No such file or directory
>          3 | #include "qapi/qapi-emit-events.h"
>            |          ^~~~~~~~~~~~~~~~~~~~~~~~~

Doh, this is 
https://lore.kernel.org/qemu-devel/20240404194757.9343-4-philmd@linaro.org/, 
again I forgot:

Based-on: <20240404194757.9343-1-philmd@linaro.org>

> 
> I don't think it's worth the bother.

OK, I'll keep it locally until I finish my full exec/ rework then.

Regards,

Phil.
diff mbox series

Patch

diff --git a/qapi/meson.build b/qapi/meson.build
index 375d564277..5e02621145 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -115,10 +115,14 @@  foreach module : qapi_all_modules
   endif
 endforeach
 
+qapi_gen_cmd = [ qapi_gen, '-o', 'qapi', '-b', '@INPUT0@' ]
+if not (have_system or have_tools)
+  qapi_gen_cmd += [ '--types-only' ]
+endif
 qapi_files = custom_target('shared QAPI source files',
   output: qapi_util_outputs + qapi_specific_outputs + qapi_nonmodule_outputs,
   input: [ files('qapi-schema.json') ],
-  command: [ qapi_gen, '-o', 'qapi', '-b', '@INPUT0@' ],
+  command: qapi_gen_cmd,
   depend_files: [ qapi_inputs, qapi_gen_depends ])
 
 # Now go through all the outputs and add them to the right sourceset.
diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
index 316736b6a2..925af5841b 100644
--- a/scripts/qapi/main.py
+++ b/scripts/qapi/main.py
@@ -33,7 +33,8 @@  def generate(schema_file: str,
              prefix: str,
              unmask: bool = False,
              builtins: bool = False,
-             gen_tracing: bool = False) -> None:
+             gen_tracing: bool = False,
+             gen_types_only: bool = False) -> None:
     """
     Generate C code for the given schema into the target directory.
 
@@ -50,9 +51,10 @@  def generate(schema_file: str,
     schema = QAPISchema(schema_file)
     gen_types(schema, output_dir, prefix, builtins)
     gen_visit(schema, output_dir, prefix, builtins)
-    gen_commands(schema, output_dir, prefix, gen_tracing)
-    gen_events(schema, output_dir, prefix)
-    gen_introspect(schema, output_dir, prefix, unmask)
+    if not gen_types_only:
+        gen_commands(schema, output_dir, prefix, gen_tracing)
+        gen_events(schema, output_dir, prefix)
+        gen_introspect(schema, output_dir, prefix, unmask)
 
 
 def main() -> int:
@@ -75,6 +77,9 @@  def main() -> int:
     parser.add_argument('-u', '--unmask-non-abi-names', action='store_true',
                         dest='unmask',
                         help="expose non-ABI names in introspection")
+    parser.add_argument('-t', '--types-only', action='store_true',
+                        dest='gen_types_only',
+                        help="Only generate QAPI types")
 
     # Option --suppress-tracing exists so we can avoid solving build system
     # problems.  TODO Drop it when we no longer need it.
@@ -96,7 +101,8 @@  def main() -> int:
                  prefix=args.prefix,
                  unmask=args.unmask,
                  builtins=args.builtins,
-                 gen_tracing=not args.suppress_tracing)
+                 gen_tracing=not args.suppress_tracing,
+                 gen_types_only=args.gen_types_only)
     except QAPIError as err:
         print(err, file=sys.stderr)
         return 1