Bug 1333899 - Part 3: Avoid extra copies in nsDataChannel::OpenContentStream. r=bz
☠☠ backed out by d931cdfe0da3 ☠ ☠
authorEric Rahm <erahm@mozilla.com>
Tue, 21 Feb 2017 17:56:36 -0800
changeset 373220 fbb7cc8d04b9b4baff398ef14836414862a0ed5c
parent 373219 f82fd05e7699aed68ad105fa9ecdd3fa7b7f689d
child 373221 89120b07453e8df375aa38d9dcc12f271a9b0394
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1333899
milestone54.0a1
Bug 1333899 - Part 3: Avoid extra copies in nsDataChannel::OpenContentStream. r=bz This switches over to cloning the URI without it's ref which will most likely avoid a copy. The new |ParsePathWithoutRef| is used, again to avoid needing a copy of the path substring. A pipe is no longer used for the input stream, instead we use a string stream which in most cases will be able to share the string data buffer rather than copying it. For the base64 case in the best case we only allocate the decoded buffer, worst case there's another buffer for the URL decoded data. So either 3/4 transient or 1 3/4 transient, both of which are better than 3.5. MozReview-Commit-ID: 4tYI3iyxMCl
netwerk/protocol/data/nsDataChannel.cpp
--- a/netwerk/protocol/data/nsDataChannel.cpp
+++ b/netwerk/protocol/data/nsDataChannel.cpp
@@ -3,86 +3,106 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // data implementation
 
 #include "nsDataChannel.h"
 
 #include "mozilla/Base64.h"
-#include "nsIOService.h"
 #include "nsDataHandler.h"
-#include "nsIPipe.h"
 #include "nsIInputStream.h"
-#include "nsIOutputStream.h"
 #include "nsEscape.h"
+#include "nsStringStream.h"
 
 using namespace mozilla;
 
+/**
+ * Helper for performing a fallible unescape.
+ *
+ * @param aStr The string to unescape.
+ * @param aBuffer Buffer to unescape into if necessary.
+ * @param rv Out: nsresult indicating success or failure of unescaping.
+ * @return Reference to the string containing the unescaped data.
+ */
+const nsACString& Unescape(const nsACString& aStr, nsACString& aBuffer,
+                           nsresult* rv)
+{
+    MOZ_ASSERT(rv);
+
+    bool appended = false;
+    *rv = NS_UnescapeURL(aStr.Data(), aStr.Length(), /* flags = */ 0,
+                         aBuffer, appended, mozilla::fallible);
+    if (NS_FAILED(*rv) || !appended) {
+        return aStr;
+    }
+
+    return aBuffer;
+}
+
 nsresult
 nsDataChannel::OpenContentStream(bool async, nsIInputStream **result,
                                  nsIChannel** channel)
 {
     NS_ENSURE_TRUE(URI(), NS_ERROR_NOT_INITIALIZED);
 
     nsresult rv;
 
-    nsAutoCString spec;
-    rv = URI()->GetAsciiSpec(spec);
-    if (NS_FAILED(rv)) return rv;
+    // In order to avoid potentially building up a new path including the
+    // ref portion of the URI, which we don't care about, we clone a version
+    // of the URI that does not have a ref and in most cases should share
+    // string buffers with the original URI.
+    nsCOMPtr<nsIURI> uri;
+    rv = URI()->CloneIgnoringRef(getter_AddRefs(uri));
+    if (NS_FAILED(rv))
+        return rv;
 
-    nsCString contentType, contentCharset, dataBuffer;
+    nsAutoCString path;
+    rv = uri->GetPath(path);
+    if (NS_FAILED(rv))
+        return rv;
+
+    nsCString contentType, contentCharset;
+    nsDependentCSubstring dataRange;
     bool lBase64;
-    rv = nsDataHandler::ParseURI(spec, contentType, &contentCharset,
-                                 lBase64, &dataBuffer);
+    rv = nsDataHandler::ParsePathWithoutRef(path, contentType, &contentCharset,
+                                            lBase64, &dataRange);
     if (NS_FAILED(rv))
         return rv;
 
-    NS_UnescapeURL(dataBuffer);
+    // This will avoid a copy if nothing needs to be unescaped.
+    nsAutoCString unescapedBuffer;
+    const nsACString& data = Unescape(dataRange, unescapedBuffer, &rv);
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
 
-    if (lBase64) {
+    if (lBase64 && &data == &unescapedBuffer) {
         // Don't allow spaces in base64-encoded content. This is only
         // relevant for escaped spaces; other spaces are stripped in
-        // NewURI.
-        dataBuffer.StripWhitespace();
+        // NewURI. We know there were no escaped spaces if the data buffer
+        // wasn't used in |Unescape|.
+        unescapedBuffer.StripWhitespace();
     }
-    
+
     nsCOMPtr<nsIInputStream> bufInStream;
-    nsCOMPtr<nsIOutputStream> bufOutStream;
-    
-    // create an unbounded pipe.
-    rv = NS_NewPipe(getter_AddRefs(bufInStream),
-                    getter_AddRefs(bufOutStream),
-                    nsIOService::gDefaultSegmentSize,
-                    UINT32_MAX,
-                    async, true);
-    if (NS_FAILED(rv))
-        return rv;
 
     uint32_t contentLen;
     if (lBase64) {
-        const uint32_t dataLen = dataBuffer.Length();
-        int32_t resultLen = 0;
-        if (dataLen >= 1 && dataBuffer[dataLen-1] == '=') {
-            if (dataLen >= 2 && dataBuffer[dataLen-2] == '=')
-                resultLen = dataLen-2;
-            else
-                resultLen = dataLen-1;
-        } else {
-            resultLen = dataLen;
-        }
-        resultLen = ((resultLen * 3) / 4);
+        nsAutoCString decodedData;
+        rv = Base64Decode(data, decodedData);
+        NS_ENSURE_SUCCESS(rv, rv);
 
-        nsAutoCString decodedData;
-        rv = Base64Decode(dataBuffer, decodedData);
-        NS_ENSURE_SUCCESS(rv, rv);
-        rv = bufOutStream->Write(decodedData.get(), resultLen, &contentLen);
+        contentLen = decodedData.Length();
+        rv = NS_NewCStringInputStream(getter_AddRefs(bufInStream), decodedData);
     } else {
-        rv = bufOutStream->Write(dataBuffer.get(), dataBuffer.Length(), &contentLen);
+        contentLen = data.Length();
+        rv = NS_NewCStringInputStream(getter_AddRefs(bufInStream), data);
     }
+
     if (NS_FAILED(rv))
         return rv;
 
     SetContentType(contentType);
     SetContentCharset(contentCharset);
     mContentLength = contentLen;
 
     bufInStream.forget(result);