Re: [pve-devel] [PATCH qemu-server 1/1] config: only fetch necessary default values in get_derived_property helper

[Fiona Ebner <f.ebner@proxmox.com>]

Am 30.09.25 um 4:20 PM schrieb Daniel Kral:
> get_derived_property(...) is called in the semi-hot path of the HA
> Manager's static load scheduler to retrieve the static stats of each VM.
> As the defaults are only needed in certain cases and for a very small
> subset of properties in the VM config, get those separately when needed.
> 
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
> get_current_memory(...) is still quite costly here, because it calls
> parse_memory(...), which calls
> PVE::JSONSchema::parse_property_string(...), which adds up for many
> guest configurations parsed in every manage(...) call, but this already
> helps quite a lot.

If this really is a problem, we could do our own parsing, i.e. returning
the value if the property string starts with \d+ or current=\d+ and
falling back to get_current_memory() if it doesn't. Of course also using
the default from $memory_fmt if not set.

> 
>  src/PVE/QemuConfig.pm | 8 +++-----
>  src/PVE/QemuServer.pm | 6 ++++++
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/src/PVE/QemuConfig.pm b/src/PVE/QemuConfig.pm
> index d0844c4c..078c87e0 100644
> --- a/src/PVE/QemuConfig.pm
> +++ b/src/PVE/QemuConfig.pm
> @@ -582,12 +582,10 @@ sub load_current_config {

We could go a step further and save the three defaults we are interested
in during module load into variables. Then you also save the hash
accesses into $confdesc and $memory_fmt.

>  sub get_derived_property {
>      my ($class, $conf, $name) = @_;
>  
> -    my $defaults = PVE::QemuServer::load_defaults();
> -
>      if ($name eq 'max-cpu') {
> -        my $cpus =
> -            ($conf->{sockets} || $defaults->{sockets}) * ($conf->{cores} || $defaults->{cores});
> -        return $conf->{vcpus} || $cpus;
> +        my $sockets = $conf->{sockets} || PVE::QemuServer::get_default_property_value('sockets');
> +        my $cores = $conf->{cores} || PVE::QemuServer::get_default_property_value('cores');
> +        return $conf->{vcpus} || ($sockets * $cores);
>      } elsif ($name eq 'max-memory') { # current usage maximum, not maximum hotpluggable
>          return get_current_memory($conf->{memory}) * 1024 * 1024;
>      } else {

Question is, how much do we really need to optimize the function here?
I'm not against it, but just want to note that looking at the static
usage will always have its limitations in practice (independent of
performance). With PSI-based usage, we should only need the static
information for to-be-started or to-be-recovered guests rather than all,
so performance of get_derived_property() becomes much less relevant. Or
what do you think?


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel