Skip to content

Commit

Permalink
qjson: surprise, allocating 6 QObjects per token is expensive
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@bonzini
bonzini authored and Markus Armbruster committed Nov 26, 2015
1 parent 95385fe commit 9bada89
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 78 deletions.
7 changes: 7 additions & 0 deletions include/qapi/qmp/json-streamer.h
View file
Expand Up @@ -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);
Expand Down
115 changes: 48 additions & 67 deletions qobject/json-parser.c
View file
Expand Up @@ -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;

Expand All @@ -43,28 +44,11 @@ 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];
Expand Down Expand Up @@ -142,9 +126,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;

Expand Down Expand Up @@ -240,19 +225,19 @@ static QString *qstring_from_escaped_str(JSONParserContext *ctxt, QObject *token
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);
Expand All @@ -279,7 +264,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);
}
Expand All @@ -290,7 +275,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) {
Expand All @@ -310,7 +296,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;
}
Expand All @@ -336,10 +322,10 @@ static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *ap)
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();

Expand All @@ -349,7 +335,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;
}
Expand All @@ -360,8 +346,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;
}
Expand Down Expand Up @@ -390,10 +376,10 @@ static QObject *parse_object(JSONParserContext *ctxt, va_list *ap)
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();

Expand All @@ -403,7 +389,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);
Expand All @@ -420,8 +406,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;
}
Expand Down Expand Up @@ -453,64 +439,60 @@ static QObject *parse_array(JSONParserContext *ctxt, va_list *ap)

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_bool(true));
} else if (!strcmp(val, "false")) {
} else if (!strcmp(token->str, "false")) {
return QOBJECT(qbool_from_bool(false));
} else if (!strcmp(val, "null")) {
} else if (!strcmp(token->str, "null")) {
return qnull();
}
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_bool(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;
}

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: {
Expand All @@ -529,32 +511,31 @@ 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));
}
/* fall through to JSON_FLOAT */
}
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();
}
}

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

token = parser_context_peek_token(ctxt);
if (token == NULL) {
parse_error(ctxt, NULL, "premature EOI");
return NULL;
}

switch (token_get_type(token)) {
switch (token->type) {
case JSON_LCURLY:
return parse_object(ctxt, ap);
case JSON_LSQUARE:
Expand Down
19 changes: 8 additions & 11 deletions qobject/json-streamer.c
View file
Expand Up @@ -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"
Expand All @@ -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:
Expand All @@ -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;
Expand Down

0 comments on commit 9bada89

Please sign in to comment.