google / quiche

BSD 3-Clause "New" or "Revised" License
660 stars 132 forks source link

optimize SerializedPacket copy/move #35

Closed ktprime closed 1 year ago

ktprime commented 1 year ago

in file quic_packet_creator.h change virtual void OnSerializedPacket(SerializedPacket serialized_packet) = 0 to virtual void OnSerializedPacket(SerializedPacket& serialized_packet) = 0 can reduce twice copy/move SerializedPacket.

in file quic_packet_creator.cc

  SerializedPacket packet(std::move(packet_));
  ClearPacket();
  RemoveSoftMaxPacketLength();
  delegate_->OnSerializedPacket(packet); //no need more/copy now,

some other interface also need changed (use reference to replace move )

bencebeky commented 1 year ago

Moves are cheap. The second move in QuicPacketCreator::OnSerializedPacket() is likely to be optimized away by the compiler. On the other hand, if QuicPacketCreator::DelegateInterface::OnSerializedPacket() was changed to take SerializedPacket by reference instead of value, then the SerializedPacket object would need to be copied in the last line of QuicConnection::OnSerializedPacket():

SendOrQueuePacket(std::move(serialized_packet));

So the proposed change would unfortunately increase the number of copies.

ktprime commented 1 year ago

Thanks for detail reply, no copy increased by my ideal. I have used it in my project. From linux perf benchmark it can reduce tiny cpu usage. some the other interface also need to be adapted.

for example

void QuicConnection::SendOrQueuePacket(SerializedPacket& packet) {
  // The caller of this function is responsible for checking CanWrite().
  WritePacket(&packet);
}

//test on vs2022 image