Merge 'Support chunked_vector and other json/httpd changes' from Travis Downs

This series came out of an effort to squash large allocations in a json httpd endpoint in redpanda: the basic idea is to optionally use chunked_fifo instead of std::vector as the underlying type in the json2code generation.

Some other bugs I ran into along the the way are also fixed, and even code that doesn't use chunked_fifo should get some benefits from the addition of move support on the json response hot path.

Test cases added where I thought it made sense.

Closes scylladb#2647

* github.com:scylladb/seastar:
  json2code: support chunked_fifo
  json: remove unused headers
  httpd: test cases for streaming
  httpd: handle streaming results in more handlers
  json: stream_object now moves value
  json: support for rvalue ranges
  chunked_fifo: make copyable
  json2code_test: factor out query method
  seastar-json2code: fix error handling

Author: avikivity

include/seastar/core/chunked_fifo.hh

@@ -24,6 +24,7 @@
 #ifndef SEASTAR_MODULE
 #include <algorithm>
 #include <cassert>
+#include <iterator>
 #include <type_traits>
 #include <seastar/util/assert.hh>
 #include <seastar/util/modules.hh>
@@ -177,10 +178,11 @@ public:
 public:
     chunked_fifo() noexcept = default;
     chunked_fifo(chunked_fifo&& x) noexcept;
-    chunked_fifo(const chunked_fifo& X) = delete;
+    chunked_fifo(const chunked_fifo&);
     ~chunked_fifo();
-    chunked_fifo& operator=(const chunked_fifo&) = delete;
+    chunked_fifo& operator=(const chunked_fifo&);
     chunked_fifo& operator=(chunked_fifo&&) noexcept;
+    inline bool operator==(const chunked_fifo& rhs) const;
     inline void push_back(const T& data);
     inline void push_back(T&& data);
     T& back() noexcept;
@@ -297,6 +299,25 @@ chunked_fifo<T, items_per_chunk>::chunked_fifo(chunked_fifo&& x) noexcept
     x._nfree_chunks = 0;
 }
 
+template <typename T, size_t items_per_chunk>
+inline
+chunked_fifo<T, items_per_chunk>::chunked_fifo(const chunked_fifo& rhs)
+        : chunked_fifo() {
+    std::copy_n(rhs.begin(), rhs.size(), std::back_inserter(*this));
+}
+
+template <typename T, size_t items_per_chunk>
+inline
+chunked_fifo<T, items_per_chunk>&
+chunked_fifo<T, items_per_chunk>::operator=(const chunked_fifo& rhs) {
+    if (&rhs != this) {
+        clear();
+        std::copy_n(rhs.begin(), rhs.size(), std::back_inserter(*this));
+        shrink_to_fit();
+    }
+    return *this;
+}
+
 template <typename T, size_t items_per_chunk>
 inline
 chunked_fifo<T, items_per_chunk>&
@@ -456,6 +477,11 @@ chunked_fifo<T, items_per_chunk>::emplace_back(Args&&... args) {
     ++_back_chunk->end;
 }
 
+template <typename T, size_t items_per_chunk>
+inline bool chunked_fifo<T, items_per_chunk>::operator==(const chunked_fifo& rhs) const {
+    return size() == rhs.size() && std::equal(begin(), end(), rhs.begin());
+}
+
 template <typename T, size_t items_per_chunk>
 inline void
 chunked_fifo<T, items_per_chunk>::push_back(const T& data) {

include/seastar/http/function_handlers.hh

@@ -82,31 +82,22 @@ public:
     function_handler(const request_function & _handle, const sstring& type)
             : _f_handle(
                     [_handle](std::unique_ptr<http::request> req, std::unique_ptr<http::reply> rep) {
-                        rep->_content += _handle(*req.get());
-                        return make_ready_future<std::unique_ptr<http::reply>>(std::move(rep));
+                        return append_result(std::move(rep), _handle(*req.get()));
                     }), _type(type) {
     }
 
     function_handler(const json_request_function& _handle)
             : _f_handle(
                     [_handle](std::unique_ptr<http::request> req, std::unique_ptr<http::reply> rep) {
-                        json::json_return_type res = _handle(*req.get());
-                        rep->_content += res._res;
-                        return make_ready_future<std::unique_ptr<http::reply>>(std::move(rep));
+                        return append_result(std::move(rep), _handle(*req.get()));
                     }), _type("json") {
     }
 
     function_handler(const future_json_function& _handle)
             : _f_handle(
                     [_handle](std::unique_ptr<http::request> req, std::unique_ptr<http::reply> rep) {
                         return _handle(std::move(req)).then([rep = std::move(rep)](json::json_return_type&& res) mutable {
-                                if (res._body_writer) {
-                                    rep->write_body("json", std::move(res._body_writer));
-                                } else {
-                                    rep->_content += res._res;
-
-                                }
-                                return make_ready_future<std::unique_ptr<http::reply>>(std::move(rep));
+                            return append_result(std::move(rep), std::move(res));
                         });
                     }), _type("json") {
     }
@@ -122,6 +113,19 @@ public:
                 });
     }
 
+private:
+    // send the json payload of result to reply, return the reply pointer
+    static future<std::unique_ptr<http::reply>> append_result(
+        std::unique_ptr<http::reply>&& reply,
+        json::json_return_type&& result) {
+        if (result._body_writer) {
+            reply->write_body("json", std::move(result._body_writer));
+        } else {
+            reply->_content += result._res;
+        }
+        return make_ready_future<std::unique_ptr<http::reply>>(std::move(reply));
+    }
+
 protected:
     future_handler_function _f_handle;
     sstring _type;

include/seastar/json/formatter.hh

@@ -43,7 +43,7 @@ namespace internal {
 
 template<typename T>
 concept is_map = requires {
-    typename T::mapped_type;
+    typename std::remove_reference_t<T>::mapped_type;
 };
 
 template<typename T>
@@ -357,8 +357,8 @@ public:
      */
     template<std::ranges::input_range Range>
     requires (!internal::is_string_like<Range>)
-    static future<> write(output_stream<char>& s, const Range& range) {
-        return do_with(std::move(range), [&s] (const auto& range) {
+    static future<> write(output_stream<char>& s, Range&& range) {
+        return do_with(std::forward<Range>(range), [&s] (const auto& range) {
             if constexpr (internal::is_map<Range>) {
                 return write(s, state::map, std::ranges::begin(range), std::ranges::end(range));
             } else {

include/seastar/json/json_elements.hh

@@ -24,15 +24,14 @@
 #ifndef SEASTAR_MODULE
 #include <string>
 #include <vector>
-#include <time.h>
-#include <sstream>
 #endif
 
+#include <seastar/core/chunked_fifo.hh>
 #include <seastar/core/do_with.hh>
+#include <seastar/core/iostream.hh>
 #include <seastar/core/loop.hh>
-#include <seastar/json/formatter.hh>
 #include <seastar/core/sstring.hh>
-#include <seastar/core/iostream.hh>
+#include <seastar/json/formatter.hh>
 #include <seastar/util/modules.hh>
 
 namespace seastar {
@@ -151,13 +150,15 @@ private:
 };
 
 /**
- * json_list is based on std vector implementation.
+ * json_list_template is an array type based on a
+ * container type passed as a template parameter, as we want to
+ * have flavors based on both vector and chunked_fifo.
  *
  * When values are added with push it is set the "set" flag to true
  * hence will be included in the parsed object
  */
-template<class T>
-class json_list : public json_base_element {
+template <class T, class Container>
+class json_list_template : public json_base_element {
 public:
 
     /**
@@ -188,7 +189,7 @@ public:
      * iteration and that it's elements can be assigned to the list elements
      */
     template<class C>
-    json_list& operator=(const C& list) {
+    json_list_template& operator=(const C& list) {
         _elements.clear();
         for  (auto i : list) {
             push(i);
@@ -198,9 +199,16 @@ public:
     virtual future<> write(output_stream<char>& s) const override {
         return formatter::write(s, _elements);
     }
-    std::vector<T> _elements;
+
+    Container _elements;
 };
 
+template <typename T>
+using json_list = json_list_template<T, std::vector<T>>;
+
+template <typename T>
+using json_chunked_list = json_list_template<T, seastar::chunked_fifo<T>>;
+
 class jsonable {
 public:
     jsonable() = default;
@@ -368,8 +376,8 @@ std::function<future<>(output_stream<char>&&)> stream_range_as_array(Container v
 template<class T>
 std::function<future<>(output_stream<char>&&)> stream_object(T val) {
     return [val = std::move(val)](output_stream<char>&& s) mutable {
-        return do_with(output_stream<char>(std::move(s)), T(std::move(val)), [](output_stream<char>& s, const T& val){
-            return formatter::write(s, val).finally([&s] {
+        return do_with(output_stream<char>(std::move(s)), T(std::move(val)), [](output_stream<char>& s, T& val){
+            return formatter::write(s, std::move(val)).finally([&s] {
                 return s.close();
             });
         });

scripts/seastar-json2code.py

@@ -97,9 +97,14 @@ def valid_type(param):
     trace_err("Type [", param, "] not defined")
     return param
 
+# Map from json list types to the C++ implementing type
+LIST_TO_IMPL = {"array": "json_list", "chunked_array": "json_chunked_list"}
 
-def type_change(param, member):
-    if param == "array":
+def is_array_type(type: str):
+    return type in LIST_TO_IMPL
+
+def type_change(param: str, member):
+    if is_array_type(param):
         if "items" not in member:
             trace_err("array without item declaration in ", param)
             return ""
@@ -111,7 +116,8 @@ def type_change(param, member):
         else:
             trace_err("array items with no type or ref declaration ", param)
             return ""
-        return "json_list< " + valid_type(t) + " >"
+        return LIST_TO_IMPL[param] + "< " + valid_type(t) + " >"
+
     return "json_element< " + valid_type(param) + " >"
 
 
@@ -371,7 +377,7 @@ def is_model_valid(name, model):
     properties = getitem(model[name], "properties", name)
     for var in properties:
         type = getitem(properties[var], "type", name + ":" + var)
-        if type == "array":
+        if is_array_type(type):
             items = getitem(properties[var], "items", name + ":" + var)
             try:
                 type = getitem(items, "type", name + ":" + var + ":items")
@@ -677,9 +683,8 @@ def parse_file(param, combined):
         if (combined):
             fprintln(combined, '#include "', base_file_name, ".cc", '"')
         create_h_file(data, hfile_name, api_name, init_method, base_api)
-    except:
-        type, value, tb = sys.exc_info()
-        print("Error while parsing JSON file '" + param + "' error ", value.message)
+    except Exception as e:
+        print("Error while parsing JSON file '" + param + "' error " + str(e))
         sys.exit(-1)
 
 if "indir" in config and config.indir != '':

tests/unit/api.json

@@ -47,6 +47,14 @@
                 "VAL2",
                 "VAL3"
               ]
+            },
+            {
+              "name": "use_streaming",
+              "description": "Whether to return the response as a stream_object",
+              "required": true,
+              "allowMultiple": false,
+              "type": "boolean",
+              "paramType": "query"
             }
           ]
         }
@@ -74,6 +82,18 @@
             "VAL2",
             "VAL3"
           ]
+        },
+        "array_var": {
+          "type": "array",
+          "items": {
+            "type": "int"
+          }
+        },
+        "chunked_array_var": {
+          "type": "chunked_array",
+          "items": {
+            "type": "int"
+          }
         }
       }
     }