Shiyuan Gao via <qemu-devel@nongnu.org> writes:
> From: Gao Shiyuan <gaoshiy...@baidu.com>
>
> No remove QemuOptsList from *_config_groups, so no need to
> traverse from the beginning every time.
>
> No functional changes.
>
> Signed-off-by: Gao Shiyuan <gaoshiy...@baidu.com>
> ---
> include/qemu/config-file.h | 3 +++
> util/qemu-config.c | 18 ++++++++----------
> 2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h
> index b82a778123..223ef7bc8f 100644
> --- a/include/qemu/config-file.h
> +++ b/include/qemu/config-file.h
> @@ -1,6 +1,9 @@
> #ifndef QEMU_CONFIG_FILE_H
> #define QEMU_CONFIG_FILE_H
>
> +#define MAX_VM_CONFIG_GROUPS 48
> +#define MAX_DRIVE_CONFIG_GROUPS 5
These are not used outside qemu-config.c, so it's better to define them
there.
> +
> typedef void QEMUConfigCB(const char *group, QDict *qdict, void *opaque,
> Error **errp);
>
> void qemu_load_module_for_opts(const char *group);
> diff --git a/util/qemu-config.c b/util/qemu-config.c
> index 42076efe1e..d7bab2047f 100644
> --- a/util/qemu-config.c
> +++ b/util/qemu-config.c
> @@ -9,8 +9,8 @@
> #include "qemu/config-file.h"
> #include "hw/boards.h"
>
> -static QemuOptsList *vm_config_groups[48];
> -static QemuOptsList *drive_config_groups[5];
> +static QemuOptsList *vm_config_groups[MAX_VM_CONFIG_GROUPS];
> +static QemuOptsList *drive_config_groups[MAX_DRIVE_CONFIG_GROUPS];
>
> static QemuOptsList *find_list(QemuOptsList **lists, const char *group,
> Error **errp)
> @@ -260,11 +260,10 @@ QemuOptsList *qemu_find_opts_err(const char *group,
> Error **errp)
>
> void qemu_add_drive_opts(QemuOptsList *list)
> {
> - int entries, i;
> + static int i;
> + static int entries = MAX_DRIVE_CONFIG_GROUPS - 1; /* keep list NULL
> terminated */
>
> - entries = ARRAY_SIZE(drive_config_groups);
> - entries--; /* keep list NULL terminated */
> - for (i = 0; i < entries; i++) {
> + for (; i < entries; i++) {
> if (drive_config_groups[i] == NULL) {
> drive_config_groups[i] = list;
> return;
> @@ -276,11 +275,10 @@ void qemu_add_drive_opts(QemuOptsList *list)
>
> void qemu_add_opts(QemuOptsList *list)
> {
> - int entries, i;
> + static int i;
> + static int entries = MAX_VM_CONFIG_GROUPS - 1; /* keep list NULL
> terminated */
>
> - entries = ARRAY_SIZE(vm_config_groups);
> - entries--; /* keep list NULL terminated */
> - for (i = 0; i < entries; i++) {
> + for (; i < entries; i++) {
> if (vm_config_groups[i] == NULL) {
> vm_config_groups[i] = list;
> return;
These two functions run at most 4 and 47 times, respectively. I don't
think speeding them up matters. Keeping them simple does.
Your patch conflates two separate changes.
1. Replace use of ARRAY_SIZE() by new macros. Doesn't feel like an
improvement to me.
2. Optimize appending to arrays drive_config_groups[] and
vm_config_groups[].
Let's have a look at 2. without 1.:
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 42076efe1e..7875359eb3 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -260,11 +260,13 @@ QemuOptsList *qemu_find_opts_err(const char *group, Error
**errp)
void qemu_add_drive_opts(QemuOptsList *list)
{
- int entries, i;
+ static int i;
+ int entries;
entries = ARRAY_SIZE(drive_config_groups);
entries--; /* keep list NULL terminated */
- for (i = 0; i < entries; i++) {
+
+ for (; i < entries; i++) {
if (drive_config_groups[i] == NULL) {
drive_config_groups[i] = list;
return;
@@ -276,11 +278,13 @@ void qemu_add_drive_opts(QemuOptsList *list)
void qemu_add_opts(QemuOptsList *list)
{
- int entries, i;
+ static int i;
+ int entries;
entries = ARRAY_SIZE(vm_config_groups);
entries--; /* keep list NULL terminated */
- for (i = 0; i < entries; i++) {
+
+ for (; i < entries; i++) {
if (vm_config_groups[i] == NULL) {
vm_config_groups[i] = list;
return;
This is a more focused patch.
It makes the functions faster, which doesn't matter. Does it make the
functions easier to understand? That would matter.
