qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: armbru@redhat.com
Subject: [Qemu-devel] [PATCH 4/4] qjson: surprise, allocating 6 QObjects per token is expensive
Date: Fri, 20 Nov 2015 10:04:29 +0100	[thread overview]
Message-ID: <1448010269-21694-5-git-send-email-pbonzini@redhat.com> (raw)
In-Reply-To: <1448010269-21694-1-git-send-email-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>
---
 include/qapi/qmp/json-streamer.h |  7 ++++
 qobject/json-parser.c            | 81 +++++++++++++++++++---------------------
 qobject/json-streamer.c          | 19 ++++------
 3 files changed, 53 insertions(+), 54 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 07d9654..0230bc9 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -22,11 +22,12 @@
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/json-lexer.h"
+#include "qapi/qmp/json-streamer.h"
 
 typedef struct JSONParserContext
 {
     Error *err;
-    QObject *current;
+    JSONToken *current;
     GQueue *buf;
 } JSONParserContext;
 
@@ -46,56 +47,46 @@ static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
 /**
  * Token manipulators
  *
- * tokens are dictionaries that contain a type, a string value, and geometry information
+ * tokens 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");
-}
-
-static int token_is_operator(QObject *obj, char op)
+static int token_is_operator(JSONToken *obj, char op)
 {
     const char *val;
 
-    if (token_get_type(obj) != JSON_OPERATOR) {
+    if (obj->type != JSON_OPERATOR) {
         return 0;
     }
 
-    val = token_get_value(obj);
+    val = obj->str;
 
     return (val[0] == op) && (val[1] == 0);
 }
 
-static int token_is_keyword(QObject *obj, const char *value)
+static int token_is_keyword(JSONToken *obj, const char *value)
 {
-    if (token_get_type(obj) != JSON_KEYWORD) {
+    if (obj->type != JSON_KEYWORD) {
         return 0;
     }
 
-    return strcmp(token_get_value(obj), value) == 0;
+    return strcmp(obj->str, value) == 0;
 }
 
-static int token_is_escape(QObject *obj, const char *value)
+static int token_is_escape(JSONToken *obj, const char *value)
 {
-    if (token_get_type(obj) != JSON_ESCAPE) {
+    if (obj->type != JSON_ESCAPE) {
         return 0;
     }
 
-    return (strcmp(token_get_value(obj), value) == 0);
+    return (strcmp(obj->str, value) == 0);
 }
 
 /**
  * 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];
@@ -173,9 +164,9 @@ 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;
 
@@ -271,19 +262,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);
@@ -310,7 +301,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);
     }
@@ -321,7 +312,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) {
@@ -367,7 +359,7 @@ out:
 static QObject *parse_object(JSONParserContext *ctxt, va_list *ap)
 {
     QDict *dict = NULL;
-    QObject *token, *peek;
+    JSONToken *token, *peek;
 
     token = parser_context_peek_token(ctxt);
     if (token == NULL) {
@@ -428,7 +420,7 @@ out:
 static QObject *parse_array(JSONParserContext *ctxt, va_list *ap)
 {
     QList *list = NULL;
-    QObject *token, *peek;
+    JSONToken *token, *peek;
 
     token = parser_context_peek_token(ctxt);
     if (token == NULL) {
@@ -498,14 +490,15 @@ out:
 
 static QObject *parse_keyword(JSONParserContext *ctxt)
 {
-    QObject *token, *ret;
+    JSONToken *token;
+    QObject *ret;
 
     token = parser_context_peek_token(ctxt);
     if (token == NULL) {
         goto out;
     }
 
-    if (token_get_type(token) != JSON_KEYWORD) {
+    if (token->type != JSON_KEYWORD) {
         goto out;
     }
 
@@ -517,7 +510,7 @@ static QObject *parse_keyword(JSONParserContext *ctxt)
     } else if (token_is_keyword(token, "null")) {
         ret = qnull();
     } else {
-        parse_error(ctxt, token, "invalid keyword `%s'", token_get_value(token));
+        parse_error(ctxt, token, "invalid keyword `%s'", token->str);
         goto out;
     }
 
@@ -530,7 +523,8 @@ out:
 
 static QObject *parse_escape(JSONParserContext *ctxt, va_list *ap)
 {
-    QObject *token = NULL, *obj;
+    QObject *obj;
+    JSONToken *token;
 
     if (ap == NULL) {
         goto out;
@@ -541,7 +535,7 @@ static QObject *parse_escape(JSONParserContext *ctxt, va_list *ap)
         goto out;
     }
 
-    if (token_get_type(token) != JSON_ESCAPE) {
+    if (token->type != JSON_ESCAPE) {
         goto out;
     }
 
@@ -574,14 +568,15 @@ out:
 
 static QObject *parse_literal(JSONParserContext *ctxt)
 {
-    QObject *token, *obj;
+    JSONToken *token;
+    QObject *obj;
 
     token = parser_context_peek_token(ctxt);
     if (token == NULL) {
         goto out;
     }
 
-    switch (token_get_type(token)) {
+    switch (token->type) {
     case JSON_STRING:
         parser_context_pop_token(ctxt);
         obj = QOBJECT(qstring_from_escaped_str(ctxt, token));
@@ -603,7 +598,7 @@ static QObject *parse_literal(JSONParserContext *ctxt)
 
         parser_context_pop_token(ctxt);
         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) {
             obj = QOBJECT(qint_from_int(value));
             break;
@@ -614,7 +609,7 @@ static QObject *parse_literal(JSONParserContext *ctxt)
         parser_context_pop_token(ctxt);
     parse_float:
         /* FIXME dependent on locale */
-        obj = QOBJECT(qfloat_from_double(strtod(token_get_value(token), NULL)));
+        obj = QOBJECT(qfloat_from_double(strtod(token->str, NULL)));
         break;
     default:
         goto out;
diff --git a/qobject/json-streamer.c b/qobject/json-streamer.c
index d2af38f..88b1e41 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"
@@ -33,7 +29,7 @@ static void json_message_free_tokens(JSONMessageParser *parser)
 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;
 
     if (type == JSON_OPERATOR) {
         switch (input->str[0]) {
@@ -54,15 +50,16 @@ static void json_message_process_token(JSONLexer *lexer, GString *input, JSONTok
         }
     }
 
-    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 + 1);
+    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;
-- 
2.5.0

  parent reply	other threads:[~2015-11-20  9:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-20  9:04 [Qemu-devel] [PATCH 0/4] qjson: save a lot of memory Paolo Bonzini
2015-11-20  9:04 ` [Qemu-devel] [PATCH 1/4] qjson: replace QString in JSONLexer with GString Paolo Bonzini
2015-11-20 18:23   ` Eric Blake
2015-11-20 20:16     ` Paolo Bonzini
2015-11-20  9:04 ` [Qemu-devel] [PATCH 2/4] qjson: do not save/restore contexts Paolo Bonzini
2015-11-20 18:28   ` Eric Blake
2015-11-20 20:17     ` Paolo Bonzini
2015-11-20  9:04 ` [Qemu-devel] [PATCH 3/4] qjson: store tokens in a GQueue Paolo Bonzini
2015-11-20 18:40   ` Eric Blake
2015-11-20  9:04 ` Paolo Bonzini [this message]
2015-11-20 18:48   ` [Qemu-devel] [PATCH 4/4] qjson: surprise, allocating 6 QObjects per token is expensive Eric Blake
2015-11-20 20:19     ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1448010269-21694-5-git-send-email-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=armbru@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).