Fix performance issues with folly::Uri::authority().
authorStephane Sezer <sas@fb.com>
Wed, 25 Sep 2013 17:50:46 +0000 (10:50 -0700)
committerPeter Griess <pgriess@fb.com>
Tue, 15 Oct 2013 01:43:30 +0000 (18:43 -0700)
Summary: String concatenation was not performed correctly in folly::Uri::authority(). This commit fixes the issue by pre-allocating enough space and buliding the string left to right.

Test Plan: Unit tests in folly/test.

Reviewed By: tudorb@fb.com

FB internal diff: D981069

folly/Uri.cpp
folly/test/UriTest.cpp

index c2bd85a2fe81db95dc303e583cb9f691eed443ad..9b1151a8f114e3f37de5266bfa1553da7ce1aaf0 100644 (file)
@@ -92,23 +92,28 @@ Uri::Uri(StringPiece str) : port_(0) {
   fragment_ = submatch(match, 4);
 }
 
-fbstring
-Uri::authority() const
-{
-  fbstring result(host());
+fbstring Uri::authority() const {
+  fbstring result;
 
-  if (port() != 0) {
-    result += fbstring(":") + to<fbstring>(port());
-  }
+  // Port is 5 characters max and we have up to 3 delimiters.
+  result.reserve(host().size() + username().size() + password().size() + 8);
 
-  if (!username().empty()) {
-    fbstring userInformation(username());
+  if (!username().empty() || !password().empty()) {
+    result.append(username());
 
     if (!password().empty()) {
-      userInformation += fbstring(":") + password();
+      result.push_back(':');
+      result.append(password());
     }
 
-    result = userInformation + "@" + result;
+    result.push_back('@');
+  }
+
+  result.append(host());
+
+  if (port() != 0) {
+    result.push_back(':');
+    toAppend(port(), &result);
   }
 
   return result;
index 97b14637f5f53c470ed60537f78e3fea2bb97eae..e4fe76996bc8fa27cf5552da97d10aae7d088708 100644 (file)
@@ -34,6 +34,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("", u.password());
     EXPECT_EQ("www.facebook.com", u.host());
     EXPECT_EQ(0, u.port());
+    EXPECT_EQ("www.facebook.com", u.authority());
     EXPECT_EQ("/hello/world", u.path());
     EXPECT_EQ("query", u.query());
     EXPECT_EQ("fragment", u.fragment());
@@ -48,6 +49,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("", u.password());
     EXPECT_EQ("www.facebook.com", u.host());
     EXPECT_EQ(8080, u.port());
+    EXPECT_EQ("www.facebook.com:8080", u.authority());
     EXPECT_EQ("/hello/world", u.path());
     EXPECT_EQ("query", u.query());
     EXPECT_EQ("fragment", u.fragment());
@@ -62,6 +64,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("", u.password());
     EXPECT_EQ("127.0.0.1", u.host());
     EXPECT_EQ(8080, u.port());
+    EXPECT_EQ("127.0.0.1:8080", u.authority());
     EXPECT_EQ("/hello/world", u.path());
     EXPECT_EQ("query", u.query());
     EXPECT_EQ("fragment", u.fragment());
@@ -76,6 +79,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("", u.password());
     EXPECT_EQ("[::1]", u.host());
     EXPECT_EQ(8080, u.port());
+    EXPECT_EQ("[::1]:8080", u.authority());
     EXPECT_EQ("/hello/world", u.path());
     EXPECT_EQ("query", u.query());
     EXPECT_EQ("fragment", u.fragment());
@@ -90,6 +94,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("pass", u.password());
     EXPECT_EQ("host.com", u.host());
     EXPECT_EQ(0, u.port());
+    EXPECT_EQ("user:pass@host.com", u.authority());
     EXPECT_EQ("/", u.path());
     EXPECT_EQ("", u.query());
     EXPECT_EQ("", u.fragment());
@@ -104,6 +109,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("", u.password());
     EXPECT_EQ("host.com", u.host());
     EXPECT_EQ(0, u.port());
+    EXPECT_EQ("user@host.com", u.authority());
     EXPECT_EQ("/", u.path());
     EXPECT_EQ("", u.query());
     EXPECT_EQ("", u.fragment());
@@ -118,6 +124,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("", u.password());
     EXPECT_EQ("host.com", u.host());
     EXPECT_EQ(0, u.port());
+    EXPECT_EQ("user@host.com", u.authority());
     EXPECT_EQ("/", u.path());
     EXPECT_EQ("", u.query());
     EXPECT_EQ("", u.fragment());
@@ -132,6 +139,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("pass", u.password());
     EXPECT_EQ("host.com", u.host());
     EXPECT_EQ(0, u.port());
+    EXPECT_EQ(":pass@host.com", u.authority());
     EXPECT_EQ("/", u.path());
     EXPECT_EQ("", u.query());
     EXPECT_EQ("", u.fragment());
@@ -146,6 +154,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("", u.password());
     EXPECT_EQ("host.com", u.host());
     EXPECT_EQ(0, u.port());
+    EXPECT_EQ("host.com", u.authority());
     EXPECT_EQ("/", u.path());
     EXPECT_EQ("", u.query());
     EXPECT_EQ("", u.fragment());
@@ -160,6 +169,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("", u.password());
     EXPECT_EQ("host.com", u.host());
     EXPECT_EQ(0, u.port());
+    EXPECT_EQ("host.com", u.authority());
     EXPECT_EQ("/", u.path());
     EXPECT_EQ("", u.query());
     EXPECT_EQ("", u.fragment());
@@ -174,6 +184,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("", u.password());
     EXPECT_EQ("", u.host());
     EXPECT_EQ(0, u.port());
+    EXPECT_EQ("", u.authority());
     EXPECT_EQ("/etc/motd", u.path());
     EXPECT_EQ("", u.query());
     EXPECT_EQ("", u.fragment());
@@ -188,6 +199,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("", u.password());
     EXPECT_EQ("", u.host());
     EXPECT_EQ(0, u.port());
+    EXPECT_EQ("", u.authority());
     EXPECT_EQ("/etc/motd", u.path());
     EXPECT_EQ("", u.query());
     EXPECT_EQ("", u.fragment());
@@ -202,6 +214,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("", u.password());
     EXPECT_EQ("etc", u.host());
     EXPECT_EQ(0, u.port());
+    EXPECT_EQ("etc", u.authority());
     EXPECT_EQ("/motd", u.path());
     EXPECT_EQ("", u.query());
     EXPECT_EQ("", u.fragment());