Eliminate extra copies for data frames

plugins/cachekey/configs.h

@@ -226,10 +226,10 @@ class Configs
    */
   bool loadClassifiers(const String &args, bool blacklist = true);
 
-  bool _prefixToBeRemoved  = false;     /**< @brief instructs the prefix (i.e. host:port) not to added to the cache key */
-  bool _pathToBeRemoved    = false;     /**< @brief instructs the path not to added to the cache key */
-  bool _canonicalPrefix    = false;     /**< @brief keep the URI scheme and authority element used as input to transforming into key */
-  String _separator        = "/";       /**< @brief a separator used to separate the cache key elements extracted from the URI */
-  CacheKeyUriType _uriType = REMAP;     /**< @brief shows which URI the cache key will be based on */
+  bool _prefixToBeRemoved  = false; /**< @brief instructs the prefix (i.e. host:port) not to added to the cache key */
+  bool _pathToBeRemoved    = false; /**< @brief instructs the path not to added to the cache key */
+  bool _canonicalPrefix    = false; /**< @brief keep the URI scheme and authority element used as input to transforming into key */
+  String _separator        = "/";   /**< @brief a separator used to separate the cache key elements extracted from the URI */
+  CacheKeyUriType _uriType = REMAP; /**< @brief shows which URI the cache key will be based on */
   CacheKeyKeyType _keyType = CACHE_KEY; /**< @brief target URI to be modified, cache key or paren selection */
 };

proxy/http2/Http2ClientSession.h

@@ -80,8 +80,15 @@ struct Http2UpgradeContext {
 class Http2Frame
 {
 public:
+  // Input frame constructor
   Http2Frame(const Http2FrameHeader &h, IOBufferReader *r) : hdr(h), ioreader(r) {}
+  // Output frame contstructor
   Http2Frame(Http2FrameType type, Http2StreamId streamid, uint8_t flags) : hdr({0, (uint8_t)type, flags, streamid}) {}
+  // Output frame constructor
+  Http2Frame(Http2FrameType type, Http2StreamId streamid, uint8_t flags, IOBufferReader *r)
+    : hdr({0, (uint8_t)type, flags, streamid}), output_reader(r)
+  {
+  }
 
   IOBufferReader *
   reader() const
@@ -99,61 +106,79 @@ class Http2Frame
   void
   alloc(int index)
   {
-    this->ioblock = new_IOBufferBlock();
-    this->ioblock->alloc(index);
+    this->output_block = new_IOBufferBlock();
+    this->output_block->alloc(index);
   }
 
   // Return the writeable buffer space for frame payload
   IOVec
   write()
   {
-    return make_iovec(this->ioblock->end(), this->ioblock->write_avail());
+    return make_iovec(this->output_block->end(), this->output_block->write_avail());
   }
 
   // Once the frame has been serialized, update the payload length of frame header.
   void
   finalize(size_t nbytes)
   {
-    if (this->ioblock) {
-      ink_assert((int64_t)nbytes <= this->ioblock->write_avail());
-      this->ioblock->fill(nbytes);
+    if (this->output_block) {
+      ink_assert((int64_t)nbytes <= this->output_block->write_avail());
+      this->output_block->fill(nbytes);
 
-      this->hdr.length = this->ioblock->size();
+      this->hdr.length = this->output_block->size();
     }
   }
 
   void
-  xmit(MIOBuffer *iobuffer)
+  xmit(MIOBuffer *out_iobuffer)
   {
     // Write frame header
     uint8_t buf[HTTP2_FRAME_HEADER_LEN];
     http2_write_frame_header(hdr, make_iovec(buf));
-    iobuffer->write(buf, sizeof(buf));
+    out_iobuffer->write(buf, sizeof(buf));
 
     // Write frame payload
     // It could be empty (e.g. SETTINGS frame with ACK flag)
-    if (ioblock && ioblock->read_avail() > 0) {
-      iobuffer->append_block(this->ioblock.get());
+    if (output_reader) {
+      if (hdr.length > 0) {
+        out_iobuffer->write(output_reader, hdr.length);
+        output_reader->consume(hdr.length);
+      }
+    } else {
+      // Write frame payload
+      // It could be empty (e.g. SETTINGS frame with ACK flag)
+      if (output_block && output_block->read_avail() > 0) {
+        out_iobuffer->append_block(this->output_block.get());
+      }
     }
   }
 
   int64_t
   size()
   {
-    if (ioblock) {
-      return HTTP2_FRAME_HEADER_LEN + ioblock->size();
+    if (output_block) {
+      return HTTP2_FRAME_HEADER_LEN + output_block->size();
     } else {
-      return HTTP2_FRAME_HEADER_LEN;
+      return HTTP2_FRAME_HEADER_LEN + hdr.length;
     }
   }
 
+  void
+  set_payload_size(size_t length)
+  {
+    hdr.length = length;
+  }
+
   // noncopyable
   Http2Frame(Http2Frame &) = delete;
   Http2Frame &operator=(const Http2Frame &) = delete;
 
 private:
-  Http2FrameHeader hdr;       // frame header
-  Ptr<IOBufferBlock> ioblock; // frame payload
+  Http2FrameHeader hdr;            // frame header
+  Ptr<IOBufferBlock> output_block; // frame payload
+  // Reader to put data into output frame
+  IOBufferReader *output_reader = nullptr;
+  // Reader for input frame
   IOBufferReader *ioreader = nullptr;
 };
 

proxy/http2/Http2ConnectionState.cc

@@ -1443,8 +1443,7 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
   const size_t write_available_size = std::min(buf_len, static_cast<size_t>(window_size));
   payload_length                    = 0;
 
-  uint8_t flags = 0x00;
-  uint8_t payload_buffer[buf_len];
+  uint8_t flags       = 0x00;
   IOBufferReader *_sm = stream->response_get_data_reader();
 
   SCOPED_MUTEX_LOCK(stream_lock, stream->mutex, this_ethread());
@@ -1454,45 +1453,36 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
     return Http2SendDataFrameResult::ERROR;
   }
 
-  // Select appropriate payload length
-  if (_sm->is_read_avail_more_than(0)) {
-    // We only need to check for window size when there is a payload
-    if (window_size <= 0) {
-      Http2StreamDebug(this->ua_session, stream->get_id(), "No window");
-      return Http2SendDataFrameResult::NO_WINDOW;
-    }
-    // Copy into the payload buffer. Seems like we should be able to skip this copy step
-    payload_length = write_available_size;
-    payload_length = _sm->read(payload_buffer, static_cast<int64_t>(write_available_size));
-  } else {
-    payload_length = 0;
-  }
-
   // Are we at the end?
   // If we return here, we never send the END_STREAM in the case of a early terminating OS.
   // OK if there is no body yet. Otherwise continue on to send a DATA frame and delete the stream
-  if (!stream->is_body_done() && payload_length == 0) {
+  if (!stream->is_body_done() && !_sm->is_read_avail_more_than(0)) {
     Http2StreamDebug(this->ua_session, stream->get_id(), "No payload");
     return Http2SendDataFrameResult::NO_PAYLOAD;
+  } else if (_sm->is_read_avail_more_than(0)) {
+    // We only need to check for window size when there is a payload
+    if (window_size <= 0) {
+      Http2StreamDebug(this->ua_session, stream->get_id(), "No window");
+      return Http2SendDataFrameResult::NO_WINDOW;
+    }
   }
 
-  if (stream->is_body_done() && !_sm->is_read_avail_more_than(0)) {
+  if (stream->is_body_done() && !_sm->is_read_avail_more_than(write_available_size)) {
     flags |= HTTP2_FLAGS_DATA_END_STREAM;
   }
 
+  Http2Frame data(HTTP2_FRAME_TYPE_DATA, stream->get_id(), flags, _sm);
+
+  payload_length = std::min(static_cast<size_t>(_sm->read_avail()), write_available_size);
+  data.set_payload_size(payload_length);
+
   // Update window size
   this->decrement_client_rwnd(payload_length);
   stream->decrement_client_rwnd(payload_length);
 
-  // Create frame
   Http2StreamDebug(ua_session, stream->get_id(), "Send a DATA frame - client window con: %5zd stream: %5zd payload: %5zd",
                    _client_rwnd, stream->client_rwnd(), payload_length);
 
-  Http2Frame data(HTTP2_FRAME_TYPE_DATA, stream->get_id(), flags);
-  data.alloc(buffer_size_index[HTTP2_FRAME_TYPE_DATA]);
-  http2_write_data(payload_buffer, payload_length, data.write());
-  data.finalize(payload_length);
-
   stream->update_sent_count(payload_length);
 
   // xmit event