* [Qemu-devel] [PATCH for-1.2 v2 2/3] json-parser: don't replicate tokens at each level of recursion
2012-08-15 17:56 [Qemu-devel] [PATCH for-1.2 v2 1/3] qlist: add qlist_size() Michael Roth
@ 2012-08-15 17:56 ` Michael Roth
2012-08-15 18:09 ` Eric Blake
2012-08-15 17:57 ` [Qemu-devel] [PATCH for-1.2 v2 3/3] check-qjson: add test for large JSON objects Michael Roth
1 sibling, 1 reply; 5+ messages in thread
From: Michael Roth @ 2012-08-15 17:56 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, armbru, lcapitulino
Currently, when parsing a stream of tokens we make a copy of the token
list at the beginning of each level of recursion so that we do not
modify the original list in cases where we need to fall back to an
earlier state.
In the worst case, we will only read 1 or 2 tokens off the list before
recursing again, which means an upper bound of roughly N^2 token allocations.
For a "reasonably" sized QMP request (in this a QMP representation of
cirrus_vga's device state, generated via QIDL, being passed in via
qom-set), this caused my 16GB's of memory to be exhausted before any
noticeable progress was made by the parser.
This patch works around the issue by using single copy of the token list
in the form of an indexable array so that we can save/restore state by
manipulating indices.
A subsequent commit adds a "large_dict" test case which exhibits the
same behavior as above. With this patch applied the test case successfully
completes in under a second.
Tested with valgrind, make check, and QMP.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
json-parser.c | 229 +++++++++++++++++++++++++++++++++++----------------------
1 file changed, 141 insertions(+), 88 deletions(-)
diff --git a/json-parser.c b/json-parser.c
index 849e215..65a87c7 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -27,6 +27,11 @@
typedef struct JSONParserContext
{
Error *err;
+ struct {
+ QObject **buf;
+ size_t pos;
+ size_t count;
+ } tokens;
} JSONParserContext;
#define BUG_ON(cond) assert(!(cond))
@@ -40,7 +45,7 @@ typedef struct JSONParserContext
* 4) deal with premature EOI
*/
-static QObject *parse_value(JSONParserContext *ctxt, QList **tokens, va_list *ap);
+static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
/**
* Token manipulators
@@ -270,27 +275,110 @@ out:
return NULL;
}
+static QObject *parser_context_pop_token(JSONParserContext *ctxt)
+{
+ QObject *token;
+ g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+ token = ctxt->tokens.buf[ctxt->tokens.pos];
+ ctxt->tokens.pos++;
+ return token;
+}
+
+/* Note: parser_context_{peek|pop}_token do not increment the
+ * token object's refcount. In both cases the references will continue
+ * to be * tracked and cleanup in parser_context_free()
+ */
+static QObject *parser_context_peek_token(JSONParserContext *ctxt)
+{
+ QObject *token;
+ g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+ token = ctxt->tokens.buf[ctxt->tokens.pos];
+ return token;
+}
+
+static JSONParserContext parser_context_save(JSONParserContext *ctxt)
+{
+ JSONParserContext saved_ctxt = {0};
+ saved_ctxt.tokens.pos = ctxt->tokens.pos;
+ saved_ctxt.tokens.count = ctxt->tokens.count;
+ saved_ctxt.tokens.buf = ctxt->tokens.buf;
+ return saved_ctxt;
+}
+
+static void parser_context_restore(JSONParserContext *ctxt,
+ JSONParserContext saved_ctxt)
+{
+ ctxt->tokens.pos = saved_ctxt.tokens.pos;
+ ctxt->tokens.count = saved_ctxt.tokens.count;
+ ctxt->tokens.buf = saved_ctxt.tokens.buf;
+}
+
+static void tokens_append_from_iter(QObject *obj, void *opaque)
+{
+ JSONParserContext *ctxt = opaque;
+ g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+ ctxt->tokens.buf[ctxt->tokens.pos++] = obj;
+ qobject_incref(obj);
+}
+
+static JSONParserContext *parser_context_new(QList *tokens)
+{
+ JSONParserContext *ctxt;
+ size_t count;
+
+ if (!tokens) {
+ return NULL;
+ }
+
+ count = qlist_size(tokens);
+ if (count == 0) {
+ return NULL;
+ }
+
+ ctxt = g_malloc0(sizeof(JSONParserContext));
+ ctxt->tokens.pos = 0;
+ ctxt->tokens.count = count;
+ ctxt->tokens.buf = g_malloc(count * sizeof(QObject *));
+ qlist_iter(tokens, tokens_append_from_iter, ctxt);
+ ctxt->tokens.pos = 0;
+
+ return ctxt;
+}
+
+/* to support error propagation, ctxt->err must be freed seperately */
+static void parser_context_free(JSONParserContext *ctxt)
+{
+ int i;
+ if (ctxt) {
+ for (i = 0; i < ctxt->tokens.count; i++) {
+ qobject_decref(ctxt->tokens.buf[i]);
+ }
+ g_free(ctxt->tokens.buf);
+ g_free(ctxt);
+ }
+}
+
/**
* Parsing rules
*/
-static int parse_pair(JSONParserContext *ctxt, QDict *dict, QList **tokens, va_list *ap)
+static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *ap)
{
QObject *key = NULL, *token = NULL, *value, *peek;
- QList *working = qlist_copy(*tokens);
+ JSONParserContext saved_ctxt = parser_context_save(ctxt);
- peek = qlist_peek(working);
+ peek = parser_context_peek_token(ctxt);
if (peek == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
}
- key = parse_value(ctxt, &working, ap);
+ key = parse_value(ctxt, ap);
if (!key || qobject_type(key) != QTYPE_QSTRING) {
parse_error(ctxt, peek, "key is not a string in object");
goto out;
}
- token = qlist_pop(working);
+ token = parser_context_pop_token(ctxt);
if (token == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
@@ -301,7 +389,7 @@ static int parse_pair(JSONParserContext *ctxt, QDict *dict, QList **tokens, va_l
goto out;
}
- value = parse_value(ctxt, &working, ap);
+ value = parse_value(ctxt, ap);
if (value == NULL) {
parse_error(ctxt, token, "Missing value in dict");
goto out;
@@ -309,28 +397,24 @@ static int parse_pair(JSONParserContext *ctxt, QDict *dict, QList **tokens, va_l
qdict_put_obj(dict, qstring_get_str(qobject_to_qstring(key)), value);
- qobject_decref(token);
qobject_decref(key);
- QDECREF(*tokens);
- *tokens = working;
return 0;
out:
- qobject_decref(token);
+ parser_context_restore(ctxt, saved_ctxt);
qobject_decref(key);
- QDECREF(working);
return -1;
}
-static QObject *parse_object(JSONParserContext *ctxt, QList **tokens, va_list *ap)
+static QObject *parse_object(JSONParserContext *ctxt, va_list *ap)
{
QDict *dict = NULL;
QObject *token, *peek;
- QList *working = qlist_copy(*tokens);
+ JSONParserContext saved_ctxt = parser_context_save(ctxt);
- token = qlist_pop(working);
+ token = parser_context_pop_token(ctxt);
if (token == NULL) {
goto out;
}
@@ -338,23 +422,22 @@ static QObject *parse_object(JSONParserContext *ctxt, QList **tokens, va_list *a
if (!token_is_operator(token, '{')) {
goto out;
}
- qobject_decref(token);
token = NULL;
dict = qdict_new();
- peek = qlist_peek(working);
+ peek = parser_context_peek_token(ctxt);
if (peek == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
}
if (!token_is_operator(peek, '}')) {
- if (parse_pair(ctxt, dict, &working, ap) == -1) {
+ if (parse_pair(ctxt, dict, ap) == -1) {
goto out;
}
- token = qlist_pop(working);
+ token = parser_context_pop_token(ctxt);
if (token == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
@@ -365,59 +448,52 @@ static QObject *parse_object(JSONParserContext *ctxt, QList **tokens, va_list *a
parse_error(ctxt, token, "expected separator in dict");
goto out;
}
- qobject_decref(token);
token = NULL;
- if (parse_pair(ctxt, dict, &working, ap) == -1) {
+ if (parse_pair(ctxt, dict, ap) == -1) {
goto out;
}
- token = qlist_pop(working);
+ token = parser_context_pop_token(ctxt);
if (token == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
}
}
- qobject_decref(token);
token = NULL;
} else {
- token = qlist_pop(working);
- qobject_decref(token);
+ token = parser_context_pop_token(ctxt);
token = NULL;
}
- QDECREF(*tokens);
- *tokens = working;
-
return QOBJECT(dict);
out:
- qobject_decref(token);
- QDECREF(working);
+ parser_context_restore(ctxt, saved_ctxt);
QDECREF(dict);
return NULL;
}
-static QObject *parse_array(JSONParserContext *ctxt, QList **tokens, va_list *ap)
+static QObject *parse_array(JSONParserContext *ctxt, va_list *ap)
{
QList *list = NULL;
QObject *token, *peek;
- QList *working = qlist_copy(*tokens);
+ JSONParserContext saved_ctxt = parser_context_save(ctxt);
- token = qlist_pop(working);
+ token = parser_context_pop_token(ctxt);
if (token == NULL) {
goto out;
}
if (!token_is_operator(token, '[')) {
+ token = NULL;
goto out;
}
- qobject_decref(token);
token = NULL;
list = qlist_new();
- peek = qlist_peek(working);
+ peek = parser_context_peek_token(ctxt);
if (peek == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
@@ -426,7 +502,7 @@ static QObject *parse_array(JSONParserContext *ctxt, QList **tokens, va_list *ap
if (!token_is_operator(peek, ']')) {
QObject *obj;
- obj = parse_value(ctxt, &working, ap);
+ obj = parse_value(ctxt, ap);
if (obj == NULL) {
parse_error(ctxt, token, "expecting value");
goto out;
@@ -434,7 +510,7 @@ static QObject *parse_array(JSONParserContext *ctxt, QList **tokens, va_list *ap
qlist_append_obj(list, obj);
- token = qlist_pop(working);
+ token = parser_context_pop_token(ctxt);
if (token == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
@@ -446,10 +522,9 @@ static QObject *parse_array(JSONParserContext *ctxt, QList **tokens, va_list *ap
goto out;
}
- qobject_decref(token);
token = NULL;
- obj = parse_value(ctxt, &working, ap);
+ obj = parse_value(ctxt, ap);
if (obj == NULL) {
parse_error(ctxt, token, "expecting value");
goto out;
@@ -457,39 +532,33 @@ static QObject *parse_array(JSONParserContext *ctxt, QList **tokens, va_list *ap
qlist_append_obj(list, obj);
- token = qlist_pop(working);
+ token = parser_context_pop_token(ctxt);
if (token == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
}
}
- qobject_decref(token);
token = NULL;
} else {
- token = qlist_pop(working);
- qobject_decref(token);
+ token = parser_context_pop_token(ctxt);
token = NULL;
}
- QDECREF(*tokens);
- *tokens = working;
-
return QOBJECT(list);
out:
- qobject_decref(token);
- QDECREF(working);
+ parser_context_restore(ctxt, saved_ctxt);
QDECREF(list);
return NULL;
}
-static QObject *parse_keyword(JSONParserContext *ctxt, QList **tokens)
+static QObject *parse_keyword(JSONParserContext *ctxt)
{
QObject *token, *ret;
- QList *working = qlist_copy(*tokens);
+ JSONParserContext saved_ctxt = parser_context_save(ctxt);
- token = qlist_pop(working);
+ token = parser_context_pop_token(ctxt);
if (token == NULL) {
goto out;
}
@@ -507,29 +576,24 @@ static QObject *parse_keyword(JSONParserContext *ctxt, QList **tokens)
goto out;
}
- qobject_decref(token);
- QDECREF(*tokens);
- *tokens = working;
-
return ret;
out:
- qobject_decref(token);
- QDECREF(working);
+ parser_context_restore(ctxt, saved_ctxt);
return NULL;
}
-static QObject *parse_escape(JSONParserContext *ctxt, QList **tokens, va_list *ap)
+static QObject *parse_escape(JSONParserContext *ctxt, va_list *ap)
{
QObject *token = NULL, *obj;
- QList *working = qlist_copy(*tokens);
+ JSONParserContext saved_ctxt = parser_context_save(ctxt);
if (ap == NULL) {
goto out;
}
- token = qlist_pop(working);
+ token = parser_context_pop_token(ctxt);
if (token == NULL) {
goto out;
}
@@ -553,25 +617,20 @@ static QObject *parse_escape(JSONParserContext *ctxt, QList **tokens, va_list *a
goto out;
}
- qobject_decref(token);
- QDECREF(*tokens);
- *tokens = working;
-
return obj;
out:
- qobject_decref(token);
- QDECREF(working);
+ parser_context_restore(ctxt, saved_ctxt);
return NULL;
}
-static QObject *parse_literal(JSONParserContext *ctxt, QList **tokens)
+static QObject *parse_literal(JSONParserContext *ctxt)
{
QObject *token, *obj;
- QList *working = qlist_copy(*tokens);
+ JSONParserContext saved_ctxt = parser_context_save(ctxt);
- token = qlist_pop(working);
+ token = parser_context_pop_token(ctxt);
if (token == NULL) {
goto out;
}
@@ -591,35 +650,30 @@ static QObject *parse_literal(JSONParserContext *ctxt, QList **tokens)
goto out;
}
- qobject_decref(token);
- QDECREF(*tokens);
- *tokens = working;
-
return obj;
out:
- qobject_decref(token);
- QDECREF(working);
+ parser_context_restore(ctxt, saved_ctxt);
return NULL;
}
-static QObject *parse_value(JSONParserContext *ctxt, QList **tokens, va_list *ap)
+static QObject *parse_value(JSONParserContext *ctxt, va_list *ap)
{
QObject *obj;
- obj = parse_object(ctxt, tokens, ap);
+ obj = parse_object(ctxt, ap);
if (obj == NULL) {
- obj = parse_array(ctxt, tokens, ap);
+ obj = parse_array(ctxt, ap);
}
if (obj == NULL) {
- obj = parse_escape(ctxt, tokens, ap);
+ obj = parse_escape(ctxt, ap);
}
if (obj == NULL) {
- obj = parse_keyword(ctxt, tokens);
+ obj = parse_keyword(ctxt);
}
if (obj == NULL) {
- obj = parse_literal(ctxt, tokens);
+ obj = parse_literal(ctxt);
}
return obj;
@@ -632,19 +686,18 @@ QObject *json_parser_parse(QList *tokens, va_list *ap)
QObject *json_parser_parse_err(QList *tokens, va_list *ap, Error **errp)
{
- JSONParserContext ctxt = {};
- QList *working;
+ JSONParserContext *ctxt = parser_context_new(tokens);
QObject *result;
- if (!tokens) {
+ if (!ctxt) {
return NULL;
}
- working = qlist_copy(tokens);
- result = parse_value(&ctxt, &working, ap);
- QDECREF(working);
+ result = parse_value(ctxt, ap);
+
+ error_propagate(errp, ctxt->err);
- error_propagate(errp, ctxt.err);
+ parser_context_free(ctxt);
return result;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PATCH for-1.2 v2 3/3] check-qjson: add test for large JSON objects
2012-08-15 17:56 [Qemu-devel] [PATCH for-1.2 v2 1/3] qlist: add qlist_size() Michael Roth
2012-08-15 17:56 ` [Qemu-devel] [PATCH for-1.2 v2 2/3] json-parser: don't replicate tokens at each level of recursion Michael Roth
@ 2012-08-15 17:57 ` Michael Roth
1 sibling, 0 replies; 5+ messages in thread
From: Michael Roth @ 2012-08-15 17:57 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, armbru, lcapitulino
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
tests/check-qjson.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 526e25e..ef9b529 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -690,6 +690,58 @@ static void unterminated_literal(void)
g_assert(obj == NULL);
}
+/*
+ * this generates json of the form:
+ * a(0,m) = [0, 1, ..., m-1]
+ * a(n,m) = {
+ * 'key0': a(0,m),
+ * 'key1': a(1,m),
+ * ...
+ * 'key(n-1)': a(n-1,m)
+ * }
+ */
+static void gen_test_json(GString *gstr, int nest_level_max,
+ int elem_count)
+{
+ int i;
+
+ g_assert(gstr);
+ if (nest_level_max == 0) {
+ g_string_append(gstr, "[");
+ for (i = 0; i < elem_count; i++) {
+ g_string_append_printf(gstr, "%d", i);
+ if (i < elem_count - 1) {
+ g_string_append_printf(gstr, ", ");
+ }
+ }
+ g_string_append(gstr, "]");
+ return;
+ }
+
+ g_string_append(gstr, "{");
+ for (i = 0; i < nest_level_max; i++) {
+ g_string_append_printf(gstr, "'key%d': ", i);
+ gen_test_json(gstr, i, elem_count);
+ if (i < nest_level_max - 1) {
+ g_string_append(gstr, ",");
+ }
+ }
+ g_string_append(gstr, "}");
+}
+
+static void large_dict(void)
+{
+ GString *gstr = g_string_new("");
+ QObject *obj;
+
+ gen_test_json(gstr, 10, 100);
+ obj = qobject_from_json(gstr->str);
+ g_assert(obj != NULL);
+
+ qobject_decref(obj);
+ g_string_free(gstr, true);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
@@ -706,6 +758,7 @@ int main(int argc, char **argv)
g_test_add_func("/literals/keyword", keyword_literal);
g_test_add_func("/dicts/simple_dict", simple_dict);
+ g_test_add_func("/dicts/large_dict", large_dict);
g_test_add_func("/lists/simple_list", simple_list);
g_test_add_func("/whitespace/simple_whitespace", simple_whitespace);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread