From 1d4fffb799981d625f7e47f810d76da6dc37888e Mon Sep 17 00:00:00 2001 From: John Keiser Date: Thu, 4 Jun 2020 19:15:35 -0700 Subject: [PATCH 01/10] Fix fallback implementation --- src/generic/stage1/json_structural_indexer.h | 89 -------------------- 1 file changed, 89 deletions(-) diff --git a/src/generic/stage1/json_structural_indexer.h b/src/generic/stage1/json_structural_indexer.h index 4f43d24a4..6f80123e7 100644 --- a/src/generic/stage1/json_structural_indexer.h +++ b/src/generic/stage1/json_structural_indexer.h @@ -73,8 +73,6 @@ private: really_inline void step(const uint8_t *block, buf_block_reader &reader) noexcept; really_inline void next(simd::simd8x64 in, json_block block, size_t idx); really_inline error_code finish(dom_parser_implementation &parser, size_t idx, size_t len, bool partial); - static really_inline uint32_t find_next_document_index(dom_parser_implementation &parser); - static really_inline size_t trim_partial_utf8(const uint8_t *buf, size_t len); json_scanner scanner{}; utf8_checker checker{}; @@ -197,91 +195,4 @@ really_inline error_code json_structural_indexer::finish(dom_parser_implementati return checker.errors(); } -/** - * This algorithm is used to quickly identify the last structural position that - * makes up a complete document. - * - * It does this by going backwards and finding the last *document boundary* (a - * place where one value follows another without a comma between them). If the - * last document (the characters after the boundary) has an equal number of - * start and end brackets, it is considered complete. - * - * Simply put, we iterate over the structural characters, starting from - * the end. We consider that we found the end of a JSON document when the - * first element of the pair is NOT one of these characters: '{' '[' ';' ',' - * and when the second element is NOT one of these characters: '}' '}' ';' ','. - * - * This simple comparison works most of the time, but it does not cover cases - * where the batch's structural indexes contain a perfect amount of documents. - * In such a case, we do not have access to the structural index which follows - * the last document, therefore, we do not have access to the second element in - * the pair, and means that we cannot identify the last document. To fix this - * issue, we keep a count of the open and closed curly/square braces we found - * while searching for the pair. When we find a pair AND the count of open and - * closed curly/square braces is the same, we know that we just passed a - * complete - * document, therefore the last json buffer location is the end of the batch - */ -really_inline uint32_t json_structural_indexer::find_next_document_index(dom_parser_implementation &parser) { - // TODO don't count separately, just figure out depth - auto arr_cnt = 0; - auto obj_cnt = 0; - for (auto i = parser.n_structural_indexes - 1; i > 0; i--) { - auto idxb = parser.structural_indexes[i]; - switch (parser.buf[idxb]) { - case ':': - case ',': - continue; - case '}': - obj_cnt--; - continue; - case ']': - arr_cnt--; - continue; - case '{': - obj_cnt++; - break; - case '[': - arr_cnt++; - break; - } - auto idxa = parser.structural_indexes[i - 1]; - switch (parser.buf[idxa]) { - case '{': - case '[': - case ':': - case ',': - continue; - } - // Last document is complete, so the next document will appear after! - if (!arr_cnt && !obj_cnt) { - return parser.n_structural_indexes; - } - // Last document is incomplete; mark the document at i + 1 as the next one - return i; - } - return 0; -} - -// Skip the last character if it is partial -really_inline size_t json_structural_indexer::trim_partial_utf8(const uint8_t *buf, size_t len) { - if (unlikely(len < 3)) { - switch (len) { - case 2: - if (buf[len-1] >= 0b11000000) { return len-1; } // 2-, 3- and 4-byte characters with only 1 byte left - if (buf[len-2] >= 0b11100000) { return len-2; } // 3- and 4-byte characters with only 2 bytes left - return len; - case 1: - if (buf[len-1] >= 0b11000000) { return len-1; } // 2-, 3- and 4-byte characters with only 1 byte left - return len; - case 0: - return len; - } - } - if (buf[len-1] >= 0b11000000) { return len-1; } // 2-, 3- and 4-byte characters with only 1 byte left - if (buf[len-2] >= 0b11100000) { return len-2; } // 3- and 4-byte characters with only 1 byte left - if (buf[len-3] >= 0b11110000) { return len-3; } // 4-byte characters with only 3 bytes left - return len; -} - } // namespace stage1 From a1aea4588fd5ed4bb864bcb960350e1a5f723bbe Mon Sep 17 00:00:00 2001 From: John Keiser Date: Tue, 2 Jun 2020 20:21:46 -0700 Subject: [PATCH 02/10] Move document stream state to implementation --- .../stage2/streaming_structural_parser.h | 168 ++++++++++++++++++ src/generic/stage2/structural_parser.h | 6 +- 2 files changed, 172 insertions(+), 2 deletions(-) create mode 100755 src/generic/stage2/streaming_structural_parser.h diff --git a/src/generic/stage2/streaming_structural_parser.h b/src/generic/stage2/streaming_structural_parser.h new file mode 100755 index 000000000..8e63d0287 --- /dev/null +++ b/src/generic/stage2/streaming_structural_parser.h @@ -0,0 +1,168 @@ +namespace stage2 { + +struct streaming_structural_parser: structural_parser { + really_inline streaming_structural_parser(dom_parser_implementation &_parser) : structural_parser(_parser, _parser.next_structural_index) {} + + // override to add streaming + WARN_UNUSED really_inline error_code start(ret_address_t finish_parser) { + // If there are no structurals left, return EMPTY + if (structurals.at_end(parser.n_structural_indexes)) { + return parser.error = EMPTY; + } + + log_start(); + init(); + + // Capacity ain't no thang for streaming, so we don't check it. + // Advance to the first character as soon as possible + advance_char(); + // Push the root scope (there is always at least one scope) + if (start_document(finish_parser)) { + return parser.error = DEPTH_ERROR; + } + return SUCCESS; + } + + // override to add streaming + WARN_UNUSED really_inline error_code finish() { + if ( structurals.past_end(parser.n_structural_indexes) ) { + log_error("IMPOSSIBLE: past the end of the JSON!"); + return parser.error = TAPE_ERROR; + } + end_document(); + parser.next_structural_index = uint32_t(structurals.next_structural_index()); + if (depth != 0) { + log_error("Unclosed objects or arrays!"); + return parser.error = TAPE_ERROR; + } + if (parser.containing_scope[depth].tape_index != 0) { + log_error("IMPOSSIBLE: root scope tape index did not start at 0!"); + return parser.error = TAPE_ERROR; + } + return SUCCESS; + } +}; + +} // namespace stage2 + +/************ + * The JSON is parsed to a tape, see the accompanying tape.md file + * for documentation. + ***********/ +WARN_UNUSED error_code dom_parser_implementation::stage2_next(dom::document &_doc) noexcept { + this->doc = &_doc; + static constexpr stage2::unified_machine_addresses addresses = INIT_ADDRESSES(); + stage2::streaming_structural_parser parser(*this); + error_code result = parser.start(addresses.finish); + if (result) { return result; } + // + // Read first value + // + switch (parser.structurals.current_char()) { + case '{': + FAIL_IF( parser.start_object(addresses.finish) ); + goto object_begin; + case '[': + FAIL_IF( parser.start_array(addresses.finish) ); + goto array_begin; + case '"': + FAIL_IF( parser.parse_string() ); + goto finish; + case 't': case 'f': case 'n': + FAIL_IF( parser.parse_single_atom() ); + goto finish; + case '0': case '1': case '2': case '3': case '4': + case '5': case '6': case '7': case '8': case '9': + FAIL_IF( + parser.structurals.with_space_terminated_copy([&](const uint8_t *copy, size_t idx) { + return parser.parse_number(©[idx], false); + }) + ); + goto finish; + case '-': + FAIL_IF( + parser.structurals.with_space_terminated_copy([&](const uint8_t *copy, size_t idx) { + return parser.parse_number(©[idx], true); + }) + ); + goto finish; + default: + parser.log_error("Document starts with a non-value character"); + goto error; + } + +// +// Object parser parsers +// +object_begin: + switch (parser.advance_char()) { + case '"': { + FAIL_IF( parser.parse_string(true) ); + goto object_key_parser; + } + case '}': + parser.end_object(); + goto scope_end; + default: + parser.log_error("Object does not start with a key"); + goto error; + } + +object_key_parser: + if (parser.advance_char() != ':' ) { parser.log_error("Missing colon after key in object"); goto error; } + parser.increment_count(); + parser.advance_char(); + GOTO( parser.parse_value(addresses, addresses.object_continue) ); + +object_continue: + switch (parser.advance_char()) { + case ',': + if (parser.advance_char() != '"' ) { parser.log_error("Key string missing at beginning of field in object"); goto error; } + FAIL_IF( parser.parse_string(true) ); + goto object_key_parser; + case '}': + parser.end_object(); + goto scope_end; + default: + parser.log_error("No comma between object fields"); + goto error; + } + +scope_end: + CONTINUE( parser.parser.ret_address[parser.depth] ); + +// +// Array parser parsers +// +array_begin: + if (parser.advance_char() == ']') { + parser.end_array(); + goto scope_end; + } + parser.increment_count(); + +main_array_switch: + /* we call update char on all paths in, so we can peek at parser.c on the + * on paths that can accept a close square brace (post-, and at start) */ + GOTO( parser.parse_value(addresses, addresses.array_continue) ); + +array_continue: + switch (parser.advance_char()) { + case ',': + parser.increment_count(); + parser.advance_char(); + goto main_array_switch; + case ']': + parser.end_array(); + goto scope_end; + default: + parser.log_error("Missing comma between array values"); + goto error; + } + +finish: + return parser.finish(); + +error: + return parser.error(); +} diff --git a/src/generic/stage2/structural_parser.h b/src/generic/stage2/structural_parser.h index 4c112b67d..57adbb1b3 100644 --- a/src/generic/stage2/structural_parser.h +++ b/src/generic/stage2/structural_parser.h @@ -394,8 +394,10 @@ WARN_UNUSED static error_code parse_structurals(dom_parser_implementation &dom_p FAIL_IF( parser.start_array(addresses.finish) ); // Make sure the outer array is closed before continuing; otherwise, there are ways we could get // into memory corruption. See https://github.com/simdjson/simdjson/issues/906 - if (parser.structurals.buf[parser.structurals.structural_indexes[dom_parser.n_structural_indexes - 1]] != ']') { - goto error; + if (!STREAMING) { + if (parser.structurals.buf[parser.structurals.structural_indexes[dom_parser.n_structural_indexes - 1]] != ']') { + goto error; + } } goto array_begin; case '"': From 3636aa5522bf916a0fe153d5cbc5049f62c0d139 Mon Sep 17 00:00:00 2001 From: John Keiser Date: Sat, 6 Jun 2020 12:04:29 -0700 Subject: [PATCH 03/10] Extend structural_parser from structural_iterator --- src/generic/stage2/structural_parser.h | 57 ++++++++++++-------------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/src/generic/stage2/structural_parser.h b/src/generic/stage2/structural_parser.h index 57adbb1b3..853f18afa 100644 --- a/src/generic/stage2/structural_parser.h +++ b/src/generic/stage2/structural_parser.h @@ -69,16 +69,15 @@ struct number_writer { } }; // struct number_writer -struct structural_parser { - structural_iterator structurals; +struct structural_parser : structural_iterator { dom_parser_implementation &parser; /** Next write location in the string buf for stage 2 parsing */ uint8_t *current_string_buf_loc{}; uint32_t depth; // For non-streaming, to pass an explicit 0 as next_structural, which enables optimizations - really_inline structural_parser(dom_parser_implementation &_parser, uint32_t next_structural) - : structurals(_parser.buf, _parser.len, _parser.structural_indexes.get(), next_structural), + really_inline structural_parser(dom_parser_implementation &_parser, uint32_t _next_structural) + : structural_iterator(_parser.buf, _parser.len, _parser.structural_indexes.get(), _next_structural), parser{_parser}, depth{0} { } @@ -174,7 +173,7 @@ struct structural_parser { WARN_UNUSED really_inline bool parse_string(bool key = false) { log_value(key ? "key" : "string"); uint8_t *dst = on_start_string(); - dst = stringparsing::parse_string(structurals.current(), dst); + dst = stringparsing::parse_string(current(), dst); if (dst == nullptr) { log_error("Invalid escape in string"); return true; @@ -191,24 +190,24 @@ struct structural_parser { return !succeeded; } WARN_UNUSED really_inline bool parse_number(bool found_minus) { - return parse_number(structurals.current(), found_minus); + return parse_number(current(), found_minus); } WARN_UNUSED really_inline bool parse_atom() { - switch (structurals.current_char()) { + switch (current_char()) { case 't': log_value("true"); - if (!atomparsing::is_valid_true_atom(structurals.current())) { return true; } + if (!atomparsing::is_valid_true_atom(current())) { return true; } append_tape(0, internal::tape_type::TRUE_VALUE); break; case 'f': log_value("false"); - if (!atomparsing::is_valid_false_atom(structurals.current())) { return true; } + if (!atomparsing::is_valid_false_atom(current())) { return true; } append_tape(0, internal::tape_type::FALSE_VALUE); break; case 'n': log_value("null"); - if (!atomparsing::is_valid_null_atom(structurals.current())) { return true; } + if (!atomparsing::is_valid_null_atom(current())) { return true; } append_tape(0, internal::tape_type::NULL_VALUE); break; default: @@ -219,20 +218,20 @@ struct structural_parser { } WARN_UNUSED really_inline bool parse_single_atom() { - switch (structurals.current_char()) { + switch (current_char()) { case 't': log_value("true"); - if (!atomparsing::is_valid_true_atom(structurals.current(), structurals.remaining_len())) { return true; } + if (!atomparsing::is_valid_true_atom(current(), remaining_len())) { return true; } append_tape(0, internal::tape_type::TRUE_VALUE); break; case 'f': log_value("false"); - if (!atomparsing::is_valid_false_atom(structurals.current(), structurals.remaining_len())) { return true; } + if (!atomparsing::is_valid_false_atom(current(), remaining_len())) { return true; } append_tape(0, internal::tape_type::FALSE_VALUE); break; case 'n': log_value("null"); - if (!atomparsing::is_valid_null_atom(structurals.current(), structurals.remaining_len())) { return true; } + if (!atomparsing::is_valid_null_atom(current(), remaining_len())) { return true; } append_tape(0, internal::tape_type::NULL_VALUE); break; default: @@ -243,7 +242,7 @@ struct structural_parser { } WARN_UNUSED really_inline ret_address_t parse_value(const unified_machine_addresses &addresses, ret_address_t continue_state) { - switch (structurals.current_char()) { + switch (current_char()) { case '"': FAIL_IF( parse_string() ); return continue_state; @@ -271,7 +270,7 @@ struct structural_parser { WARN_UNUSED really_inline error_code finish() { end_document(); - parser.next_structural_index = uint32_t(structurals.next_structural_index()); + parser.next_structural_index = uint32_t(next_structural_index()); if (depth != 0) { log_error("Unclosed objects or arrays!"); @@ -295,7 +294,7 @@ struct structural_parser { if (depth >= parser.max_depth()) { return parser.error = DEPTH_ERROR; } - switch (structurals.current_char()) { + switch (current_char()) { case '"': return parser.error = STRING_ERROR; case '0': @@ -329,14 +328,14 @@ struct structural_parser { WARN_UNUSED really_inline error_code start(ret_address_t finish_state) { // If there are no structurals left, return EMPTY - if (structurals.at_end(parser.n_structural_indexes)) { + if (at_end(parser.n_structural_indexes)) { return parser.error = EMPTY; } log_start(); init(); // Advance to the first character as soon as possible - structurals.advance_char(); + advance_char(); // Push the root scope (there is always at least one scope) if (start_document(finish_state)) { return parser.error = DEPTH_ERROR; @@ -344,12 +343,8 @@ struct structural_parser { return SUCCESS; } - really_inline char advance_char() { - return structurals.advance_char(); - } - really_inline void log_value(const char *type) { - logger::log_line(structurals, "", type, ""); + logger::log_line(*this, "", type, ""); } static really_inline void log_start() { @@ -357,17 +352,17 @@ struct structural_parser { } really_inline void log_start_value(const char *type) { - logger::log_line(structurals, "+", type, ""); + logger::log_line(*this, "+", type, ""); if (logger::LOG_ENABLED) { logger::log_depth++; } } really_inline void log_end_value(const char *type) { if (logger::LOG_ENABLED) { logger::log_depth--; } - logger::log_line(structurals, "-", type, ""); + logger::log_line(*this, "-", type, ""); } really_inline void log_error(const char *error) { - logger::log_line(structurals, "", "ERROR", error); + logger::log_line(*this, "", "ERROR", error); } }; // struct structural_parser @@ -386,7 +381,7 @@ WARN_UNUSED static error_code parse_structurals(dom_parser_implementation &dom_p // // Read first value // - switch (parser.structurals.current_char()) { + switch (parser.current_char()) { case '{': FAIL_IF( parser.start_object(addresses.finish) ); goto object_begin; @@ -395,7 +390,7 @@ WARN_UNUSED static error_code parse_structurals(dom_parser_implementation &dom_p // Make sure the outer array is closed before continuing; otherwise, there are ways we could get // into memory corruption. See https://github.com/simdjson/simdjson/issues/906 if (!STREAMING) { - if (parser.structurals.buf[parser.structurals.structural_indexes[dom_parser.n_structural_indexes - 1]] != ']') { + if (parser.buf[parser.structural_indexes[dom_parser.n_structural_indexes - 1]] != ']') { goto error; } } @@ -409,14 +404,14 @@ WARN_UNUSED static error_code parse_structurals(dom_parser_implementation &dom_p case '0': case '1': case '2': case '3': case '4': case '5': case '6': case '7': case '8': case '9': FAIL_IF( - parser.structurals.with_space_terminated_copy([&](const uint8_t *copy, size_t idx) { + parser.with_space_terminated_copy([&](const uint8_t *copy, size_t idx) { return parser.parse_number(©[idx], false); }) ); goto finish; case '-': FAIL_IF( - parser.structurals.with_space_terminated_copy([&](const uint8_t *copy, size_t idx) { + parser.with_space_terminated_copy([&](const uint8_t *copy, size_t idx) { return parser.parse_number(©[idx], true); }) ); From 48062380fa356cda44a49a87b060f0a91d361944 Mon Sep 17 00:00:00 2001 From: John Keiser Date: Sat, 6 Jun 2020 12:09:06 -0700 Subject: [PATCH 04/10] Move parser to structural_iterator --- src/generic/stage2/structural_iterator.h | 28 +++++++++++++----------- src/generic/stage2/structural_parser.h | 4 +--- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/generic/stage2/structural_iterator.h b/src/generic/stage2/structural_iterator.h index 92a990b21..50ee3edec 100644 --- a/src/generic/stage2/structural_iterator.h +++ b/src/generic/stage2/structural_iterator.h @@ -2,12 +2,21 @@ namespace stage2 { class structural_iterator { public: - really_inline structural_iterator(const uint8_t* _buf, size_t _len, const uint32_t *_structural_indexes, size_t next_structural_index) - : buf{_buf}, - len{_len}, - structural_indexes{_structural_indexes}, - next_structural{next_structural_index} - {} + const uint8_t* const buf; + const size_t len; + const uint32_t* const structural_indexes; + size_t next_structural; // next structural index + size_t idx{0}; // location of the structural character in the input (buf) + uint8_t c{0}; // used to track the (structural) character we are looking at + dom_parser_implementation &parser; + + really_inline structural_iterator(dom_parser_implementation &_parser, size_t _next_structural) + : buf{_parser.buf}, + len{_parser.len}, + structural_indexes{_parser.structural_indexes.get()}, + next_structural{_next_structural}, + parser{_parser} { + } really_inline char advance_char() { idx = structural_indexes[next_structural]; next_structural++; @@ -63,13 +72,6 @@ public: really_inline size_t next_structural_index() { return next_structural; } - - const uint8_t* const buf; - const size_t len; - const uint32_t* const structural_indexes; - size_t next_structural; // next structural index - size_t idx{0}; // location of the structural character in the input (buf) - uint8_t c{0}; // used to track the (structural) character we are looking at }; } // namespace stage2 diff --git a/src/generic/stage2/structural_parser.h b/src/generic/stage2/structural_parser.h index 853f18afa..0e3ea6353 100644 --- a/src/generic/stage2/structural_parser.h +++ b/src/generic/stage2/structural_parser.h @@ -70,15 +70,13 @@ struct number_writer { }; // struct number_writer struct structural_parser : structural_iterator { - dom_parser_implementation &parser; /** Next write location in the string buf for stage 2 parsing */ uint8_t *current_string_buf_loc{}; uint32_t depth; // For non-streaming, to pass an explicit 0 as next_structural, which enables optimizations really_inline structural_parser(dom_parser_implementation &_parser, uint32_t _next_structural) - : structural_iterator(_parser.buf, _parser.len, _parser.structural_indexes.get(), _next_structural), - parser{_parser}, + : structural_iterator(_parser, _next_structural), depth{0} { } From 8793dd3ceb7e7830084fc7d3caf4ad6a3a66d742 Mon Sep 17 00:00:00 2001 From: John Keiser Date: Sat, 6 Jun 2020 12:12:18 -0700 Subject: [PATCH 05/10] Don't store len locally --- src/generic/stage2/structural_iterator.h | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/generic/stage2/structural_iterator.h b/src/generic/stage2/structural_iterator.h index 50ee3edec..a7a5199ff 100644 --- a/src/generic/stage2/structural_iterator.h +++ b/src/generic/stage2/structural_iterator.h @@ -3,7 +3,6 @@ namespace stage2 { class structural_iterator { public: const uint8_t* const buf; - const size_t len; const uint32_t* const structural_indexes; size_t next_structural; // next structural index size_t idx{0}; // location of the structural character in the input (buf) @@ -12,7 +11,6 @@ public: really_inline structural_iterator(dom_parser_implementation &_parser, size_t _next_structural) : buf{_parser.buf}, - len{_parser.len}, structural_indexes{_parser.structural_indexes.get()}, next_structural{_next_structural}, parser{_parser} { @@ -33,7 +31,7 @@ public: return &buf[idx]; } really_inline size_t remaining_len() { - return len - idx; + return parser.len - idx; } template really_inline bool with_space_terminated_copy(const F& f) { @@ -50,12 +48,12 @@ public: * practice unless you are in the strange scenario where you have many JSON * documents made of single atoms. */ - char *copy = static_cast(malloc(len + SIMDJSON_PADDING)); + char *copy = static_cast(malloc(parser.len + SIMDJSON_PADDING)); if (copy == nullptr) { return true; } - memcpy(copy, buf, len); - memset(copy + len, ' ', SIMDJSON_PADDING); + memcpy(copy, buf, parser.len); + memset(copy + parser.len, ' ', SIMDJSON_PADDING); bool result = f(reinterpret_cast(copy), idx); free(copy); return result; From 59d9bc9e4895faa27e7b4300082bd70aebea8880 Mon Sep 17 00:00:00 2001 From: John Keiser Date: Sat, 6 Jun 2020 12:17:24 -0700 Subject: [PATCH 06/10] Store the pointer to the next structural instead of base structural_indexes and an index --- src/generic/stage2/logger.h | 2 +- src/generic/stage2/structural_iterator.h | 21 ++++++++------------- src/generic/stage2/structural_parser.h | 4 ++-- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/generic/stage2/logger.h b/src/generic/stage2/logger.h index 7d6fb97a0..2b36ef78e 100644 --- a/src/generic/stage2/logger.h +++ b/src/generic/stage2/logger.h @@ -62,7 +62,7 @@ namespace logger { } printf("| %c ", printable_char(structurals.at_beginning() ? ' ' : structurals.current_char())); printf("| %c ", printable_char(structurals.peek_char())); - printf("| %5u ", structurals.structural_indexes[structurals.next_structural]); + printf("| %5u ", structurals.parser.structural_indexes[*structurals.next_structural]); printf("| %-*s ", LOG_DETAIL_LEN, detail); printf("| %*zu ", LOG_INDEX_LEN, structurals.idx); printf("|\n"); diff --git a/src/generic/stage2/structural_iterator.h b/src/generic/stage2/structural_iterator.h index a7a5199ff..a5405e67e 100644 --- a/src/generic/stage2/structural_iterator.h +++ b/src/generic/stage2/structural_iterator.h @@ -3,20 +3,18 @@ namespace stage2 { class structural_iterator { public: const uint8_t* const buf; - const uint32_t* const structural_indexes; - size_t next_structural; // next structural index + uint32_t *next_structural; size_t idx{0}; // location of the structural character in the input (buf) uint8_t c{0}; // used to track the (structural) character we are looking at dom_parser_implementation &parser; - really_inline structural_iterator(dom_parser_implementation &_parser, size_t _next_structural) + really_inline structural_iterator(dom_parser_implementation &_parser, size_t next_structural_index) : buf{_parser.buf}, - structural_indexes{_parser.structural_indexes.get()}, - next_structural{_next_structural}, + next_structural{&_parser.structural_indexes[next_structural_index]}, parser{_parser} { } really_inline char advance_char() { - idx = structural_indexes[next_structural]; + idx = *next_structural; next_structural++; c = *current(); return c; @@ -25,7 +23,7 @@ public: return c; } really_inline char peek_char() { - return buf[structural_indexes[next_structural]]; + return buf[*next_structural]; } really_inline const uint8_t* current() { return &buf[idx]; @@ -59,16 +57,13 @@ public: return result; } really_inline bool past_end(uint32_t n_structural_indexes) { - return next_structural > n_structural_indexes; + return next_structural > &parser.structural_indexes[n_structural_indexes]; } really_inline bool at_end(uint32_t n_structural_indexes) { - return next_structural == n_structural_indexes; + return next_structural == &parser.structural_indexes[n_structural_indexes]; } really_inline bool at_beginning() { - return next_structural == 0; - } - really_inline size_t next_structural_index() { - return next_structural; + return next_structural == &parser.structural_indexes[0]; } }; diff --git a/src/generic/stage2/structural_parser.h b/src/generic/stage2/structural_parser.h index 0e3ea6353..d22bb2a84 100644 --- a/src/generic/stage2/structural_parser.h +++ b/src/generic/stage2/structural_parser.h @@ -268,7 +268,7 @@ struct structural_parser : structural_iterator { WARN_UNUSED really_inline error_code finish() { end_document(); - parser.next_structural_index = uint32_t(next_structural_index()); + parser.next_structural_index = uint32_t(next_structural - &parser.structural_indexes[0]); if (depth != 0) { log_error("Unclosed objects or arrays!"); @@ -388,7 +388,7 @@ WARN_UNUSED static error_code parse_structurals(dom_parser_implementation &dom_p // Make sure the outer array is closed before continuing; otherwise, there are ways we could get // into memory corruption. See https://github.com/simdjson/simdjson/issues/906 if (!STREAMING) { - if (parser.buf[parser.structural_indexes[dom_parser.n_structural_indexes - 1]] != ']') { + if (parser.buf[dom_parser.structural_indexes[dom_parser.n_structural_indexes - 1]] != ']') { goto error; } } From 8a8792d47fec3d36bcf94f8dbc24b8225dab967a Mon Sep 17 00:00:00 2001 From: John Keiser Date: Sat, 6 Jun 2020 12:34:52 -0700 Subject: [PATCH 07/10] Remove most uses of current_char() --- src/generic/stage2/structural_parser.h | 87 +++++++++----------------- 1 file changed, 31 insertions(+), 56 deletions(-) diff --git a/src/generic/stage2/structural_parser.h b/src/generic/stage2/structural_parser.h index d22bb2a84..70ffbad95 100644 --- a/src/generic/stage2/structural_parser.h +++ b/src/generic/stage2/structural_parser.h @@ -191,61 +191,25 @@ struct structural_parser : structural_iterator { return parse_number(current(), found_minus); } - WARN_UNUSED really_inline bool parse_atom() { - switch (current_char()) { - case 't': - log_value("true"); - if (!atomparsing::is_valid_true_atom(current())) { return true; } - append_tape(0, internal::tape_type::TRUE_VALUE); - break; - case 'f': - log_value("false"); - if (!atomparsing::is_valid_false_atom(current())) { return true; } - append_tape(0, internal::tape_type::FALSE_VALUE); - break; - case 'n': - log_value("null"); - if (!atomparsing::is_valid_null_atom(current())) { return true; } - append_tape(0, internal::tape_type::NULL_VALUE); - break; - default: - log_error("IMPOSSIBLE: unrecognized parse_atom structural character"); - return true; - } - return false; - } - - WARN_UNUSED really_inline bool parse_single_atom() { - switch (current_char()) { - case 't': - log_value("true"); - if (!atomparsing::is_valid_true_atom(current(), remaining_len())) { return true; } - append_tape(0, internal::tape_type::TRUE_VALUE); - break; - case 'f': - log_value("false"); - if (!atomparsing::is_valid_false_atom(current(), remaining_len())) { return true; } - append_tape(0, internal::tape_type::FALSE_VALUE); - break; - case 'n': - log_value("null"); - if (!atomparsing::is_valid_null_atom(current(), remaining_len())) { return true; } - append_tape(0, internal::tape_type::NULL_VALUE); - break; - default: - log_error("IMPOSSIBLE: unrecognized parse_atom structural character"); - return true; - } - return false; - } - WARN_UNUSED really_inline ret_address_t parse_value(const unified_machine_addresses &addresses, ret_address_t continue_state) { - switch (current_char()) { + switch (advance_char()) { case '"': FAIL_IF( parse_string() ); return continue_state; - case 't': case 'f': case 'n': - FAIL_IF( parse_atom() ); + case 't': + log_value("true"); + FAIL_IF( !atomparsing::is_valid_true_atom(current()) ); + append_tape(0, internal::tape_type::TRUE_VALUE); + return continue_state; + case 'f': + log_value("false"); + FAIL_IF( !atomparsing::is_valid_false_atom(current()) ); + append_tape(0, internal::tape_type::FALSE_VALUE); + return continue_state; + case 'n': + log_value("null"); + FAIL_IF( !atomparsing::is_valid_null_atom(current()) ); + append_tape(0, internal::tape_type::NULL_VALUE); return continue_state; case '0': case '1': case '2': case '3': case '4': case '5': case '6': case '7': case '8': case '9': @@ -396,8 +360,20 @@ WARN_UNUSED static error_code parse_structurals(dom_parser_implementation &dom_p case '"': FAIL_IF( parser.parse_string() ); goto finish; - case 't': case 'f': case 'n': - FAIL_IF( parser.parse_single_atom() ); + case 't': + parser.log_value("true"); + FAIL_IF( !atomparsing::is_valid_true_atom(parser.current(), parser.remaining_len()) ); + parser.append_tape(0, internal::tape_type::TRUE_VALUE); + goto finish; + case 'f': + parser.log_value("false"); + FAIL_IF( !atomparsing::is_valid_false_atom(parser.current(), parser.remaining_len()) ); + parser.append_tape(0, internal::tape_type::FALSE_VALUE); + goto finish; + case 'n': + parser.log_value("null"); + FAIL_IF( !atomparsing::is_valid_null_atom(parser.current(), parser.remaining_len()) ); + parser.append_tape(0, internal::tape_type::NULL_VALUE); goto finish; case '0': case '1': case '2': case '3': case '4': case '5': case '6': case '7': case '8': case '9': @@ -439,7 +415,6 @@ object_begin: object_key_state: if (parser.advance_char() != ':' ) { parser.log_error("Missing colon after key in object"); goto error; } - parser.advance_char(); GOTO( parser.parse_value(addresses, addresses.object_continue) ); object_continue: @@ -464,7 +439,8 @@ scope_end: // Array parser states // array_begin: - if (parser.advance_char() == ']') { + if (parser.peek_char() == ']') { + parser.advance_char(); parser.end_array(); goto scope_end; } @@ -479,7 +455,6 @@ array_continue: switch (parser.advance_char()) { case ',': parser.increment_count(); - parser.advance_char(); goto main_array_switch; case ']': parser.end_array(); From 5f00b37e2125b696774386552c1027143f3782b9 Mon Sep 17 00:00:00 2001 From: John Keiser Date: Sat, 6 Jun 2020 13:08:34 -0700 Subject: [PATCH 08/10] Stop caching the buffer index --- src/generic/stage2/logger.h | 2 +- src/generic/stage2/structural_iterator.h | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/generic/stage2/logger.h b/src/generic/stage2/logger.h index 2b36ef78e..f00ebf270 100644 --- a/src/generic/stage2/logger.h +++ b/src/generic/stage2/logger.h @@ -64,7 +64,7 @@ namespace logger { printf("| %c ", printable_char(structurals.peek_char())); printf("| %5u ", structurals.parser.structural_indexes[*structurals.next_structural]); printf("| %-*s ", LOG_DETAIL_LEN, detail); - printf("| %*zu ", LOG_INDEX_LEN, structurals.idx); + printf("| %*u ", LOG_INDEX_LEN, *(structurals.next_structural-1)); printf("|\n"); } } diff --git a/src/generic/stage2/structural_iterator.h b/src/generic/stage2/structural_iterator.h index a5405e67e..6866c7df9 100644 --- a/src/generic/stage2/structural_iterator.h +++ b/src/generic/stage2/structural_iterator.h @@ -4,7 +4,6 @@ class structural_iterator { public: const uint8_t* const buf; uint32_t *next_structural; - size_t idx{0}; // location of the structural character in the input (buf) uint8_t c{0}; // used to track the (structural) character we are looking at dom_parser_implementation &parser; @@ -14,9 +13,8 @@ public: parser{_parser} { } really_inline char advance_char() { - idx = *next_structural; + c = buf[*next_structural]; next_structural++; - c = *current(); return c; } really_inline char current_char() { @@ -26,10 +24,13 @@ public: return buf[*next_structural]; } really_inline const uint8_t* current() { - return &buf[idx]; + return &buf[current_structural_index()]; } really_inline size_t remaining_len() { - return parser.len - idx; + return parser.len - current_structural_index(); + } + really_inline uint32_t current_structural_index() { + return *(next_structural-1); } template really_inline bool with_space_terminated_copy(const F& f) { @@ -52,7 +53,7 @@ public: } memcpy(copy, buf, parser.len); memset(copy + parser.len, ' ', SIMDJSON_PADDING); - bool result = f(reinterpret_cast(copy), idx); + bool result = f(reinterpret_cast(copy), current_structural_index()); free(copy); return result; } From d178e089a6434b6a3429c8e8a9187ba7c1f3813d Mon Sep 17 00:00:00 2001 From: John Keiser Date: Sat, 6 Jun 2020 13:40:34 -0700 Subject: [PATCH 09/10] Stop caching current structural, keep current index around instead of next --- src/generic/stage2/logger.h | 4 +- src/generic/stage2/structural_iterator.h | 47 ++++++++++++------------ src/generic/stage2/structural_parser.h | 11 +++--- 3 files changed, 30 insertions(+), 32 deletions(-) diff --git a/src/generic/stage2/logger.h b/src/generic/stage2/logger.h index f00ebf270..7da4f0442 100644 --- a/src/generic/stage2/logger.h +++ b/src/generic/stage2/logger.h @@ -62,9 +62,9 @@ namespace logger { } printf("| %c ", printable_char(structurals.at_beginning() ? ' ' : structurals.current_char())); printf("| %c ", printable_char(structurals.peek_char())); - printf("| %5u ", structurals.parser.structural_indexes[*structurals.next_structural]); + printf("| %5u ", structurals.parser.structural_indexes[*(structurals.current_structural+1)]); printf("| %-*s ", LOG_DETAIL_LEN, detail); - printf("| %*u ", LOG_INDEX_LEN, *(structurals.next_structural-1)); + printf("| %*u ", LOG_INDEX_LEN, *structurals.current_structural); printf("|\n"); } } diff --git a/src/generic/stage2/structural_iterator.h b/src/generic/stage2/structural_iterator.h index 6866c7df9..3228b3a1c 100644 --- a/src/generic/stage2/structural_iterator.h +++ b/src/generic/stage2/structural_iterator.h @@ -3,34 +3,33 @@ namespace stage2 { class structural_iterator { public: const uint8_t* const buf; - uint32_t *next_structural; - uint8_t c{0}; // used to track the (structural) character we are looking at + uint32_t *current_structural; dom_parser_implementation &parser; - really_inline structural_iterator(dom_parser_implementation &_parser, size_t next_structural_index) + // Start a structural + really_inline structural_iterator(dom_parser_implementation &_parser, size_t start_structural_index) : buf{_parser.buf}, - next_structural{&_parser.structural_indexes[next_structural_index]}, + current_structural{&_parser.structural_indexes[start_structural_index]}, parser{_parser} { } - really_inline char advance_char() { - c = buf[*next_structural]; - next_structural++; - return c; - } - really_inline char current_char() { - return c; - } - really_inline char peek_char() { - return buf[*next_structural]; - } + // Get the buffer position of the current structural character really_inline const uint8_t* current() { - return &buf[current_structural_index()]; + return &buf[*current_structural]; + } + // Get the current structural character + really_inline char current_char() { + return buf[*current_structural]; + } + // Get the next structural character without advancing + really_inline char peek_char() { + return buf[*(current_structural+1)]; + } + really_inline char advance_char() { + current_structural++; + return buf[*current_structural]; } really_inline size_t remaining_len() { - return parser.len - current_structural_index(); - } - really_inline uint32_t current_structural_index() { - return *(next_structural-1); + return parser.len - *current_structural; } template really_inline bool with_space_terminated_copy(const F& f) { @@ -53,18 +52,18 @@ public: } memcpy(copy, buf, parser.len); memset(copy + parser.len, ' ', SIMDJSON_PADDING); - bool result = f(reinterpret_cast(copy), current_structural_index()); + bool result = f(reinterpret_cast(copy), *current_structural); free(copy); return result; } really_inline bool past_end(uint32_t n_structural_indexes) { - return next_structural > &parser.structural_indexes[n_structural_indexes]; + return current_structural >= &parser.structural_indexes[n_structural_indexes]; } really_inline bool at_end(uint32_t n_structural_indexes) { - return next_structural == &parser.structural_indexes[n_structural_indexes]; + return current_structural == &parser.structural_indexes[n_structural_indexes]; } really_inline bool at_beginning() { - return next_structural == &parser.structural_indexes[0]; + return current_structural == parser.structural_indexes.get(); } }; diff --git a/src/generic/stage2/structural_parser.h b/src/generic/stage2/structural_parser.h index 70ffbad95..ed2f861a9 100644 --- a/src/generic/stage2/structural_parser.h +++ b/src/generic/stage2/structural_parser.h @@ -72,11 +72,12 @@ struct number_writer { struct structural_parser : structural_iterator { /** Next write location in the string buf for stage 2 parsing */ uint8_t *current_string_buf_loc{}; + /** Current depth (nested objects and arrays) */ uint32_t depth; // For non-streaming, to pass an explicit 0 as next_structural, which enables optimizations - really_inline structural_parser(dom_parser_implementation &_parser, uint32_t _next_structural) - : structural_iterator(_parser, _next_structural), + really_inline structural_parser(dom_parser_implementation &_parser, uint32_t start_structural_index) + : structural_iterator(_parser, start_structural_index), depth{0} { } @@ -232,7 +233,7 @@ struct structural_parser : structural_iterator { WARN_UNUSED really_inline error_code finish() { end_document(); - parser.next_structural_index = uint32_t(next_structural - &parser.structural_indexes[0]); + parser.next_structural_index = uint32_t(current_structural + 1 - &parser.structural_indexes[0]); if (depth != 0) { log_error("Unclosed objects or arrays!"); @@ -283,6 +284,7 @@ struct structural_parser : structural_iterator { } really_inline void init() { + log_start(); current_string_buf_loc = parser.doc->string_buf.get(); parser.current_loc = 0; parser.error = UNINITIALIZED; @@ -294,10 +296,7 @@ struct structural_parser : structural_iterator { return parser.error = EMPTY; } - log_start(); init(); - // Advance to the first character as soon as possible - advance_char(); // Push the root scope (there is always at least one scope) if (start_document(finish_state)) { return parser.error = DEPTH_ERROR; From e15e1e253dba3002133ac2cb4420d3c5bfe27b0a Mon Sep 17 00:00:00 2001 From: John Keiser Date: Fri, 12 Jun 2020 09:10:16 -0700 Subject: [PATCH 10/10] peek_char -> peek_next_char --- src/generic/stage2/logger.h | 2 +- src/generic/stage2/structural_iterator.h | 2 +- src/generic/stage2/structural_parser.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/generic/stage2/logger.h b/src/generic/stage2/logger.h index 7da4f0442..c682fb0de 100644 --- a/src/generic/stage2/logger.h +++ b/src/generic/stage2/logger.h @@ -61,7 +61,7 @@ namespace logger { printf(" "); } printf("| %c ", printable_char(structurals.at_beginning() ? ' ' : structurals.current_char())); - printf("| %c ", printable_char(structurals.peek_char())); + printf("| %c ", printable_char(structurals.peek_next_char())); printf("| %5u ", structurals.parser.structural_indexes[*(structurals.current_structural+1)]); printf("| %-*s ", LOG_DETAIL_LEN, detail); printf("| %*u ", LOG_INDEX_LEN, *structurals.current_structural); diff --git a/src/generic/stage2/structural_iterator.h b/src/generic/stage2/structural_iterator.h index 3228b3a1c..ae47ec91d 100644 --- a/src/generic/stage2/structural_iterator.h +++ b/src/generic/stage2/structural_iterator.h @@ -21,7 +21,7 @@ public: return buf[*current_structural]; } // Get the next structural character without advancing - really_inline char peek_char() { + really_inline char peek_next_char() { return buf[*(current_structural+1)]; } really_inline char advance_char() { diff --git a/src/generic/stage2/structural_parser.h b/src/generic/stage2/structural_parser.h index ed2f861a9..6fabcd88d 100644 --- a/src/generic/stage2/structural_parser.h +++ b/src/generic/stage2/structural_parser.h @@ -438,7 +438,7 @@ scope_end: // Array parser states // array_begin: - if (parser.peek_char() == ']') { + if (parser.peek_next_char() == ']') { parser.advance_char(); parser.end_array(); goto scope_end;