diff --git a/include/nlohmann/detail/input/binary_reader.hpp b/include/nlohmann/detail/input/binary_reader.hpp index 8a6a0a6d5..8edc53ae9 100644 --- a/include/nlohmann/detail/input/binary_reader.hpp +++ b/include/nlohmann/detail/input/binary_reader.hpp @@ -1856,14 +1856,18 @@ class binary_reader Only the start of an array is tried, which covers arrays of numbers (e.g. coordinates) but costs arrays of other values only one call, not one - per element. + per element. An array of one element is not tried at all: that element is + mostly an object or an array (`[[...]]`, the single ring of a GeoJSON + polygon), for which the call would find no scalar, while a single scalar, + which the run would read faster, is rare. The benchmarks + `FromBinaryShape/nested` and `FromBinaryShape/singles` show both sides. @param[in] len the number of elements @return whether the SAX parser accepted the array and the values read */ bool enter_cbor_array(const std::size_t len) { - return enter_array(len) && (len == 0 || parse_cbor_scalar_run(std::integral_constant {})); + return enter_array(len) && (len <= 1 || parse_cbor_scalar_run(std::integral_constant {})); } /// input that is not contiguous: every value is read by parse_cbor_value @@ -2764,12 +2768,16 @@ class binary_reader /*! @brief open a MessagePack array and read the scalars it begins with (see @ref parse_msgpack_scalar_run) + + An array of one element is not tried, for the reason given at + @ref enter_cbor_array. + @param[in] len the number of elements @return whether the SAX parser accepted the array and the values read */ bool enter_msgpack_array(const std::size_t len) { - return enter_array(len) && (len == 0 || parse_msgpack_scalar_run(std::integral_constant {})); + return enter_array(len) && (len <= 1 || parse_msgpack_scalar_run(std::integral_constant {})); } /// input that is not contiguous: every value is read by parse_msgpack_value diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 693320ed5..47a4aaa18 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -19863,14 +19863,18 @@ class binary_reader Only the start of an array is tried, which covers arrays of numbers (e.g. coordinates) but costs arrays of other values only one call, not one - per element. + per element. An array of one element is not tried at all: that element is + mostly an object or an array (`[[...]]`, the single ring of a GeoJSON + polygon), for which the call would find no scalar, while a single scalar, + which the run would read faster, is rare. The benchmarks + `FromBinaryShape/nested` and `FromBinaryShape/singles` show both sides. @param[in] len the number of elements @return whether the SAX parser accepted the array and the values read */ bool enter_cbor_array(const std::size_t len) { - return enter_array(len) && (len == 0 || parse_cbor_scalar_run(std::integral_constant {})); + return enter_array(len) && (len <= 1 || parse_cbor_scalar_run(std::integral_constant {})); } /// input that is not contiguous: every value is read by parse_cbor_value @@ -20771,12 +20775,16 @@ class binary_reader /*! @brief open a MessagePack array and read the scalars it begins with (see @ref parse_msgpack_scalar_run) + + An array of one element is not tried, for the reason given at + @ref enter_cbor_array. + @param[in] len the number of elements @return whether the SAX parser accepted the array and the values read */ bool enter_msgpack_array(const std::size_t len) { - return enter_array(len) && (len == 0 || parse_msgpack_scalar_run(std::integral_constant {})); + return enter_array(len) && (len <= 1 || parse_msgpack_scalar_run(std::integral_constant {})); } /// input that is not contiguous: every value is read by parse_msgpack_value diff --git a/tests/benchmarks/src/benchmarks.cpp b/tests/benchmarks/src/benchmarks.cpp index 0d9e06d21..9e38a420c 100644 --- a/tests/benchmarks/src/benchmarks.cpp +++ b/tests/benchmarks/src/benchmarks.cpp @@ -629,6 +629,22 @@ static json make_containers() return j; } +/// many arrays of one number each: one-element arrays are left to the general +/// path (CBOR and MessagePack), which reads a lone number slower than the loop +/// for arrays of scalars would +static json make_singles() +{ + json data = json::array(); + for (std::size_t i = 0; i < 100000; ++i) + { + data.push_back(json::array({i})); + } + + json j = json::object(); + j["data"] = std::move(data); + return j; +} + /// one flat array of numbers: the scalar decoding path, which must not move static json make_scalars() { @@ -676,6 +692,8 @@ BENCHMARK_CAPTURE(FromBinaryShape, containers / ubjson_optimized, make_container BENCHMARK_CAPTURE(FromBinaryShape, containers / bjdata, make_containers, binary_format::bjdata); BENCHMARK_CAPTURE(FromBinaryShape, containers / bson, make_containers, binary_format::bson); BENCHMARK_CAPTURE(FromBinaryShape, containers / bon8, make_containers, binary_format::bon8); +BENCHMARK_CAPTURE(FromBinaryShape, singles / cbor, make_singles, binary_format::cbor); +BENCHMARK_CAPTURE(FromBinaryShape, singles / msgpack, make_singles, binary_format::msgpack); // BSON names every array element, so a large array measures key generation // rather than scalar decoding and is left out here BENCHMARK_CAPTURE(FromBinaryShape, scalars / cbor, make_scalars, binary_format::cbor); diff --git a/tests/src/unit-cbor.cpp b/tests/src/unit-cbor.cpp index 930cbe7f5..15b9a7fae 100644 --- a/tests/src/unit-cbor.cpp +++ b/tests/src/unit-cbor.cpp @@ -2520,6 +2520,34 @@ TEST_CASE("CBOR arrays of scalars from contiguous and non-contiguous input") } } +TEST_CASE("CBOR arrays of one element from contiguous and non-contiguous input") +{ + // contiguous input leaves an array of one element to the general path + // rather than to the loop that reads arrays of scalars; both inputs must + // still give the same events, value, or error for the complete document + // and for every truncation of it, with enough bytes after each array that + // the loop could have been entered + const json j = json::parse(R"({"a": [[[[1.5]]]], "b": [7], "c": [{"x": 2.5}], "d": ["s"], "e": [[1, 2]], "f": [null], "g": 1.25})"); + const std::vector bytes = json::to_cbor(j); + + CHECK(json::from_cbor(bytes) == j); + + for (std::size_t length = 0; length <= bytes.size(); ++length) + { + CAPTURE(length) + const std::vector contiguous(bytes.begin(), bytes.begin() + static_cast(length)); + const std::list bytewise(contiguous.begin(), contiguous.end()); + + CHECK(json::from_cbor(contiguous, true, false).dump() == json::from_cbor(bytewise.begin(), bytewise.end(), true, false).dump()); + + SaxEventLogger from_contiguous; + SaxEventLogger from_bytewise; + CHECK(json::sax_parse(contiguous, &from_contiguous, json::input_format_t::cbor, true, false) + == json::sax_parse(bytewise.begin(), bytewise.end(), &from_bytewise, json::input_format_t::cbor, true, false)); + CHECK(from_contiguous.events == from_bytewise.events); + } +} + TEST_CASE("CBOR indefinite-length strings do not recurse per chunk") { // Reading an indefinite-length string or byte array used to call itself diff --git a/tests/src/unit-msgpack.cpp b/tests/src/unit-msgpack.cpp index 9b5891fdd..9c50d22fd 100644 --- a/tests/src/unit-msgpack.cpp +++ b/tests/src/unit-msgpack.cpp @@ -2311,3 +2311,31 @@ TEST_CASE("MessagePack arrays of scalars from contiguous and non-contiguous inpu == json::sax_parse(bytewise.begin(), bytewise.end(), &from_bytewise, json::input_format_t::msgpack)); } } + +TEST_CASE("MessagePack arrays of one element from contiguous and non-contiguous input") +{ + // contiguous input leaves an array of one element to the general path + // rather than to the loop that reads arrays of scalars; both inputs must + // still give the same events, value, or error for the complete document + // and for every truncation of it, with enough bytes after each array that + // the loop could have been entered + const json j = json::parse(R"({"a": [[[[1.5]]]], "b": [7], "c": [{"x": 2.5}], "d": ["s"], "e": [[1, 2]], "f": [null], "g": 1.25})"); + const std::vector bytes = json::to_msgpack(j); + + CHECK(json::from_msgpack(bytes) == j); + + for (std::size_t length = 0; length <= bytes.size(); ++length) + { + CAPTURE(length) + const std::vector contiguous(bytes.begin(), bytes.begin() + static_cast(length)); + const std::list bytewise(contiguous.begin(), contiguous.end()); + + CHECK(json::from_msgpack(contiguous, true, false).dump() == json::from_msgpack(bytewise.begin(), bytewise.end(), true, false).dump()); + + SaxEventLogger from_contiguous; + SaxEventLogger from_bytewise; + CHECK(json::sax_parse(contiguous, &from_contiguous, json::input_format_t::msgpack, true, false) + == json::sax_parse(bytewise.begin(), bytewise.end(), &from_bytewise, json::input_format_t::msgpack, true, false)); + CHECK(from_contiguous.events == from_bytewise.events); + } +}