From bc9fde934777c8bdaf3261563899147130d105ef Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Wed, 9 Aug 2023 17:33:36 +0000 Subject: [PATCH] Remove Http3NoError allocations The Http3Error mechanism is written around dynamic polymorphism, with Http3Error as the base class and a few child classes for specifc types of errors. One of those was Http3NoError. It was observed that the frequent allocations for Http3NoError in many places was wasteful, so this patch removes those allocations by instead returning nullptr instead and removes the Http3NoError class since it is no longer used. Fixes #9992 --- proxy/http3/Http3App.cc | 6 ++--- proxy/http3/Http3FrameCollector.cc | 2 +- proxy/http3/Http3FrameDispatcher.cc | 4 +-- proxy/http3/Http3HeaderVIOAdaptor.cc | 2 +- proxy/http3/Http3ProtocolEnforcer.cc | 2 +- proxy/http3/Http3StreamDataVIOAdaptor.cc | 2 +- proxy/http3/Http3Types.h | 10 +------ proxy/http3/test/Mock.h | 2 +- proxy/http3/test/test_Http3FrameDispatcher.cc | 26 ++++++++++++------- 9 files changed, 27 insertions(+), 29 deletions(-) diff --git a/proxy/http3/Http3App.cc b/proxy/http3/Http3App.cc index 1ec6182852a..71b82566ab5 100644 --- a/proxy/http3/Http3App.cc +++ b/proxy/http3/Http3App.cc @@ -192,7 +192,7 @@ Http3App::create_uni_stream(QUICStreamId &new_stream_id, Http3StreamType type) void Http3App::_handle_uni_stream_on_read_ready(int /* event */, VIO *vio) { - Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError()); + Http3ErrorUPtr error = Http3ErrorUPtr(nullptr); Http3StreamType type; QUICStreamVCAdapter *adapter = static_cast(vio->vc_server); auto it = this->_remote_uni_stream_map.find(adapter->stream().id()); @@ -390,7 +390,7 @@ Http3SettingsHandler::handle_frame(std::shared_ptr frame, int3 if (!settings_frame) { // make error - return Http3ErrorUPtr(new Http3NoError()); + return Http3ErrorUPtr(nullptr); } if (settings_frame->is_valid()) { @@ -426,7 +426,7 @@ Http3SettingsHandler::handle_frame(std::shared_ptr frame, int3 Debug("http3", "SETTINGS_NUM_PLACEHOLDERS: %" PRId64, num_placeholders); } - return Http3ErrorUPtr(new Http3NoError()); + return Http3ErrorUPtr(nullptr); } // diff --git a/proxy/http3/Http3FrameCollector.cc b/proxy/http3/Http3FrameCollector.cc index 1052abd6104..90e2830b05e 100644 --- a/proxy/http3/Http3FrameCollector.cc +++ b/proxy/http3/Http3FrameCollector.cc @@ -47,7 +47,7 @@ Http3FrameCollector::on_write_ready(QUICStreamId stream_id, MIOBuffer &writer, s all_done &= g->is_done(); } - return Http3ErrorUPtr(new Http3NoError()); + return Http3ErrorUPtr(nullptr); } void diff --git a/proxy/http3/Http3FrameDispatcher.cc b/proxy/http3/Http3FrameDispatcher.cc index 039bc60174f..493a76aa9be 100644 --- a/proxy/http3/Http3FrameDispatcher.cc +++ b/proxy/http3/Http3FrameDispatcher.cc @@ -44,7 +44,7 @@ Http3ErrorUPtr Http3FrameDispatcher::on_read_ready(QUICStreamId stream_id, Http3StreamType stream_type, IOBufferReader &reader, uint64_t &nread) { std::shared_ptr frame(nullptr); - Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError()); + Http3ErrorUPtr error = Http3ErrorUPtr(nullptr); nread = 0; uint32_t frame_count = 0; @@ -108,7 +108,7 @@ Http3FrameDispatcher::on_read_ready(QUICStreamId stream_id, Http3StreamType stre std::vector handlers = this->_handlers[static_cast(type)]; for (auto h : handlers) { error = h->handle_frame(frame, frame_count - 1, stream_type); - if (error->cls != Http3ErrorClass::NONE) { + if (error && error->cls != Http3ErrorClass::NONE) { return error; } } diff --git a/proxy/http3/Http3HeaderVIOAdaptor.cc b/proxy/http3/Http3HeaderVIOAdaptor.cc index ffe59c85f37..242b3c63332 100644 --- a/proxy/http3/Http3HeaderVIOAdaptor.cc +++ b/proxy/http3/Http3HeaderVIOAdaptor.cc @@ -65,7 +65,7 @@ Http3HeaderVIOAdaptor::handle_frame(std::shared_ptr frame, int ink_abort("should not be here"); } - return Http3ErrorUPtr(new Http3NoError()); + return Http3ErrorUPtr(nullptr); } bool diff --git a/proxy/http3/Http3ProtocolEnforcer.cc b/proxy/http3/Http3ProtocolEnforcer.cc index b2b64daad68..97a5ba80646 100644 --- a/proxy/http3/Http3ProtocolEnforcer.cc +++ b/proxy/http3/Http3ProtocolEnforcer.cc @@ -37,7 +37,7 @@ Http3ProtocolEnforcer::interests() Http3ErrorUPtr Http3ProtocolEnforcer::handle_frame(std::shared_ptr frame, int32_t frame_seq, Http3StreamType s_type) { - Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError()); + Http3ErrorUPtr error = Http3ErrorUPtr(nullptr); Http3FrameType f_type = frame->type(); if (s_type == Http3StreamType::CONTROL) { if (frame_seq == 0 && f_type != Http3FrameType::SETTINGS) { diff --git a/proxy/http3/Http3StreamDataVIOAdaptor.cc b/proxy/http3/Http3StreamDataVIOAdaptor.cc index a711a629df1..2ee24df2dca 100644 --- a/proxy/http3/Http3StreamDataVIOAdaptor.cc +++ b/proxy/http3/Http3StreamDataVIOAdaptor.cc @@ -48,7 +48,7 @@ Http3StreamDataVIOAdaptor::handle_frame(std::shared_ptr frame, this->_buffer->write(dframe->payload(), dframe->payload_length()); this->_total_data_length += dframe->payload_length(); - return Http3ErrorUPtr(new Http3NoError()); + return Http3ErrorUPtr(nullptr); } void diff --git a/proxy/http3/Http3Types.h b/proxy/http3/Http3Types.h index 2336959ea08..e92064a6cc7 100644 --- a/proxy/http3/Http3Types.h +++ b/proxy/http3/Http3Types.h @@ -113,12 +113,6 @@ class Http3Error : cls(Http3ErrorClass::APPLICATION), app_error_code(error_code), msg(error_msg){}; }; -class Http3NoError : public Http3Error -{ -public: - Http3NoError() : Http3Error() {} -}; - class Http3ConnectionError : public Http3Error { public: @@ -142,6 +136,4 @@ class Http3StreamError : public Http3Error Http3Stream *stream; }; -using Http3ErrorUPtr = std::unique_ptr; -using Http3ConnectionErrorUPtr = std::unique_ptr; -using Http3StreamErrorUPtr = std::unique_ptr; +using Http3ErrorUPtr = std::unique_ptr; diff --git a/proxy/http3/test/Mock.h b/proxy/http3/test/Mock.h index f3b923d09e6..a4999e9f8c7 100644 --- a/proxy/http3/test/Mock.h +++ b/proxy/http3/test/Mock.h @@ -42,6 +42,6 @@ class Http3MockFrameHandler : public Http3FrameHandler handle_frame(std::shared_ptr frame, int32_t /* frame_seq */, Http3StreamType /* s_type */) override { this->total_frame_received++; - return Http3ErrorUPtr(new Http3NoError()); + return Http3ErrorUPtr(nullptr); } }; diff --git a/proxy/http3/test/test_Http3FrameDispatcher.cc b/proxy/http3/test/test_Http3FrameDispatcher.cc index 0987e4e8dd4..4562e24be31 100644 --- a/proxy/http3/test/test_Http3FrameDispatcher.cc +++ b/proxy/http3/test/test_Http3FrameDispatcher.cc @@ -47,7 +47,7 @@ TEST_CASE("Http3FrameHandler dispatch", "[http3]") MIOBuffer *buf = new_MIOBuffer(BUFFER_SIZE_INDEX_512); IOBufferReader *reader = buf->alloc_reader(); uint64_t nread = 0; - Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError()); + Http3ErrorUPtr error = Http3ErrorUPtr(nullptr); buf->write(input, sizeof(input)); @@ -56,7 +56,7 @@ TEST_CASE("Http3FrameHandler dispatch", "[http3]") CHECK(nread == 0); error = http3FrameDispatcher.on_read_ready(0, Http3StreamType::UNKNOWN, *reader, nread); - CHECK(error->app_error_code == Http3ErrorCode::H3_NO_ERROR); + CHECK(!error); CHECK(handler.total_frame_received == 1); CHECK(nread == 12); } @@ -95,7 +95,7 @@ TEST_CASE("control stream tests", "[http3]") MIOBuffer *buf = new_MIOBuffer(BUFFER_SIZE_INDEX_512); IOBufferReader *reader = buf->alloc_reader(); uint64_t nread = 0; - Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError()); + Http3ErrorUPtr error = Http3ErrorUPtr(nullptr); buf->write(input, sizeof(input)); @@ -104,6 +104,7 @@ TEST_CASE("control stream tests", "[http3]") CHECK(nread == 0); error = http3FrameDispatcher.on_read_ready(0, Http3StreamType::CONTROL, *reader, nread); + REQUIRE(error); CHECK(error->app_error_code == Http3ErrorCode::H3_FRAME_UNEXPECTED); CHECK(handler.total_frame_received == 1); CHECK(nread == sizeof(input)); @@ -135,7 +136,7 @@ TEST_CASE("control stream tests", "[http3]") MIOBuffer *buf = new_MIOBuffer(BUFFER_SIZE_INDEX_512); IOBufferReader *reader = buf->alloc_reader(); uint64_t nread = 0; - Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError()); + Http3ErrorUPtr error = Http3ErrorUPtr(nullptr); buf->write(input, sizeof(input)); @@ -144,6 +145,7 @@ TEST_CASE("control stream tests", "[http3]") CHECK(nread == 0); error = http3FrameDispatcher.on_read_ready(0, Http3StreamType::CONTROL, *reader, nread); + REQUIRE(error); CHECK(error->app_error_code == Http3ErrorCode::H3_MISSING_SETTINGS); CHECK(handler.total_frame_received == 0); CHECK(nread == 3); @@ -173,7 +175,7 @@ TEST_CASE("control stream tests", "[http3]") MIOBuffer *buf = new_MIOBuffer(BUFFER_SIZE_INDEX_512); IOBufferReader *reader = buf->alloc_reader(); uint64_t nread = 0; - Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError()); + Http3ErrorUPtr error = Http3ErrorUPtr(nullptr); buf->write(input, sizeof(input)); @@ -182,6 +184,7 @@ TEST_CASE("control stream tests", "[http3]") CHECK(nread == 0); error = http3FrameDispatcher.on_read_ready(0, Http3StreamType::CONTROL, *reader, nread); + REQUIRE(error); CHECK(error->app_error_code == Http3ErrorCode::H3_FRAME_UNEXPECTED); CHECK(handler.total_frame_received == 1); CHECK(nread == sizeof(input)); @@ -211,7 +214,7 @@ TEST_CASE("control stream tests", "[http3]") MIOBuffer *buf = new_MIOBuffer(BUFFER_SIZE_INDEX_512); IOBufferReader *reader = buf->alloc_reader(); uint64_t nread = 0; - Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError()); + Http3ErrorUPtr error = Http3ErrorUPtr(nullptr); buf->write(input, sizeof(input)); @@ -220,6 +223,7 @@ TEST_CASE("control stream tests", "[http3]") CHECK(nread == 0); error = http3FrameDispatcher.on_read_ready(0, Http3StreamType::CONTROL, *reader, nread); + REQUIRE(error); CHECK(error->app_error_code == Http3ErrorCode::H3_FRAME_UNEXPECTED); CHECK(handler.total_frame_received == 1); CHECK(nread == sizeof(input)); @@ -249,7 +253,7 @@ TEST_CASE("control stream tests", "[http3]") MIOBuffer *buf = new_MIOBuffer(BUFFER_SIZE_INDEX_512); IOBufferReader *reader = buf->alloc_reader(); uint64_t nread = 0; - Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError()); + Http3ErrorUPtr error = Http3ErrorUPtr(nullptr); buf->write(input, sizeof(input)); @@ -258,6 +262,7 @@ TEST_CASE("control stream tests", "[http3]") CHECK(nread == 0); error = http3FrameDispatcher.on_read_ready(0, Http3StreamType::CONTROL, *reader, nread); + REQUIRE(error); CHECK(error->app_error_code == Http3ErrorCode::H3_FRAME_UNEXPECTED); CHECK(handler.total_frame_received == 1); CHECK(nread == sizeof(input)); @@ -277,13 +282,13 @@ TEST_CASE("ignore unknown frames", "[http3]") MIOBuffer *buf = new_MIOBuffer(BUFFER_SIZE_INDEX_512); IOBufferReader *reader = buf->alloc_reader(); uint64_t nread = 0; - Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError()); + Http3ErrorUPtr error = Http3ErrorUPtr(nullptr); buf->write(input, sizeof(input)); CHECK(nread == 0); error = http3FrameDispatcher.on_read_ready(0, Http3StreamType::UNKNOWN, *reader, nread); - CHECK(error->app_error_code == Http3ErrorCode::H3_NO_ERROR); + CHECK(!error); CHECK(nread == 0); } } @@ -308,7 +313,7 @@ TEST_CASE("Reserved frame type not allowed", "[http3]") MIOBuffer *buf = new_MIOBuffer(BUFFER_SIZE_INDEX_512); IOBufferReader *reader = buf->alloc_reader(); uint64_t nread = 0; - Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError()); + Http3ErrorUPtr error = Http3ErrorUPtr(nullptr); buf->write(input, sizeof(input)); @@ -317,6 +322,7 @@ TEST_CASE("Reserved frame type not allowed", "[http3]") CHECK(nread == 0); error = http3FrameDispatcher.on_read_ready(0, Http3StreamType::UNKNOWN, *reader, nread); + REQUIRE(error); CHECK(error->app_error_code == Http3ErrorCode::H3_FRAME_UNEXPECTED); CHECK(handler.total_frame_received == 0); CHECK(nread == 12);