Bug 1251821: increase UDP socket send buffer on Win 7 r=jdm,jesup
authorNils Ohlmeier [:drno] <drno@ohlmeier.org>
Fri, 08 Apr 2016 16:51:21 -0700
changeset 295036 3cc3b76f4205ae74b7f4137abd6d6e11b51b9d61
parent 295035 7a20af9bdc35290862e7cbc0e580a761d7e700a2
child 295037 ab752670d22aae2f4f56cd042a98ec239e0dc931
push id30217
push usercbook@mozilla.com
push dateWed, 27 Apr 2016 09:58:13 +0000
treeherdermozilla-central@ab0044bfa1df [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm, jesup
bugs1251821
milestone49.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1251821: increase UDP socket send buffer on Win 7 r=jdm,jesup MozReview-Commit-ID: ETkx1Z56bI8
dom/network/PUDPSocket.ipdl
dom/network/UDPSocket.cpp
dom/network/UDPSocketChild.cpp
dom/network/UDPSocketParent.cpp
dom/network/UDPSocketParent.h
dom/network/interfaces/nsIUDPSocketChild.idl
media/mtransport/nr_socket_prsock.cpp
netwerk/base/nsIUDPSocket.idl
netwerk/base/nsUDPSocket.cpp
--- a/dom/network/PUDPSocket.ipdl
+++ b/dom/network/PUDPSocket.ipdl
@@ -36,17 +36,18 @@ namespace mozilla {
 namespace net {
 
 //-------------------------------------------------------------------
 protocol PUDPSocket
 {
   manager PNecko or PBackground;
 
 parent:
-  async Bind(UDPAddressInfo addressInfo, bool addressReuse, bool loopback, uint32_t recvBufferSize);
+  async Bind(UDPAddressInfo addressInfo, bool addressReuse, bool loopback,
+             uint32_t recvBufferSize, uint32_t sendBufferSize);
   async Connect(UDPAddressInfo addressInfo);
 
   async OutgoingData(UDPData data, UDPSocketAddr addr);
 
   async JoinMulticast(nsCString multicastAddress, nsCString iface);
   async LeaveMulticast(nsCString multicastAddress, nsCString iface);
 
   async Close();
--- a/dom/network/UDPSocket.cpp
+++ b/dom/network/UDPSocket.cpp
@@ -499,16 +499,17 @@ UDPSocket::InitRemote(const nsAString& a
   }
 
   rv = sock->Bind(mListenerProxy,
                   principal,
                   NS_ConvertUTF16toUTF8(aLocalAddress),
                   aLocalPort,
                   mAddressReuse,
                   mLoopback,
+                  0,
                   0);
 
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   mSocketChild = sock;
 
--- a/dom/network/UDPSocketChild.cpp
+++ b/dom/network/UDPSocketChild.cpp
@@ -167,17 +167,18 @@ UDPSocketChild::SetBackgroundSpinsEvents
 
 NS_IMETHODIMP
 UDPSocketChild::Bind(nsIUDPSocketInternal* aSocket,
                      nsIPrincipal* aPrincipal,
                      const nsACString& aHost,
                      uint16_t aPort,
                      bool aAddressReuse,
                      bool aLoopback,
-                     uint32_t recvBufferSize)
+                     uint32_t recvBufferSize,
+                     uint32_t sendBufferSize)
 {
   UDPSOCKET_LOG(("%s: %s:%u", __FUNCTION__, PromiseFlatCString(aHost).get(), aPort));
 
   NS_ENSURE_ARG(aSocket);
 
   mSocket = aSocket;
   AddIPDLReference();
 
@@ -186,17 +187,18 @@ UDPSocketChild::Bind(nsIUDPSocketInterna
     // convert it to a PrincipalInfo
     MOZ_ASSERT(!aPrincipal);
     mBackgroundManager->SendPUDPSocketConstructor(this, void_t(), mFilterName);
   } else {
     gNeckoChild->SendPUDPSocketConstructor(this, IPC::Principal(aPrincipal),
                                            mFilterName);
   }
 
-  SendBind(UDPAddressInfo(nsCString(aHost), aPort), aAddressReuse, aLoopback, recvBufferSize);
+  SendBind(UDPAddressInfo(nsCString(aHost), aPort), aAddressReuse, aLoopback,
+           recvBufferSize, sendBufferSize);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 UDPSocketChild::Connect(nsIUDPSocketInternal* aSocket, const nsACString & aHost, uint16_t aPort)
 {
   UDPSOCKET_LOG(("%s: %s:%u", __FUNCTION__, PromiseFlatCString(aHost).get(), aPort));
 
--- a/dom/network/UDPSocketParent.cpp
+++ b/dom/network/UDPSocketParent.cpp
@@ -157,21 +157,24 @@ UDPSocketParent::Init(const IPC::Princip
   return true;
 }
 
 // PUDPSocketParent methods
 
 bool
 UDPSocketParent::RecvBind(const UDPAddressInfo& aAddressInfo,
                           const bool& aAddressReuse, const bool& aLoopback,
-                          const uint32_t& recvBufferSize)
+                          const uint32_t& recvBufferSize,
+                          const uint32_t& sendBufferSize)
 {
   UDPSOCKET_LOG(("%s: %s:%u", __FUNCTION__, aAddressInfo.addr().get(), aAddressInfo.port()));
 
-  if (NS_FAILED(BindInternal(aAddressInfo.addr(), aAddressInfo.port(), aAddressReuse, aLoopback, recvBufferSize))) {
+  if (NS_FAILED(BindInternal(aAddressInfo.addr(), aAddressInfo.port(),
+                             aAddressReuse, aLoopback, recvBufferSize,
+                             sendBufferSize))) {
     FireInternalError(__LINE__);
     return true;
   }
 
   nsCOMPtr<nsINetAddr> localAddr;
   mSocket->GetLocalAddr(getter_AddRefs(localAddr));
 
   nsCString addr;
@@ -190,21 +193,24 @@ UDPSocketParent::RecvBind(const UDPAddre
   mozilla::Unused << SendCallbackOpened(UDPAddressInfo(addr, port));
 
   return true;
 }
 
 nsresult
 UDPSocketParent::BindInternal(const nsCString& aHost, const uint16_t& aPort,
                               const bool& aAddressReuse, const bool& aLoopback,
-                              const uint32_t& recvBufferSize)
+                              const uint32_t& recvBufferSize,
+                              const uint32_t& sendBufferSize)
 {
   nsresult rv;
 
-  UDPSOCKET_LOG(("%s: [this=%p] %s:%u addressReuse: %d loopback: %d recvBufferSize: %lu", __FUNCTION__, this, nsCString(aHost).get(), aPort, aAddressReuse, aLoopback, recvBufferSize));
+  UDPSOCKET_LOG(("%s: [this=%p] %s:%u addressReuse: %d loopback: %d recvBufferSize: %lu, sendBufferSize: %lu",
+                __FUNCTION__, this, nsCString(aHost).get(), aPort,
+                aAddressReuse, aLoopback, recvBufferSize, sendBufferSize));
 
   nsCOMPtr<nsIUDPSocket> sock =
       do_CreateInstance("@mozilla.org/network/udp-socket;1", &rv);
 
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
@@ -240,22 +246,29 @@ UDPSocketParent::BindInternal(const nsCS
     return rv;
   }
   if (family == nsINetAddr::FAMILY_INET) {
     rv = sock->SetMulticastLoopback(aLoopback);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   }
+  // TODO: once bug 1252759 is fixed query buffer first and only increase
   if (recvBufferSize != 0) {
     rv = sock->SetRecvBufferSize(recvBufferSize);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       UDPSOCKET_LOG(("%s: [this=%p] %s:%u failed to set recv buffer size to: %lu", __FUNCTION__, this, nsCString(aHost).get(), aPort, recvBufferSize));
     }
   }
+  if (sendBufferSize != 0) {
+    rv = sock->SetSendBufferSize(sendBufferSize);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      UDPSOCKET_LOG(("%s: [this=%p] %s:%u failed to set send buffer size to: %lu", __FUNCTION__, this, nsCString(aHost).get(), aPort, sendBufferSize));
+    }
+  }
 
   // register listener
   rv = sock->AsyncListen(this);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   mSocket = sock;
--- a/dom/network/UDPSocketParent.h
+++ b/dom/network/UDPSocketParent.h
@@ -31,17 +31,18 @@ public:
 
   explicit UDPSocketParent(PBackgroundParent* aManager);
   explicit UDPSocketParent(PNeckoParent* aManager);
 
   bool Init(const IPC::Principal& aPrincipal, const nsACString& aFilter);
 
   virtual bool RecvBind(const UDPAddressInfo& aAddressInfo,
                         const bool& aAddressReuse, const bool& aLoopback,
-                        const uint32_t& recvBufferSize) override;
+                        const uint32_t& recvBufferSize,
+                        const uint32_t& sendBufferSize) override;
   virtual bool RecvConnect(const UDPAddressInfo& aAddressInfo) override;
   void DoSendConnectResponse(const UDPAddressInfo& aAddressInfo);
   void SendConnectResponse(nsIEventTarget *aThread,
                            const UDPAddressInfo& aAddressInfo);
   void DoConnect(nsCOMPtr<nsIUDPSocket>& aSocket,
                  nsCOMPtr<nsIEventTarget>& aReturnThread,
                  const UDPAddressInfo& aAddressInfo);
 
@@ -59,17 +60,18 @@ public:
 private:
   virtual ~UDPSocketParent();
 
   virtual void ActorDestroy(ActorDestroyReason why) override;
   void Send(const InfallibleTArray<uint8_t>& aData, const UDPSocketAddr& aAddr);
   void Send(const InputStreamParams& aStream, const UDPSocketAddr& aAddr);
   nsresult BindInternal(const nsCString& aHost, const uint16_t& aPort,
                         const bool& aAddressReuse, const bool& aLoopback,
-                        const uint32_t& recvBufferSize);
+                        const uint32_t& recvBufferSize,
+                        const uint32_t& sendBufferSize);
   nsresult ConnectInternal(const nsCString& aHost, const uint16_t& aPort);
   void FireInternalError(uint32_t aLineNo);
   void SendInternalError(nsIEventTarget *aThread,
                          uint32_t aLineNo);
 
   // One of these will be null and the other non-null.
   PBackgroundParent* mBackgroundManager;
   PNeckoParent* mNeckoManager;
--- a/dom/network/interfaces/nsIUDPSocketChild.idl
+++ b/dom/network/interfaces/nsIUDPSocketChild.idl
@@ -27,17 +27,18 @@ interface nsIUDPSocketChild : nsISupport
   attribute AUTF8String filterName;
 
   // Allow hosting this over PBackground instead of PNecko
   [noscript] void setBackgroundSpinsEvents();
 
   // Tell the chrome process to bind the UDP socket to a given local host and port
   void bind(in nsIUDPSocketInternal socket, in nsIPrincipal principal,
             in AUTF8String host, in unsigned short port,
-            in bool addressReuse, in bool loopback, in uint32_t recvBufferSize);
+            in bool addressReuse, in bool loopback, in uint32_t recvBufferSize,
+            in uint32_t sendBufferSize);
 
   // Tell the chrome process to connect the UDP socket to a given remote host and port
   void connect(in nsIUDPSocketInternal socket, in AUTF8String host, in unsigned short port);
 
   // Tell the chrome process to perform equivalent operations to all following methods
   void send(in AUTF8String host, in unsigned short port,
             [const, array, size_is(byteLength)] in uint8_t bytes,
             in unsigned long byteLength);
--- a/media/mtransport/nr_socket_prsock.cpp
+++ b/media/mtransport/nr_socket_prsock.cpp
@@ -602,30 +602,58 @@ int NrSocket::create(nr_transport_addr *
     case IPPROTO_UDP:
       if (!(fd_ = PR_OpenUDPSocket(naddr.raw.family))) {
         r_log(LOG_GENERIC,LOG_CRIT,"Couldn't create UDP socket, "
               "family=%d, err=%d", naddr.raw.family, PR_GetError());
         ABORT(R_INTERNAL);
       }
 #ifdef XP_WIN
       if (!mozilla::IsWin8OrLater()) {
-        PRSocketOptionData opt_rcvbuf;
-        opt_rcvbuf.option = PR_SockOpt_RecvBufferSize;
-        // Increase default receive buffer size on <= Win7 to be able to
-        // receive an unpaced HD (>= 720p = 1280x720 - I Frame ~ 21K size)
+        // Increase default send and receive buffer sizes on <= Win7 to be able to
+        // receive and send an unpaced HD (>= 720p = 1280x720 - I Frame ~ 21K size)
         // stream without losing packets.
         // Manual testing showed that 100K buffer size was not enough and the
         // packet loss dis-appeared with 256K buffer size.
         // See bug 1252769 for future improvements of this.
-        opt_rcvbuf.value.recv_buffer_size = 256 * 1024;
-        status = PR_SetSocketOption(fd_, &opt_rcvbuf);
-        if (status != PR_SUCCESS) {
+        PRSize min_buffer_size = 256 * 1024;
+        PRSocketOptionData opt_rcvbuf;
+        opt_rcvbuf.option = PR_SockOpt_RecvBufferSize;
+        if ((status = PR_GetSocketOption(fd_, &opt_rcvbuf)) == PR_SUCCESS) {
+          if (opt_rcvbuf.value.recv_buffer_size < min_buffer_size) {
+            opt_rcvbuf.value.recv_buffer_size = min_buffer_size;
+            if ((status = PR_SetSocketOption(fd_, &opt_rcvbuf)) != PR_SUCCESS) {
+              r_log(LOG_GENERIC, LOG_CRIT,
+                "Couldn't set socket receive buffer size: %d", status);
+            }
+          } else {
+            r_log(LOG_GENERIC, LOG_INFO,
+              "Socket receive buffer size is already: %d",
+              opt_rcvbuf.value.recv_buffer_size);
+          }
+        } else {
           r_log(LOG_GENERIC, LOG_CRIT,
-            "Couldn't set receive buffer size socket option: %d", status);
-          ABORT(R_INTERNAL);
+            "Couldn't get socket receive buffer size: %d", status);
+        }
+        PRSocketOptionData opt_sndbuf;
+        opt_sndbuf.option = PR_SockOpt_SendBufferSize;
+        if ((status = PR_GetSocketOption(fd_, &opt_sndbuf)) == PR_SUCCESS) {
+          if (opt_sndbuf.value.recv_buffer_size < min_buffer_size) {
+            opt_sndbuf.value.recv_buffer_size = min_buffer_size;
+            if ((status = PR_SetSocketOption(fd_, &opt_sndbuf)) != PR_SUCCESS) {
+              r_log(LOG_GENERIC, LOG_CRIT,
+                "Couldn't set socket send buffer size: %d", status);
+            }
+          } else {
+            r_log(LOG_GENERIC, LOG_INFO,
+              "Socket send buffer size is already: %d",
+              opt_sndbuf.value.recv_buffer_size);
+          }
+        } else {
+          r_log(LOG_GENERIC, LOG_CRIT,
+            "Couldn't get socket send buffer size: %d", status);
         }
       }
 #endif
       break;
     case IPPROTO_TCP:
       if (!(fd_ = PR_OpenTCPSocket(naddr.raw.family))) {
         r_log(LOG_GENERIC,LOG_CRIT,"Couldn't create TCP socket, "
               "family=%d, err=%d", naddr.raw.family, PR_GetError());
@@ -1477,17 +1505,17 @@ int NrUdpSocketIpc::accept(nr_transport_
   MOZ_ASSERT(false);
   return R_INTERNAL;
 }
 
 // IO thread executors
 void NrUdpSocketIpc::create_i(const nsACString &host, const uint16_t port) {
   ASSERT_ON_THREAD(io_thread_);
 
-  uint32_t recvBuffSize = 0;
+  uint32_t minBuffSize = 0;
   nsresult rv;
   nsCOMPtr<nsIUDPSocketChild> socketChild = do_CreateInstance("@mozilla.org/udp-socket-child;1", &rv);
   if (NS_FAILED(rv)) {
     ReentrantMonitorAutoEnter mon(monitor_);
     err_ = true;
     MOZ_ASSERT(false, "Failed to create UDPSocketChild");
     return;
   }
@@ -1508,30 +1536,31 @@ void NrUdpSocketIpc::create_i(const nsAC
   if (NS_FAILED(rv)) {
     err_ = true;
     mon.NotifyAll();
     return;
   }
 
 #ifdef XP_WIN
   if (!mozilla::IsWin8OrLater()) {
-    // Increase default receive buffer size on <= Win7 to be able to
-    // receive an unpaced HD (>= 720p = 1280x720 - I Frame ~ 21K size)
+    // Increase default receive and send buffer size on <= Win7 to be able to
+    // receive and send an unpaced HD (>= 720p = 1280x720 - I Frame ~ 21K size)
     // stream without losing packets.
     // Manual testing showed that 100K buffer size was not enough and the
     // packet loss dis-appeared with 256K buffer size.
     // See bug 1252769 for future improvements of this.
-    recvBuffSize = 256 * 1024;
+    minBuffSize = 256 * 1024;
   }
 #endif
   // XXX bug 1126232 - don't use null Principal!
   if (NS_FAILED(socket_child_->Bind(proxy, nullptr, host, port,
                                     /* reuse = */ false,
                                     /* loopback = */ false,
-                                    /* recv buffer size */ recvBuffSize))) {
+                                    /* recv buffer size */ minBuffSize,
+                                    /* send buffer size */ minBuffSize))) {
     err_ = true;
     MOZ_ASSERT(false, "Failed to create UDP socket");
     mon.NotifyAll();
     return;
   }
 }
 
 void NrUdpSocketIpc::connect_i(const nsACString &host, const uint16_t port) {
--- a/netwerk/base/nsIUDPSocket.idl
+++ b/netwerk/base/nsIUDPSocket.idl
@@ -261,16 +261,23 @@ interface nsIUDPSocket : nsISupports
     [noscript] attribute NetAddr multicastInterfaceAddr;
 
     /**
      * recvBufferSize
      *
      * The size of the receive buffer. Default depends on the OS.
      */
     [noscript] attribute long recvBufferSize;
+
+    /**
+     * sendBufferSize
+     *
+     * The size of the send buffer. Default depends on the OS.
+     */
+    [noscript] attribute long sendBufferSize;
 };
 
 /**
  * nsIUDPSocketListener
  *
  * This interface is notified whenever a UDP socket accepts a new connection.
  * The transport is in the connected state, and read/write streams can be opened
  * using the normal nsITransport API.  The address of the client can be found by
--- a/netwerk/base/nsUDPSocket.cpp
+++ b/netwerk/base/nsUDPSocket.cpp
@@ -1497,16 +1497,43 @@ nsUDPSocket::SetRecvBufferSize(int size)
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return NS_ERROR_FAILURE;
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
+nsUDPSocket::GetSendBufferSize(int* size)
+{
+  // Bug 1252759 - missing support for GetSocketOption
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
+NS_IMETHODIMP
+nsUDPSocket::SetSendBufferSize(int size)
+{
+  if (NS_WARN_IF(!mFD)) {
+    return NS_ERROR_NOT_INITIALIZED;
+  }
+
+  PRSocketOptionData opt;
+
+  opt.option = PR_SockOpt_SendBufferSize;
+  opt.value.send_buffer_size = size;
+
+  nsresult rv = SetSocketOption(opt);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return NS_ERROR_FAILURE;
+  }
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 nsUDPSocket::GetMulticastInterface(nsACString& aIface)
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 nsUDPSocket::GetMulticastInterfaceAddr(NetAddr* aIface)
 {