Bin Guo <[email protected]> writes: > For every NULL slot in a VMS_ARRAY_OF_POINTER (or every entry of a > dynamic array), the saver allocates a 1-element fake VMStateField via > g_new0 and frees it again right after the save. For arrays of > thousands of entries this is thousands of malloc/free pairs on the > hot save path. > > Replace the heap-allocated marker with a stack-resident field > populated by an init helper. The caller passes a pointer to a local > VMStateField, the helper fills it in (still asserting the > precondition), and no g_free is needed. > > Signed-off-by: Bin Guo <[email protected]> > --- > migration/vmstate.c | 41 ++++++++++++++++------------------------- > 1 file changed, 16 insertions(+), 25 deletions(-) > > diff --git a/migration/vmstate.c b/migration/vmstate.c > index 2f13b48a37..9ced78532f 100644 > --- a/migration/vmstate.c > +++ b/migration/vmstate.c > @@ -59,29 +59,23 @@ vmstate_field_exists(const VMStateDescription *vmsd, > const VMStateField *field, > * array of a VMS_ARRAY_OF_POINTER VMSD field. It's needed because we > * can't dereference the NULL pointer. > */ > -static const VMStateField * > -vmsd_create_ptr_marker_field(const VMStateField *field) > +static void > +vmsd_init_ptr_marker_field(VMStateField *fake, const VMStateField *field) > { > - VMStateField *fake = g_new0(VMStateField, 1); > - > /* It can only happen on an array of pointers! */ > assert(field->flags & VMS_ARRAY_OF_POINTER); > > - /* Some of fake's properties should match the original's */ > - fake->name = field->name; > - fake->version_id = field->version_id; > - > - /* Do not need "field_exists" check as it always exists */ > - fake->field_exists = NULL; > - > - /* See vmstate_info_ptr_marker - use 1 byte to represent ptr status */ > - fake->size = 1; > - fake->info = &vmstate_info_ptr_marker; > - fake->flags = VMS_SINGLE; > - > - /* All the rest fields shouldn't matter.. */ > - > - return (const VMStateField *)fake; > + /* See vmstate_info_ptr_marker - 1 byte represents ptr status */ > + *fake = (VMStateField) { > + .name = field->name, > + .version_id = field->version_id, > + /* Marker always exists, no field_exists callback needed */ > + .field_exists = NULL, > + .size = 1, > + .info = &vmstate_info_ptr_marker, > + .flags = VMS_SINGLE, > + /* All other fields stay zero-initialised */ > + }; > } > > static int vmstate_n_elems(void *opaque, const VMStateField *field) > @@ -680,6 +674,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const > VMStateDescription *vmsd, > for (i = 0; i < n_elems; i++) { > void *curr_elem = first_elem + size * i; > const VMStateField *inner_field; > + VMStateField marker_field; > /* maximum number of elements to compress in the JSON blob */ > int max_elems = vmsd_can_compress(field) ? (n_elems - i) : 1; > bool use_marker_field, is_null = false; > @@ -693,7 +688,8 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const > VMStateDescription *vmsd, > use_marker_field = use_dynamic_array || is_null; > > if (use_marker_field) { > - inner_field = vmsd_create_ptr_marker_field(field); > + vmsd_init_ptr_marker_field(&marker_field, field); > + inner_field = &marker_field; > } else { > inner_field = field; > } > @@ -730,11 +726,6 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const > VMStateDescription *vmsd, > inner_field, vmdesc_loop, > i, max_elems, errp); > > - /* If we used a fake temp field.. free it now */ > - if (use_marker_field) { > - g_clear_pointer((gpointer *)&inner_field, g_free); > - } > - > if (!ok) { > goto out; > }
Reviewed-by: Fabiano Rosas <[email protected]>
