Inline getPacketSequenceNum and getPacketNumSpace

Summary: As it turns out these end up being hot functions. Most of the hotness is it taking the cache miss, but we're able to reduce it from .9% exclusive each to .1-.3% exclusive each by inlining it (which eliminates a `callq` instruction for each).

Reviewed By: yangchi

Differential Revision: D19069685

fbshipit-source-id: c971a4d1c26a7e48008c36a129e0a842a27ca87f

quic/api/QuicTransportFunctions.cpp

@@ -570,7 +570,7 @@ uint64_t writeCryptoAndAckDataToSocket(
       std::move(FrameScheduler::Builder(
                     connection,
                     encryptionLevel,
-                    longHeaderTypeToPacketNumberSpace(packetType),
+                    LongHeader::typeToPacketNumberSpace(packetType),
                     "CryptoAndAcksScheduler")
                     .ackFrames()
                     .cryptoFrames())
@@ -583,7 +583,7 @@ uint64_t writeCryptoAndAckDataToSocket(
       srcConnId,
       dstConnId,
       std::move(builder),
-      longHeaderTypeToPacketNumberSpace(packetType),
+      LongHeader::typeToPacketNumberSpace(packetType),
       scheduler,
       congestionControlWritableBytes,
       packetLimit,
@@ -686,7 +686,7 @@ uint64_t writeZeroRttDataToSocket(
       std::move(FrameScheduler::Builder(
                     connection,
                     encryptionLevel,
-                    longHeaderTypeToPacketNumberSpace(type),
+                    LongHeader::typeToPacketNumberSpace(type),
                     "ZeroRttScheduler")
                     .streamFrames()
                     .streamRetransmissions()
@@ -701,7 +701,7 @@ uint64_t writeZeroRttDataToSocket(
       srcConnId,
       dstConnId,
       std::move(builder),
-      longHeaderTypeToPacketNumberSpace(type),
+      LongHeader::typeToPacketNumberSpace(type),
       scheduler,
       congestionControlWritableBytes,
       packetLimit,
@@ -827,7 +827,7 @@ void writeLongClose(
       srcConnId,
       dstConnId,
       getNextPacketNum(
-          connection, longHeaderTypeToPacketNumberSpace(headerType)),
+          connection, LongHeader::typeToPacketNumberSpace(headerType)),
       version);
   writeCloseCommon(
       sock,

quic/api/test/QuicTransportFunctionsTest.cpp

@@ -64,7 +64,7 @@ void writeCryptoDataProbesToSocketForTest(
     LongHeader::Types type = LongHeader::Types::Initial) {
   auto encryptionLevel =
       protectionTypeToEncryptionLevel(longHeaderTypeToProtectionType(type));
-  auto pnSpace = longHeaderTypeToPacketNumberSpace(type);
+  auto pnSpace = LongHeader::typeToPacketNumberSpace(type);
   auto scheduler = std::move(FrameScheduler::Builder(
                                  conn, encryptionLevel, pnSpace, "Crypto")
                                  .cryptoFrames())
@@ -75,7 +75,7 @@ void writeCryptoDataProbesToSocketForTest(
       *conn.clientConnectionId,
       *conn.serverConnectionId,
       LongHeaderBuilder(type),
-      longHeaderTypeToPacketNumberSpace(type),
+      LongHeader::typeToPacketNumberSpace(type),
       scheduler,
       probesToSend,
       aead,

quic/codec/Types.cpp

@@ -145,17 +145,6 @@ const ShortHeader* PacketHeader::asShort() const {
   }
 }
 
-PacketNum PacketHeader::getPacketSequenceNum() const {
-  switch (headerForm_) {
-    case HeaderForm::Long:
-      return longHeader.getPacketSequenceNum();
-    case HeaderForm::Short:
-      return shortHeader.getPacketSequenceNum();
-    default:
-      folly::assume_unreachable();
-  }
-}
-
 HeaderForm PacketHeader::getHeaderForm() const {
   return headerForm_;
 }
@@ -171,17 +160,6 @@ ProtectionType PacketHeader::getProtectionType() const {
   }
 }
 
-PacketNumberSpace PacketHeader::getPacketNumberSpace() const {
-  switch (headerForm_) {
-    case HeaderForm::Long:
-      return longHeader.getPacketNumberSpace();
-    case HeaderForm::Short:
-      return shortHeader.getPacketNumberSpace();
-    default:
-      folly::assume_unreachable();
-  }
-}
-
 LongHeader::LongHeader(
     Types type,
     LongHeaderInvariant invariant,
@@ -235,10 +213,6 @@ const std::string& LongHeader::getToken() const {
   return token_;
 }
 
-PacketNum LongHeader::getPacketSequenceNum() const {
-  return packetSequenceNum_;
-}
-
 void LongHeader::setPacketNumber(PacketNum packetNum) {
   packetSequenceNum_ = packetNum;
 }
@@ -247,10 +221,6 @@ ProtectionType LongHeader::getProtectionType() const {
   return longHeaderTypeToProtectionType(getHeaderType());
 }
 
-PacketNumberSpace LongHeader::getPacketNumberSpace() const {
-  return longHeaderTypeToPacketNumberSpace(getHeaderType());
-}
-
 ProtectionType longHeaderTypeToProtectionType(
     LongHeader::Types longHeaderType) {
   switch (longHeaderType) {
@@ -268,20 +238,6 @@ ProtectionType longHeaderTypeToProtectionType(
 ShortHeaderInvariant::ShortHeaderInvariant(ConnectionId dcid)
     : destinationConnId(std::move(dcid)) {}
 
-PacketNumberSpace longHeaderTypeToPacketNumberSpace(
-    LongHeader::Types longHeaderType) {
-  switch (longHeaderType) {
-    case LongHeader::Types::Initial:
-    case LongHeader::Types::Retry:
-      return PacketNumberSpace::Initial;
-    case LongHeader::Types::Handshake:
-      return PacketNumberSpace::Handshake;
-    case LongHeader::Types::ZeroRtt:
-      return PacketNumberSpace::AppData;
-  }
-  folly::assume_unreachable();
-}
-
 ShortHeader::ShortHeader(
     ProtectionType protectionType,
     ConnectionId connId,
@@ -308,18 +264,10 @@ ProtectionType ShortHeader::getProtectionType() const {
   return protectionType_;
 }
 
-PacketNumberSpace ShortHeader::getPacketNumberSpace() const {
-  return PacketNumberSpace::AppData;
-}
-
 const ConnectionId& ShortHeader::getConnectionId() const {
   return connectionId_;
 }
 
-PacketNum ShortHeader::getPacketSequenceNum() const {
-  return packetSequenceNum_;
-}
-
 void ShortHeader::setPacketNumber(PacketNum packetNum) {
   packetSequenceNum_ = packetNum;
 }

quic/codec/Types.h

@@ -645,6 +645,20 @@ struct LongHeader {
     Retry = 0x3,
   };
 
+  // Note this is defined in the header so it is inlined for performance.
+  static PacketNumberSpace typeToPacketNumberSpace(Types longHeaderType) {
+    switch (longHeaderType) {
+      case LongHeader::Types::Initial:
+      case LongHeader::Types::Retry:
+        return PacketNumberSpace::Initial;
+      case LongHeader::Types::Handshake:
+        return PacketNumberSpace::Handshake;
+      case LongHeader::Types::ZeroRtt:
+        return PacketNumberSpace::AppData;
+    }
+    folly::assume_unreachable();
+  }
+
   LongHeader(
       Types type,
       const ConnectionId& srcConnId,
@@ -670,11 +684,17 @@ struct LongHeader {
   const ConnectionId& getDestinationConnId() const;
   const folly::Optional<ConnectionId>& getOriginalDstConnId() const;
   QuicVersion getVersion() const;
-  PacketNumberSpace getPacketNumberSpace() const;
+  // Note this is defined in the header so it is inlined for performance.
+  PacketNumberSpace getPacketNumberSpace() const {
+    return typeToPacketNumberSpace(longHeaderType_);
+  }
   ProtectionType getProtectionType() const;
   bool hasToken() const;
   const std::string& getToken() const;
-  PacketNum getPacketSequenceNum() const;
+  // Note this is defined in the header so it is inlined for performance.
+  PacketNum getPacketSequenceNum() const {
+    return packetSequenceNum_;
+  }
 
   void setPacketNumber(PacketNum packetNum);
 
@@ -717,8 +737,12 @@ struct ShortHeader {
       PacketNum packetNum);
 
   ProtectionType getProtectionType() const;
-  PacketNumberSpace getPacketNumberSpace() const;
+  PacketNumberSpace getPacketNumberSpace() const {
-  PacketNum getPacketSequenceNum() const;
+    return PacketNumberSpace::AppData;
+  }
+  PacketNum getPacketSequenceNum() const {
+    return packetSequenceNum_;
+  }
   const ConnectionId& getConnectionId() const;
 
   void setPacketNumber(PacketNum packetNum);
@@ -755,10 +779,30 @@ struct PacketHeader {
   const LongHeader* asLong() const;
   const ShortHeader* asShort() const;
 
-  PacketNum getPacketSequenceNum() const;
+  // Note this is defined in the header so it is inlined for performance.
+  PacketNum getPacketSequenceNum() const {
+    switch (headerForm_) {
+      case HeaderForm::Long:
+        return longHeader.getPacketSequenceNum();
+      case HeaderForm::Short:
+        return shortHeader.getPacketSequenceNum();
+      default:
+        folly::assume_unreachable();
+    }
+  }
   HeaderForm getHeaderForm() const;
   ProtectionType getProtectionType() const;
-  PacketNumberSpace getPacketNumberSpace() const;
+  // Note this is defined in the header so it is inlined for performance.
+  PacketNumberSpace getPacketNumberSpace() const {
+    switch (headerForm_) {
+      case HeaderForm::Long:
+        return longHeader.getPacketNumberSpace();
+      case HeaderForm::Short:
+        return shortHeader.getPacketNumberSpace();
+      default:
+        folly::assume_unreachable();
+    }
+  }
 
  private:
   void destroyHeader();
@@ -773,8 +817,6 @@ struct PacketHeader {
 
 ProtectionType longHeaderTypeToProtectionType(LongHeader::Types type);
 
-PacketNumberSpace longHeaderTypeToPacketNumberSpace(LongHeader::Types type);
-
 struct StreamTypeField {
  public:
   explicit StreamTypeField(uint8_t field) : field_(field) {}