moonchen commented on code in PR #11890:
URL: https://github.com/apache/trafficserver/pull/11890#discussion_r1925533339
##########
src/iocore/eventsystem/P_IOBuffer.h:
##########
@@ -656,6 +656,15 @@ new_MIOBuffer_internal(const char *location, int64_t
size_index)
TS_INLINE void
free_MIOBuffer(MIOBuffer *mio)
{
+#if TS_USE_LINUX_SPLICE
+ // check if mio is PipeIOBuffer using dynamic_cast
+ PipeIOBuffer *pipe_mio = dynamic_cast<PipeIOBuffer *>(mio);
Review Comment:
Relying on a runtime type check with dynamic_cast to select the proper
deallocation mechanism is generally considered an anti-pattern. Instead,
ClassAllocator has a `Destruct_on_free_` parameter that allows you to use a
virtual destructor to clean up properly depending on the underlying type.
##########
src/proxy/http/HttpSM.cc:
##########
@@ -7274,6 +7274,32 @@ HttpSM::setup_blind_tunnel(bool send_response_hdr,
IOBufferReader *initial)
// header buffer into new buffer
client_request_body_bytes +=
from_ua_buf->write(_ua.get_txn()->get_remote_reader());
+#if TS_USE_LINUX_SPLICE
+ MIOBuffer *from_ua_pipe_buf = new_PipeIOBuffer(BUFFER_SIZE_INDEX_32K);
Review Comment:
`BUFFER_SIZE_INDEX_32K` is equal to 8. Is this the capacity of the pipe
that we're requesting from the kernel?
##########
src/iocore/net/UnixNetVConnection.cc:
##########
@@ -508,6 +508,81 @@ UnixNetVConnection::net_read_io(NetHandler *nh)
read_disable(nh, this);
return;
}
+#if TS_USE_LINUX_SPLICE
+ // Check if the buffer is a PipeIOBuffer
+ PipeIOBuffer *pipe_buffer = dynamic_cast<PipeIOBuffer *>(buf.writer());
+ if (pipe_buffer) {
+ // Use splice_to to transfer data from socket directly to pipe with
SPLICE_F_MORE hint
+ int64_t to_splice = std::min(ntodo, pipe_buffer->write_avail());
+ if (to_splice > 0) {
+ r = con.sock.splice_to(pipe_buffer->fd[1], to_splice, SPLICE_F_MOVE |
SPLICE_F_NONBLOCK);
+
+ if (r <= 0) {
+ // Temporary Unavailable, Non-Blocking I/O
+ if (r == -EAGAIN || r == -ENOTCONN) {
Review Comment:
Is it possible that we get EAGAIN here because the pipe is at capacity? How
do we handle that case?
##########
include/iocore/eventsystem/IOBuffer.h:
##########
@@ -1508,3 +1512,116 @@ IOBufferChain::iterator::operator->() const
{
return _b;
}
+
+#if TS_USE_LINUX_SPLICE
+
+class PipeIOBufferReader : public IOBufferReader
Review Comment:
Managing header dependency in ATS (specifically iocore) has been an ongoing
challenge. One of the reasons is that we often combine multiple classes into a
single header file. Please move the new classes to their own header and source
files.
##########
src/iocore/net/UnixNetVConnection.cc:
##########
@@ -508,6 +508,81 @@ UnixNetVConnection::net_read_io(NetHandler *nh)
read_disable(nh, this);
return;
}
+#if TS_USE_LINUX_SPLICE
Review Comment:
This function is getting quite long, with two independent code paths to read
from socket to buffer. I would prefer using polymorphism to handle the
different socket-to-buffer copies, but at least we should split this function
up so that it's more readable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
