From bc95264c432d952e83d316cf1935c352eefea6b7 Mon Sep 17 00:00:00 2001 From: Will Wang Date: Sat, 3 Oct 2026 05:49:21 -0700 Subject: [PATCH] Requeue a split body event without recounting its chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Avoid rebuilding a `BufQueue` and walks the whole chain to count its bytes again. Calling the `.move()` Old code: 1. `bodyBuf.move()` hands back a bare unique_ptr and the calculated `chainLength_` is recalculated walking the chain again (this is unnecessary) New code: 1. std::move copies nothing on its own. It only marks bodyBuf as an rvalue BufQueue which then use the move constructor Cowboy edge task T289680342: this walk is ≈ 96 % of the measured cost (≈ $60 K a year on edge hosts). Reviewed By: hanidamlaj Differential Revision: D122754751 fbshipit-source-id: fe5ee78f318332de6f1b842c786e8c30e4b80c3e --- proxygen/lib/http/coro/HTTPStreamSource.cpp | 2 +- .../lib/http/coro/test/HTTPSourceTests.cpp | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/proxygen/lib/http/coro/HTTPStreamSource.cpp b/proxygen/lib/http/coro/HTTPStreamSource.cpp index 75baab326..a8681b343 100644 --- a/proxygen/lib/http/coro/HTTPStreamSource.cpp +++ b/proxygen/lib/http/coro/HTTPStreamSource.cpp @@ -267,7 +267,7 @@ folly::coro::Task HTTPStreamSource::readBodyEvent(uint32_t max) { // the queue. If this happens, EOM must be false. BufQueue bodyBuf(std::move(res.event.body)); res.event.body = bodyBuf.splitAtMost(max); - bodyQueue_.emplace_front(bodyBuf.move(), res.eom); + bodyQueue_.emplace_front(std::move(bodyBuf), res.eom); res.eom = false; length = max; } diff --git a/proxygen/lib/http/coro/test/HTTPSourceTests.cpp b/proxygen/lib/http/coro/test/HTTPSourceTests.cpp index 33d744508..7166ce238 100644 --- a/proxygen/lib/http/coro/test/HTTPSourceTests.cpp +++ b/proxygen/lib/http/coro/test/HTTPSourceTests.cpp @@ -224,6 +224,28 @@ TEST_F(HTTPStreamSourceTest, Body) { EXPECT_EQ(bodyEvents_[4].eom, true); } +// A body event larger than the read limit is split, and the rest goes back to +// the front of the queue with its length. +TEST_F(HTTPStreamSourceTest, SplitBodyKeepsTheRemainderLength) { + auto resp = std::make_unique(); + resp->setStatusCode(200); + stream_.headers(std::move(resp)); + stream_.body(BufQueue(folly::IOBuf::copyBuffer("abcdef")), 0); + stream_.eom(); + + co_withExecutor(&evb_, drainSource(&stream_, 4)).start(); + evb_.loopOnce(); + + EXPECT_FALSE(error_.hasValue()); + ASSERT_GE(bodyEvents_.size(), 2); + EXPECT_EQ(bodyEvents_[0].event.body.chainLength(), 4); + EXPECT_FALSE(bodyEvents_[0].eom); + EXPECT_EQ(bodyEvents_[1].event.body.chainLength(), 2); + auto rest = bodyEvents_[1].event.body.move(); + rest->coalesce(); + EXPECT_EQ(rest->moveToFbString(), std::string("ef")); +} + TEST_F(HTTPStreamSourceTest, PushPromiseBeforeResponseHeaders) { auto promiseSource = HTTPFixedSource::makeFixedResponse( 200, folly::IOBuf::copyBuffer("push body"));