From 7bef5fab7d59aa9a6a1eb6ea747ba04811bc01e2 Mon Sep 17 00:00:00 2001

From: Markus Armbruster <armbru@redhat.com>

Date: Wed, 27 Jul 2016 07:35:10 +0200

Subject: [PATCH 12/16] qjson: surprise, allocating 6 QObjects per token is

 expensive

RH-Author: Markus Armbruster <armbru@redhat.com>

Message-id: <1469604913-12442-14-git-send-email-armbru@redhat.com>

Patchwork-id: 71475

O-Subject: [RHEL-7.3 qemu-kvm PATCH v2 12/15] qjson: surprise, allocating 6 QObjects per token is expensive

Bugzilla: 1276036

RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>

RH-Acked-by: John Snow <jsnow@redhat.com>

RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>

From: Paolo Bonzini <pbonzini@redhat.com>

Replace the contents of the tokens GQueue with a simple struct.  This cuts

the amount of memory allocated by tests/check-qjson from ~500MB to ~20MB,

and the execution time from 600ms to 80ms on my laptop.  Still a lot (some

could be saved by using an intrusive list, such as QSIMPLEQ, instead of

the GQueue), but the savings are already massive and the right thing to

do would probably be to get rid of json-streamer completely.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Message-Id: <1448300659-23559-5-git-send-email-pbonzini@redhat.com>

[Straightforwardly rebased on my patches]

Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

(cherry picked from commit 9bada8971173345ceb37ed1a47b00a01a4dd48cf)

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

Conflicts:

	qobject/json-parser.c

Straighforward conflicts because lacking commit fc48ffc "qobject: Use

'bool' for qbool", we still use qbool_from_int(), and we lack commit

e549e71 "json-parser: Accept 'null' in QMP".

Signed-off-by: Markus Armbruster <armbru@redhat.com>

---

 include/qapi/qmp/json-streamer.h |   7 +++

 qobject/json-parser.c            | 113 ++++++++++++++++-----------------------

 qobject/json-streamer.c          |  19 +++----

 3 files changed, 62 insertions(+), 77 deletions(-)

diff --git a/include/qapi/qmp/json-streamer.h b/include/qapi/qmp/json-streamer.h

index e9f2937..09b3d3e 100644

--- a/include/qapi/qmp/json-streamer.h

+++ b/include/qapi/qmp/json-streamer.h

@@ -18,6 +18,13 @@

 #include "glib-compat.h"

 #include "qapi/qmp/json-lexer.h"

+typedef struct JSONToken {

+    int type;

+    int x;

+    int y;

+    char str[];

+} JSONToken;

+

 typedef struct JSONMessageParser

 {

     void (*emit)(struct JSONMessageParser *parser, GQueue *tokens);

diff --git a/qobject/json-parser.c b/qobject/json-parser.c

index 6e5e257..944e1a1 100644

--- a/qobject/json-parser.c

+++ b/qobject/json-parser.c

@@ -23,11 +23,12 @@

 #include "qapi/qmp/json-parser.h"

 #include "qapi/qmp/json-lexer.h"

 #include "qapi/qmp/qerror.h"

+#include "qapi/qmp/json-streamer.h"

 typedef struct JSONParserContext

 {

     Error *err;

-    QObject *current;

+    JSONToken *current;

     GQueue *buf;

 } JSONParserContext;

@@ -45,27 +46,10 @@ typedef struct JSONParserContext

 static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);

 /**

- * Token manipulators

- *

- * tokens are dictionaries that contain a type, a string value, and geometry information

- * about a token identified by the lexer.  These are routines that make working with

- * these objects a bit easier.

- */

-static const char *token_get_value(QObject *obj)

-{

-    return qdict_get_str(qobject_to_qdict(obj), "token");

-}

-

-static JSONTokenType token_get_type(QObject *obj)

-{

-    return qdict_get_int(qobject_to_qdict(obj), "type");

-}

-

-/**

  * Error handler

  */

 static void GCC_FMT_ATTR(3, 4) parse_error(JSONParserContext *ctxt,

-                                           QObject *token, const char *msg, ...)

+                                           JSONToken *token, const char *msg, ...)

 {

     va_list ap;

     char message[1024];

@@ -143,9 +127,10 @@ static int hex2decimal(char ch)

  *      \t

  *      \u four-hex-digits 

  */

-static QString *qstring_from_escaped_str(JSONParserContext *ctxt, QObject *token)

+static QString *qstring_from_escaped_str(JSONParserContext *ctxt,

+                                         JSONToken *token)

 {

-    const char *ptr = token_get_value(token);

+    const char *ptr = token->str;

     QString *str;

     int double_quote = 1;

@@ -241,19 +226,19 @@ out:

     return NULL;

 }

-/* Note: unless the token object returned by parser_context_peek_token

- * or parser_context_pop_token is explicitly incref'd, it will be

- * deleted as soon as parser_context_pop_token is called again.

+/* Note: the token object returned by parser_context_peek_token or

+ * parser_context_pop_token is deleted as soon as parser_context_pop_token

+ * is called again.

  */

-static QObject *parser_context_pop_token(JSONParserContext *ctxt)

+static JSONToken *parser_context_pop_token(JSONParserContext *ctxt)

 {

-    qobject_decref(ctxt->current);

+    g_free(ctxt->current);

     assert(!g_queue_is_empty(ctxt->buf));

     ctxt->current = g_queue_pop_head(ctxt->buf);

     return ctxt->current;

 }

-static QObject *parser_context_peek_token(JSONParserContext *ctxt)

+static JSONToken *parser_context_peek_token(JSONParserContext *ctxt)

 {

     assert(!g_queue_is_empty(ctxt->buf));

     return g_queue_peek_head(ctxt->buf);

@@ -280,7 +265,7 @@ static void parser_context_free(JSONParserContext *ctxt)

         while (!g_queue_is_empty(ctxt->buf)) {

             parser_context_pop_token(ctxt);

         }

-        qobject_decref(ctxt->current);

+        g_free(ctxt->current);

         g_queue_free(ctxt->buf);

         g_free(ctxt);

     }

@@ -291,7 +276,8 @@ static void parser_context_free(JSONParserContext *ctxt)

  */

 static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *ap)

 {

-    QObject *key = NULL, *token = NULL, *value, *peek;

+    QObject *key = NULL, *value;

+    JSONToken *peek, *token;

     peek = parser_context_peek_token(ctxt);

     if (peek == NULL) {

@@ -311,7 +297,7 @@ static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *ap)

         goto out;

     }

-    if (token_get_type(token) != JSON_COLON) {

+    if (token->type != JSON_COLON) {

         parse_error(ctxt, token, "missing : in object pair");

         goto out;

     }

@@ -337,10 +323,10 @@ out:

 static QObject *parse_object(JSONParserContext *ctxt, va_list *ap)

 {

     QDict *dict = NULL;

-    QObject *token, *peek;

+    JSONToken *token, *peek;

     token = parser_context_pop_token(ctxt);

-    assert(token && token_get_type(token) == JSON_LCURLY);

+    assert(token && token->type == JSON_LCURLY);

     dict = qdict_new();

@@ -350,7 +336,7 @@ static QObject *parse_object(JSONParserContext *ctxt, va_list *ap)

         goto out;

     }

-    if (token_get_type(peek) != JSON_RCURLY) {

+    if (peek->type != JSON_RCURLY) {

         if (parse_pair(ctxt, dict, ap) == -1) {

             goto out;

         }

@@ -361,8 +347,8 @@ static QObject *parse_object(JSONParserContext *ctxt, va_list *ap)

             goto out;

         }

-        while (token_get_type(token) != JSON_RCURLY) {

-            if (token_get_type(token) != JSON_COMMA) {

+        while (token->type != JSON_RCURLY) {

+            if (token->type != JSON_COMMA) {

                 parse_error(ctxt, token, "expected separator in dict");

                 goto out;

             }

@@ -391,10 +377,10 @@ out:

 static QObject *parse_array(JSONParserContext *ctxt, va_list *ap)

 {

     QList *list = NULL;

-    QObject *token, *peek;

+    JSONToken *token, *peek;

     token = parser_context_pop_token(ctxt);

-    assert(token && token_get_type(token) == JSON_LSQUARE);

+    assert(token && token->type == JSON_LSQUARE);

     list = qlist_new();

@@ -404,7 +390,7 @@ static QObject *parse_array(JSONParserContext *ctxt, va_list *ap)

         goto out;

     }

-    if (token_get_type(peek) != JSON_RSQUARE) {

+    if (peek->type != JSON_RSQUARE) {

         QObject *obj;

         obj = parse_value(ctxt, ap);

@@ -421,8 +407,8 @@ static QObject *parse_array(JSONParserContext *ctxt, va_list *ap)

             goto out;

         }

-        while (token_get_type(token) != JSON_RSQUARE) {

-            if (token_get_type(token) != JSON_COMMA) {

+        while (token->type != JSON_RSQUARE) {

+            if (token->type != JSON_COMMA) {

                 parse_error(ctxt, token, "expected separator in list");

                 goto out;

             }

@@ -454,49 +440,45 @@ out:

 static QObject *parse_keyword(JSONParserContext *ctxt)

 {

-    QObject *token;

-    const char *val;

+    JSONToken *token;

     token = parser_context_pop_token(ctxt);

-    assert(token && token_get_type(token) == JSON_KEYWORD);

-    val = token_get_value(token);

+    assert(token && token->type == JSON_KEYWORD);

-    if (!strcmp(val, "true")) {

+    if (!strcmp(token->str, "true")) {

         return QOBJECT(qbool_from_int(true));

-    } else if (!strcmp(val, "false")) {

+    } else if (!strcmp(token->str, "false")) {

         return QOBJECT(qbool_from_int(false));

     }

-    parse_error(ctxt, token, "invalid keyword '%s'", val);

+    parse_error(ctxt, token, "invalid keyword '%s'", token->str);

     return NULL;

 }

 static QObject *parse_escape(JSONParserContext *ctxt, va_list *ap)

 {

-    QObject *token;

-    const char *val;

+    JSONToken *token;

     if (ap == NULL) {

         return NULL;

     }

     token = parser_context_pop_token(ctxt);

-    assert(token && token_get_type(token) == JSON_ESCAPE);

-    val = token_get_value(token);

+    assert(token && token->type == JSON_ESCAPE);

-    if (!strcmp(val, "%p")) {

+    if (!strcmp(token->str, "%p")) {

         return va_arg(*ap, QObject *);

-    } else if (!strcmp(val, "%i")) {

+    } else if (!strcmp(token->str, "%i")) {

         return QOBJECT(qbool_from_int(va_arg(*ap, int)));

-    } else if (!strcmp(val, "%d")) {

+    } else if (!strcmp(token->str, "%d")) {

         return QOBJECT(qint_from_int(va_arg(*ap, int)));

-    } else if (!strcmp(val, "%ld")) {

+    } else if (!strcmp(token->str, "%ld")) {

         return QOBJECT(qint_from_int(va_arg(*ap, long)));

-    } else if (!strcmp(val, "%lld") ||

-               !strcmp(val, "%I64d")) {

+    } else if (!strcmp(token->str, "%lld") ||

+               !strcmp(token->str, "%I64d")) {

         return QOBJECT(qint_from_int(va_arg(*ap, long long)));

-    } else if (!strcmp(val, "%s")) {

+    } else if (!strcmp(token->str, "%s")) {

         return QOBJECT(qstring_from_str(va_arg(*ap, const char *)));

-    } else if (!strcmp(val, "%f")) {

+    } else if (!strcmp(token->str, "%f")) {

         return QOBJECT(qfloat_from_double(va_arg(*ap, double)));

     }

     return NULL;

@@ -504,12 +486,12 @@ static QObject *parse_escape(JSONParserContext *ctxt, va_list *ap)

 static QObject *parse_literal(JSONParserContext *ctxt)

 {

-    QObject *token;

+    JSONToken *token;

     token = parser_context_pop_token(ctxt);

     assert(token);

-    switch (token_get_type(token)) {

+    switch (token->type) {

     case JSON_STRING:

         return QOBJECT(qstring_from_escaped_str(ctxt, token));

     case JSON_INTEGER: {

@@ -528,7 +510,7 @@ static QObject *parse_literal(JSONParserContext *ctxt)

         int64_t value;

         errno = 0; /* strtoll doesn't set errno on success */

-        value = strtoll(token_get_value(token), NULL, 10);

+        value = strtoll(token->str, NULL, 10);

         if (errno != ERANGE) {

             return QOBJECT(qint_from_int(value));

         }

@@ -536,8 +518,7 @@ static QObject *parse_literal(JSONParserContext *ctxt)

     }

     case JSON_FLOAT:

         /* FIXME dependent on locale */

-        return QOBJECT(qfloat_from_double(strtod(token_get_value(token),

-                                                 NULL)));

+        return QOBJECT(qfloat_from_double(strtod(token->str, NULL)));

     default:

         abort();

     }

@@ -545,7 +526,7 @@ static QObject *parse_literal(JSONParserContext *ctxt)

 static QObject *parse_value(JSONParserContext *ctxt, va_list *ap)

 {

-    QObject *token;

+    JSONToken *token;

     token = parser_context_peek_token(ctxt);

     if (token == NULL) {

@@ -553,7 +534,7 @@ static QObject *parse_value(JSONParserContext *ctxt, va_list *ap)

         return NULL;

     }

-    switch (token_get_type(token)) {

+    switch (token->type) {

     case JSON_LCURLY:

         return parse_object(ctxt, ap);

     case JSON_LSQUARE:

diff --git a/qobject/json-streamer.c b/qobject/json-streamer.c

index f7a3e78..e87230d 100644

--- a/qobject/json-streamer.c

+++ b/qobject/json-streamer.c

@@ -11,10 +11,6 @@

  *

  */

-#include "qapi/qmp/qlist.h"

-#include "qapi/qmp/qstring.h"

-#include "qapi/qmp/qint.h"

-#include "qapi/qmp/qdict.h"

 #include "qemu-common.h"

 #include "qapi/qmp/json-lexer.h"

 #include "qapi/qmp/json-streamer.h"

@@ -34,7 +30,7 @@ static void json_message_process_token(JSONLexer *lexer, GString *input,

                                        JSONTokenType type, int x, int y)

 {

     JSONMessageParser *parser = container_of(lexer, JSONMessageParser, lexer);

-    QDict *dict;

+    JSONToken *token;

     switch (type) {

     case JSON_LCURLY:

@@ -53,15 +49,16 @@ static void json_message_process_token(JSONLexer *lexer, GString *input,

         break;

     }

-    dict = qdict_new();

-    qdict_put(dict, "type", qint_from_int(type));

-    qdict_put(dict, "token", qstring_from_str(input->str));

-    qdict_put(dict, "x", qint_from_int(x));

-    qdict_put(dict, "y", qint_from_int(y));

+    token = g_malloc(sizeof(JSONToken) + input->len + 1);

+    token->type = type;

+    memcpy(token->str, input->str, input->len);

+    token->str[input->len] = 0;

+    token->x = x;

+    token->y = y;

     parser->token_size += input->len;

-    g_queue_push_tail(parser->tokens, dict);

+    g_queue_push_tail(parser->tokens, token);

     if (type == JSON_ERROR) {

         goto out_emit_bad;

-- 

1.8.3.1