| Message ID | 20231113141936.30567-1-gaoshiyuan@baidu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | util/qemu-config: Remove unnecessary traversal | expand |
Shiyuan Gao via <qemu-devel@nongnu.org> writes: > From: Gao Shiyuan <gaoshiyuan@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 <gaoshiyuan@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.
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 + 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;