Address the rest of the review of editable json documents

From gregmarr's review of #5630:

- alloc_nodes() gives a request of half of the next chunk or more a chunk
  of its own, so that the rest of the current chunk stays available for
  smaller requests instead of being abandoned.
- The comments on edit_state, append_text(), and reserve_moved() say that
  views stay valid across every edit (not just the first), that n is 0 for
  empty strings, and why both capacities are checked.
- The architecture page names flag bits 3, 4, and 5 (linked) separately
  and says that a discarded view has no node; erase.md rewords overload 3
  and says why it never throws invalid_iterator.202.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

docs/mkdocs/docs/api/basic_json_document/erase.md

@@ -18,10 +18,10 @@ has `erase`; calling it on a read-only `basic_json_document` fails to compile (`
 1. Removes every member of `object` whose key is `key` (see [Notes](#notes) on duplicate keys) and returns how many
    were removed; `#!cpp 0` if `object` has no member with this key.
 2. Removes the element at index `idx` of `array`, which must already exist (`#!cpp idx < array.size()`).
-3. Removes the value the JSON pointer `ptr` refers to, relative to [`root()`](root.md), and returns how many values
-   were removed: the *parent* of the target must already exist, and the target itself is removed as in 1. (an object
-   member; `#!cpp 0` or more) or 2. (an array element; always `#!cpp 1`). `ptr` must not be empty -- [`root()`](root.md)
-   itself cannot be erased.
+3. Removes the value the JSON pointer `ptr` refers to, relative to [`root()`](root.md). `ptr` must not be empty
+   ([`root()`](root.md) itself cannot be erased), and the *parent* of the target must already exist. The target is
+   removed as in 1. if the parent is an object (returns the number of removed members, `#!cpp 0` or more) and as in 2.
+   if it is an array (always returns `#!cpp 1`).
 
 ## Template parameters
 
@@ -71,8 +71,8 @@ has `erase`; calling it on a read-only `basic_json_document` fails to compile (`
 
 Overloads 1. and 2. also throw [`invalid_iterator.202`](../../home/exceptions.md#jsonexceptioninvalid_iterator202)
 ("view does not belong to this document") if `object`/`array` is a [discarded](../basic_json_view/is_discarded.md)
-view or a view of a *different* document. Overload 3. takes no view: it always starts from this document's own
-[`root()`](root.md).
+view or a view of a *different* document. Overload 3. never throws `invalid_iterator.202`: it takes no view, and it
+always starts from this document's own [`root()`](root.md).
 
 ## Complexity
 

docs/mkdocs/docs/home/architecture.md

@@ -194,8 +194,8 @@ packet-beta
 
 | Bytes | Field   | Type       | Contents                                                                                                                      |
 |-------|---------|------------|-------------------------------------------------------------------------------------------------------------------------------|
-| 0     | `kind`  | `uint8_t`  | the type, numbered as [`value_t`](../api/basic_json/value_t.md): 0 null, 1 object, 2 array, 3 string, 4 boolean, 5 signed integer, 6 unsigned integer, 7 float; 10 for a link (see below) |
-| 1     | `flags` | `uint8_t`  | bits 0-1: where a string's bytes (or a number's token) are (0: the source text, 1: the buffer of decoded strings, for strings with escapes, 2: the edit buffer); bit 2: the value of a boolean; bits 3 and 4: moved and new (see below) |
+| 0     | `kind`  | `uint8_t`  | the type, numbered as [`value_t`](../api/basic_json/value_t.md): 0 null, 1 object, 2 array, 3 string, 4 boolean, 5 signed integer, 6 unsigned integer, 7 float; 10 for a link (see below). There is no node for a discarded value: a [discarded](../api/basic_json_view/is_discarded.md) view has none |
+| 1     | `flags` | `uint8_t`  | bits 0-1: where a string's bytes (or a number's token) are (0: the source text, 1: the buffer of decoded strings, for strings with escapes, 2: the edit buffer); bit 2: the value of a boolean; bit 3: moved, bit 4: new, bit 5: linked (see below) |
 | 2-3   | `extra` | `uint16_t` | numbers: the number of integer digits (low byte) and fraction digits (high byte), 255 for more; objects: the number of their key hash index (1-based), or 0; otherwise 0 |
 | 4-7   | `off`   | `uint32_t` | where the value starts: the first byte after a string's opening quote (or its position in the buffer of decoded strings), the first byte of a number or literal, the bracket of an array or object |
 | 8-11  | `len`   | `uint32_t` | strings: the length after decoding; floats and literals: the length of the token; arrays and objects: the number of elements |
@@ -257,6 +257,8 @@ edited:
   sequence (a header node, then the entries), whose number is in `off`. The entries are links (`kind` 10), whose bytes
   8-15 hold the address of the value's node, so values never move.
 - A node written by an edit gets the flag *new* (bit 4): it has no position in the source text.
+- A value that an entry of such a sequence links to gets the flag *linked* (bit 5): its extent in the parsed layout
+  (`next`) no longer matters, so a scalar can replace an array or object there in place.
 - Views of read-only documents compile without any of this: how views walk the index is a template parameter
   (`navigation<Editable>`).
 

include/nlohmann/detail/view/document_data.hpp

@@ -58,10 +58,12 @@ struct document_data
     bool discarded = true;
 
     /// The storage of edits (editable documents only; see edit_storage.hpp).
-    /// Edits never move or resize the parsed index, so views stay valid: an
-    /// array/object whose elements change gets node_flags::moved, and its
-    /// elements then live in a separate sequence (a header node, then the
-    /// entries), whose entries link to the values.
+    /// No edit moves or resizes the parsed index, and the nodes of values
+    /// written by edits never move either, so a view stays valid whenever it
+    /// was taken, before the first edit or between later ones: an array/object
+    /// whose elements change gets node_flags::moved, and its elements then live
+    /// in a separate sequence (a header node, then the entries), whose entries
+    /// link to the values.
     struct edit_state
     {
         std::vector<node*> moved{};                    ///< element sequences of moved arrays/objects (header node first) // NOLINT(readability-redundant-member-init)

include/nlohmann/detail/view/edit_storage.hpp

@@ -50,7 +50,19 @@ inline node* alloc_nodes(document_data& d, std::size_t k)
     document_data::edit_state& e = edit_state_of(d);
     if (NLOHMANN_VIEW_UNLIKELY(static_cast<std::size_t>(e.chunk_end - e.chunk_cur) < k))
     {
-        const std::size_t count = (std::max)(k, e.chunk_next);
+        if (k >= e.chunk_next / 2)
+        {
+            // a large request (half of the next chunk or more) gets a chunk of
+            // its own, so that the rest of the current chunk stays available
+            // for smaller ones
+            std::unique_ptr<node[]> own(new node[k]()); // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays)
+            e.chunks.push_back(std::move(own));
+            e.bytes += k * sizeof(node);
+            return e.chunks.back().get();
+        }
+        // a smaller request leaves the rest of the current chunk unused
+        // (fewer than k nodes, and k is less than half of the new chunk)
+        const std::size_t count = e.chunk_next;
         std::unique_ptr<node[]> fresh(new node[count]()); // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays)
         e.chunks.push_back(std::move(fresh));
         e.chunk_cur = e.chunks.back().get();
@@ -81,7 +93,8 @@ inline std::size_t text_capacity(std::size_t cap, std::size_t used, std::size_t
 }
 
 /// copy n bytes into the edit arena and return their offset; a new buffer
-/// leaves the old one alive, so that string views into it remain valid
+/// leaves the old one alive, so that string views into it remain valid; n is
+/// 0 for an empty string (written by an edit, or the key of a new member)
 inline std::uint32_t append_text(document_data& d, const char* s, std::size_t n)
 {
     document_data::edit_state& e = edit_state_of(d);
@@ -89,7 +102,7 @@ inline std::uint32_t append_text(document_data& d, const char* s, std::size_t n)
     {
         const std::size_t cap = text_capacity(e.text_cap, e.text_used, n);
         std::unique_ptr<char[]> fresh(new char[cap]); // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays)
-        if (e.text_used != 0)
+        if (NLOHMANN_VIEW_LIKELY(e.text_used != 0)) // (0 only for the first buffer, and then texts is empty)
         {
             std::memcpy(fresh.get(), e.texts.back().get(), e.text_used);
         }
@@ -127,8 +140,10 @@ inline void reserve_moved(document_data& d)
     if (e.moved.size() == e.moved.capacity() || e.moved_cap.size() == e.moved_cap.capacity())
     {
         // both grow before either changes, so that the push_backs cannot throw
-        e.moved.reserve((2 * e.moved.size()) + 16);
-        e.moved_cap.reserve((2 * e.moved.size()) + 16);
+        // (both conditions are needed: reserve() may allocate more than asked,
+        // so the two capacities can differ)
+        e.moved.reserve((e.moved.size() * 2) + 16);
+        e.moved_cap.reserve((e.moved.size() * 2) + 16);
     }
 }
 

single_include/nlohmann/json_view.hpp

@@ -372,10 +372,12 @@ struct document_data
     bool discarded = true;
 
     /// The storage of edits (editable documents only; see edit_storage.hpp).
-    /// Edits never move or resize the parsed index, so views stay valid: an
-    /// array/object whose elements change gets node_flags::moved, and its
-    /// elements then live in a separate sequence (a header node, then the
-    /// entries), whose entries link to the values.
+    /// No edit moves or resizes the parsed index, and the nodes of values
+    /// written by edits never move either, so a view stays valid whenever it
+    /// was taken, before the first edit or between later ones: an array/object
+    /// whose elements change gets node_flags::moved, and its elements then live
+    /// in a separate sequence (a header node, then the entries), whose entries
+    /// link to the values.
     struct edit_state
     {
         std::vector<node*> moved{};                    ///< element sequences of moved arrays/objects (header node first) // NOLINT(readability-redundant-member-init)
@@ -2874,7 +2876,19 @@ inline node* alloc_nodes(document_data& d, std::size_t k)
     document_data::edit_state& e = edit_state_of(d);
     if (NLOHMANN_VIEW_UNLIKELY(static_cast<std::size_t>(e.chunk_end - e.chunk_cur) < k))
     {
-        const std::size_t count = (std::max)(k, e.chunk_next);
+        if (k >= e.chunk_next / 2)
+        {
+            // a large request (half of the next chunk or more) gets a chunk of
+            // its own, so that the rest of the current chunk stays available
+            // for smaller ones
+            std::unique_ptr<node[]> own(new node[k]()); // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays)
+            e.chunks.push_back(std::move(own));
+            e.bytes += k * sizeof(node);
+            return e.chunks.back().get();
+        }
+        // a smaller request leaves the rest of the current chunk unused
+        // (fewer than k nodes, and k is less than half of the new chunk)
+        const std::size_t count = e.chunk_next;
         std::unique_ptr<node[]> fresh(new node[count]()); // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays)
         e.chunks.push_back(std::move(fresh));
         e.chunk_cur = e.chunks.back().get();
@@ -2905,7 +2919,8 @@ inline std::size_t text_capacity(std::size_t cap, std::size_t used, std::size_t
 }
 
 /// copy n bytes into the edit arena and return their offset; a new buffer
-/// leaves the old one alive, so that string views into it remain valid
+/// leaves the old one alive, so that string views into it remain valid; n is
+/// 0 for an empty string (written by an edit, or the key of a new member)
 inline std::uint32_t append_text(document_data& d, const char* s, std::size_t n)
 {
     document_data::edit_state& e = edit_state_of(d);
@@ -2913,7 +2928,7 @@ inline std::uint32_t append_text(document_data& d, const char* s, std::size_t n)
     {
         const std::size_t cap = text_capacity(e.text_cap, e.text_used, n);
         std::unique_ptr<char[]> fresh(new char[cap]); // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays)
-        if (e.text_used != 0)
+        if (NLOHMANN_VIEW_LIKELY(e.text_used != 0)) // (0 only for the first buffer, and then texts is empty)
         {
             std::memcpy(fresh.get(), e.texts.back().get(), e.text_used);
         }
@@ -2951,8 +2966,10 @@ inline void reserve_moved(document_data& d)
     if (e.moved.size() == e.moved.capacity() || e.moved_cap.size() == e.moved_cap.capacity())
     {
         // both grow before either changes, so that the push_backs cannot throw
-        e.moved.reserve((2 * e.moved.size()) + 16);
-        e.moved_cap.reserve((2 * e.moved.size()) + 16);
+        // (both conditions are needed: reserve() may allocate more than asked,
+        // so the two capacities can differ)
+        e.moved.reserve((e.moved.size() * 2) + 16);
+        e.moved_cap.reserve((e.moved.size() * 2) + 16);
     }
 }
 

tests/src/unit-json_view_edit.cpp

@@ -26,6 +26,7 @@ using ptr_t = ordered_json::json_pointer;
 #include <functional>
 #include <iterator>
 #include <limits>
+#include <memory>
 #include <random>
 #include <set>
 #include <string>
@@ -915,6 +916,29 @@ TEST_CASE("json_view edits: the size of a text arena")
 }
 #endif
 
+TEST_CASE("json_view edits: the chunks of new nodes")
+{
+    using nlohmann::detail::view::alloc_nodes;
+    using nlohmann::detail::view::document_data;
+    const std::unique_ptr<document_data, document_data::deleter> d(document_data::create(0));
+    // the first chunk has room for 64 nodes, the next one for 128
+    const nlohmann::detail::view::node* const first = alloc_nodes(*d, 10);
+    CHECK(d->edits->chunks.size() == 1);
+    // a request of half of the next chunk (128 nodes) or more gets a chunk of
+    // its own
+    const nlohmann::detail::view::node* const large = alloc_nodes(*d, 100);
+    CHECK(d->edits->chunks.size() == 2);
+    CHECK(large == d->edits->chunks.back().get());
+    // and the rest of the first chunk is still used
+    CHECK(alloc_nodes(*d, 50) == first + 10);
+    CHECK(d->edits->chunks.size() == 2);
+    // a smaller request that does not fit starts a new (doubled) chunk
+    CHECK(alloc_nodes(*d, 10) == d->edits->chunks.back().get());
+    CHECK(d->edits->chunks.size() == 3);
+    CHECK(d->edits->chunk_end - d->edits->chunk_cur == 128 - 10);
+    CHECK(d->edits->bytes == (64 + 100 + 128) * sizeof(nlohmann::detail::view::node));
+}
+
 TEST_CASE("json_view edits: replacing arrays and objects by scalars")
 {
     // the first assignment switches the parent to links; the later ones do