[ovs-dev] [PATCH] json: Avoid extra memory allocation and string copy parsing object members.

Ben Pfaff blp at ovn.org
Sat Mar 31 19:38:30 UTC 2018


Thanks, applied to master.

On Mon, Mar 26, 2018 at 09:53:23AM -0700, Yifeng Sun wrote:
> Thanks for the fix, looks good to me.
> 
> 
> Reviewed-by: Yifeng Sun <pkusunyifeng at gmail.com>
> 
> On Fri, Mar 23, 2018 at 3:46 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> > Until now, every time the JSON parser added an object member, it made an
> > extra copy of the member name and then freed the original copy.  This is
> > wasteful, so this commit eliminates the extra copy.
> >
> > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > ---
> >  include/openvswitch/json.h  |  1 +
> >  include/openvswitch/shash.h |  1 +
> >  lib/json.c                  |  9 +++++++--
> >  lib/shash.c                 | 23 +++++++++++++++++++++++
> >  4 files changed, 32 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/openvswitch/json.h b/include/openvswitch/json.h
> > index 61b9a02cfc19..bcf6a27826ae 100644
> > --- a/include/openvswitch/json.h
> > +++ b/include/openvswitch/json.h
> > @@ -91,6 +91,7 @@ struct json *json_array_create_3(struct json *, struct
> > json *, struct json *);
> >
> >  struct json *json_object_create(void);
> >  void json_object_put(struct json *, const char *name, struct json *value);
> > +void json_object_put_nocopy(struct json *, char *name, struct json
> > *value);
> >  void json_object_put_string(struct json *,
> >                              const char *name, const char *value);
> >  void json_object_put_format(struct json *,
> > diff --git a/include/openvswitch/shash.h b/include/openvswitch/shash.h
> > index afc482364f24..c249e13e1f28 100644
> > --- a/include/openvswitch/shash.h
> > +++ b/include/openvswitch/shash.h
> > @@ -62,6 +62,7 @@ struct shash_node *shash_add_nocopy(struct shash *, char
> > *, const void *);
> >  bool shash_add_once(struct shash *, const char *, const void *);
> >  void shash_add_assert(struct shash *, const char *, const void *);
> >  void *shash_replace(struct shash *, const char *, const void *data);
> > +void *shash_replace_nocopy(struct shash *, char *name, const void *data);
> >  void shash_delete(struct shash *, struct shash_node *);
> >  char *shash_steal(struct shash *, struct shash_node *);
> >  struct shash_node *shash_find(const struct shash *, const char *);
> > diff --git a/lib/json.c b/lib/json.c
> > index 07ca87b2130f..603fd1df8b6a 100644
> > --- a/lib/json.c
> > +++ b/lib/json.c
> > @@ -279,6 +279,12 @@ json_object_put(struct json *json, const char *name,
> > struct json *value)
> >      json_destroy(shash_replace(json->u.object, name, value));
> >  }
> >
> > +void
> > +json_object_put_nocopy(struct json *json, char *name, struct json *value)
> > +{
> > +    json_destroy(shash_replace_nocopy(json->u.object, name, value));
> > +}
> > +
> >  void
> >  json_object_put_string(struct json *json, const char *name, const char
> > *value)
> >  {
> > @@ -1217,8 +1223,7 @@ json_parser_put_value(struct json_parser *p, struct
> > json *value)
> >  {
> >      struct json_parser_node *node = json_parser_top(p);
> >      if (node->json->type == JSON_OBJECT) {
> > -        json_object_put(node->json, p->member_name, value);
> > -        free(p->member_name);
> > +        json_object_put_nocopy(node->json, p->member_name, value);
> >          p->member_name = NULL;
> >      } else if (node->json->type == JSON_ARRAY) {
> >          json_array_add(node->json, value);
> > diff --git a/lib/shash.c b/lib/shash.c
> > index 318a30ffc0f0..a8433629ab84 100644
> > --- a/lib/shash.c
> > +++ b/lib/shash.c
> > @@ -166,6 +166,29 @@ shash_replace(struct shash *sh, const char *name,
> > const void *data)
> >      }
> >  }
> >
> > +/* Searches for 'name' in 'sh'.  If it does not already exist, adds it
> > along
> > + * with 'data' and returns NULL.  If it does already exist, replaces its
> > data
> > + * by 'data' and returns the data that it formerly contained.
> > + *
> > + * Takes ownership of 'name'. */
> > +void *
> > +shash_replace_nocopy(struct shash *sh, char *name, const void *data)
> > +{
> > +    size_t hash = hash_name(name);
> > +    struct shash_node *node;
> > +
> > +    node = shash_find__(sh, name, strlen(name), hash);
> > +    if (!node) {
> > +        shash_add_nocopy__(sh, name, data, hash);
> > +        return NULL;
> > +    } else {
> > +        free(name);
> > +        void *old_data = node->data;
> > +        node->data = CONST_CAST(void *, data);
> > +        return old_data;
> > +    }
> > +}
> > +
> >  /* Deletes 'node' from 'sh' and frees the node's name.  The caller is
> > still
> >   * responsible for freeing the node's data, if necessary. */
> >  void
> > --
> > 2.16.1
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >


More information about the dev mailing list