trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zw...@apache.org
Subject [04/12] git commit: TS-2501 Refactor and improve performance for the case without expansions
Date Thu, 30 Jan 2014 23:31:17 GMT
TS-2501 Refactor and improve performance for the case without expansions


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/e8f18ea0
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/e8f18ea0
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/e8f18ea0

Branch: refs/heads/5.0.x
Commit: e8f18ea04e074670ba6c4d98c00edffd0d2d4a68
Parents: 8c47c09
Author: Leif Hedstrom <zwoop@apache.org>
Authored: Thu Jan 23 05:07:44 2014 -0700
Committer: Leif Hedstrom <zwoop@apache.org>
Committed: Mon Jan 27 15:29:51 2014 -0700

----------------------------------------------------------------------
 CHANGES                             |   3 +
 plugins/header_rewrite/Makefile.am  |   3 +-
 plugins/header_rewrite/expander.cc  | 138 +++++++++++++++++++++++++++++++
 plugins/header_rewrite/expander.h   |  49 +++++++++++
 plugins/header_rewrite/operator.h   | 138 -------------------------------
 plugins/header_rewrite/operators.cc |   8 +-
 plugins/header_rewrite/value.h      |  13 ++-
 7 files changed, 208 insertions(+), 144 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e8f18ea0/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 325773f..f71c4a2 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 4.2.0
 
+  *) [TS-2501] Refactor and improve performance for the case without
+   expansions. Review: Alexey Ivanov <aivanov@linkedin.com>.
+
   *) [TS-2533 ] Add three commands previously provided by traffic_shell.
 
   *) [TS-2304] Make the healthcheck plugin watch for file permission changes.

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e8f18ea0/plugins/header_rewrite/Makefile.am
----------------------------------------------------------------------
diff --git a/plugins/header_rewrite/Makefile.am b/plugins/header_rewrite/Makefile.am
index bef7840..ee17870 100644
--- a/plugins/header_rewrite/Makefile.am
+++ b/plugins/header_rewrite/Makefile.am
@@ -33,7 +33,8 @@ header_rewrite_la_SOURCES = \
   regex_helper.cc \
   resources.cc \
   ruleset.cc \
-  statement.cc
+  statement.cc \
+  expander.cc
 header_rewrite_la_LDFLAGS = $(BOOST_LDFLAGS) $(TS_PLUGIN_LDFLAGS)
 
 endif

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e8f18ea0/plugins/header_rewrite/expander.cc
----------------------------------------------------------------------
diff --git a/plugins/header_rewrite/expander.cc b/plugins/header_rewrite/expander.cc
new file mode 100644
index 0000000..258ba47
--- /dev/null
+++ b/plugins/header_rewrite/expander.cc
@@ -0,0 +1,138 @@
+/*
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+*/
+
+//////////////////////////////////////////////////////////////////////////////////////////////
+// expander.cc: Implementation of the Variable Expander base class
+//
+#include <ts/ts.h>
+#include "expander.h"
+
+
+// Helper function to cleanly get the IP as a string.
+std::string
+getIP(sockaddr const * s_sockaddr)
+{
+  const struct sockaddr_in *s_sockaddr_in;
+  const struct sockaddr_in6 *s_sockaddr_in6;
+
+  if (s_sockaddr == NULL)
+    return "";
+
+  char res[INET6_ADDRSTRLEN] = { '\0' };
+
+  switch (s_sockaddr->sa_family) {
+  case AF_INET:
+    s_sockaddr_in = reinterpret_cast<const struct sockaddr_in *>(s_sockaddr);
+    inet_ntop(s_sockaddr_in->sin_family, &s_sockaddr_in->sin_addr, res, INET_ADDRSTRLEN);
+    break;
+  case AF_INET6:
+    s_sockaddr_in6 = reinterpret_cast<const struct sockaddr_in6 *>(s_sockaddr);
+    inet_ntop(s_sockaddr_in6->sin6_family, &s_sockaddr_in6->sin6_addr, res, INET6_ADDRSTRLEN);
+    break;
+  }
+
+  return res;
+}
+
+
+// Main expander method
+std::string
+VariableExpander::expand(const Resources& res)
+{
+  std::string result;
+
+  result.reserve(512); // TODO: Can be optimized
+  result.assign(_source);
+
+  while (true) {
+    std::string::size_type start = result.find("%<");
+    if (start == std::string::npos)
+      break;
+
+    std::string::size_type end = result.find(">", start);
+    if (end == std::string::npos)
+      break;
+
+    std::string first_part = result.substr(0, start);
+    std::string last_part = result.substr(end + 1);
+
+    // Now evaluate the variable
+    std::string variable = result.substr(start, end - start + 1);
+
+    // This will be the value to replace the "variable" section of the string with
+    std::string resolved_variable = "";
+
+    // Initialize some stuff
+    TSMBuffer bufp;
+    TSMLoc hdr_loc;
+    TSMLoc url_loc;
+
+    if (variable == "%<proto>") {
+      // Protocol of the incoming request
+      if (TSHttpTxnPristineUrlGet(res.txnp, &bufp, &url_loc) == TS_SUCCESS) {
+        int len;
+        resolved_variable = TSUrlSchemeGet(bufp, url_loc, &len);
+      }
+    } else if (variable == "%<port>") {
+      // Original port of the incoming request
+      if (TSHttpTxnClientReqGet(res.txnp, &bufp, &hdr_loc) == TS_SUCCESS) {
+        if (TSHttpHdrUrlGet(bufp, hdr_loc, &url_loc) == TS_SUCCESS) {
+          std::stringstream out;
+          out << TSUrlPortGet(bufp, url_loc);
+          resolved_variable = out.str();
+          TSHandleMLocRelease(bufp, hdr_loc, url_loc);
+        }
+        TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc);
+      }
+    } else if (variable == "%<chi>") {
+      // IP address of the client's host machine
+      resolved_variable = getIP(TSHttpTxnClientAddrGet(res.txnp));
+    } else if (variable == "%<cqhl>") {
+      // The client request header length; the header length in the client request to Traffic
Server.
+      std::stringstream out;
+      out << TSHttpHdrLengthGet(res.client_bufp, res.client_hdr_loc);
+      resolved_variable = out.str();
+    } else if (variable == "%<cqhm>") {
+      // The HTTP method in the client request to Traffic Server: GET, POST, and so on (subset
of cqtx).
+      int method_len;
+      const char *methodp = TSHttpHdrMethodGet(res.client_bufp, res.client_hdr_loc, &method_len);
+      if (methodp && method_len) {
+        resolved_variable.assign(methodp, method_len);
+      }
+    } else if (variable == "%<cquup>") {
+      // The client request unmapped URL path. This field records a URL path
+      // before it is remapped (reverse proxy mode).
+      if (TSHttpTxnPristineUrlGet(res.txnp, &bufp, &url_loc) == TS_SUCCESS) {
+        int path_len;
+        const char *path = TSUrlPathGet(bufp, url_loc, &path_len);
+
+        if (path && path_len) {
+          resolved_variable.assign(path, path_len);
+        }
+        TSHandleMLocRelease(bufp, TS_NULL_MLOC, url_loc);
+      }
+    }
+
+    // TODO(SaveTheRbtz): Can be optimized
+    result.assign(first_part);
+    result.append(resolved_variable);
+    result.append(last_part);
+  }
+
+  return result;
+}

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e8f18ea0/plugins/header_rewrite/expander.h
----------------------------------------------------------------------
diff --git a/plugins/header_rewrite/expander.h b/plugins/header_rewrite/expander.h
new file mode 100644
index 0000000..cc28637
--- /dev/null
+++ b/plugins/header_rewrite/expander.h
@@ -0,0 +1,49 @@
+/*
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+*/
+
+//////////////////////////////////////////////////////////////////////////////////////////////
+// Public interface for the Variable Expander
+//
+#ifndef __EXPANDER_H__
+#define __EXPANDER_H__ 1
+
+#include <string>
+#include <ts/ts.h>
+
+#include "resources.h"
+#include "statement.h"
+#include "parser.h"
+
+#include <iostream>
+#include <arpa/inet.h>
+#include <sstream>
+
+class VariableExpander {
+public:
+  VariableExpander(const std::string &source)
+    : _source(source)
+  { }
+
+  std::string expand(const Resources& res);
+
+private:
+  std::string _source;
+};
+
+
+#endif // __EXPANDER_H

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e8f18ea0/plugins/header_rewrite/operator.h
----------------------------------------------------------------------
diff --git a/plugins/header_rewrite/operator.h b/plugins/header_rewrite/operator.h
index 5c24925..a017b35 100644
--- a/plugins/header_rewrite/operator.h
+++ b/plugins/header_rewrite/operator.h
@@ -29,9 +29,6 @@
 #include "statement.h"
 #include "parser.h"
 
-#include <iostream>
-#include <arpa/inet.h>
-#include <sstream>
 
 // Operator modifiers
 enum OperModifiers {
@@ -96,139 +93,4 @@ private:
   DISALLOW_COPY_AND_ASSIGN(OperatorHeaders);
 };
 
-
-class VariableExpander {
-private:
-  std::string _source;
-public:
-  VariableExpander(const std::string &source) :
-      _source(source) {
-  }
-
-  std::string expand(const Resources& res) {
-    std::string result;
-    result.reserve(512); // TODO: Can be optimized
-    result.assign(_source);
-
-    while (true) {
-      std::string::size_type start = result.find("%<");
-      if (start == std::string::npos)
-        break;
-
-      std::string::size_type end = result.find(">", start);
-      if (end == std::string::npos)
-        break;
-
-      std::string first_part = result.substr(0, start);
-      std::string last_part = result.substr(end + 1);
-
-      // Now evaluate the variable
-      std::string variable = result.substr(start, end - start + 1);
-
-      // This will be the value to replace the "variable" section of the string with
-      std::string resolved_variable = "";
-
-      // Initialize some stuff
-      TSMBuffer bufp;
-      TSMLoc hdr_loc;
-      TSMLoc url_loc;
-
-      if (variable == "%<proto>") {
-        // Protocol of the incoming request
-        if (TSHttpTxnPristineUrlGet(res.txnp, &bufp, &url_loc) == TS_SUCCESS) {
-          int len;
-          resolved_variable = TSUrlSchemeGet(bufp, url_loc, &len);
-        }
-      } else if (variable == "%<port>") {
-        // Original port of the incoming request
-        if (TSHttpTxnClientReqGet(res.txnp, &bufp, &hdr_loc) == TS_SUCCESS) {
-          if (TSHttpHdrUrlGet(bufp, hdr_loc, &url_loc) == TS_SUCCESS) {
-            std::stringstream out;
-            out << TSUrlPortGet(bufp, url_loc);
-            resolved_variable = out.str();
-            TSHandleMLocRelease(bufp, hdr_loc, url_loc);
-          }
-          TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc);
-        }
-      } else if (variable == "%<chi>") {
-        // IP address of the client's host machine
-        resolved_variable = getIP(TSHttpTxnClientAddrGet(res.txnp));
-      } else if (variable == "%<cqhl>") {
-        // The client request header length; the header length in the client request to Traffic
Server.
-        std::stringstream out;
-        out << TSHttpHdrLengthGet(res.client_bufp, res.client_hdr_loc);
-        resolved_variable = out.str();
-      } else if (variable == "%<cqhm>") {
-        // The HTTP method in the client request to Traffic Server: GET, POST, and so on
(subset of cqtx).
-        int method_len;
-        const char *methodp = TSHttpHdrMethodGet(res.client_bufp, res.client_hdr_loc, &method_len);
-        if (methodp && method_len) {
-          resolved_variable.assign(methodp, method_len);
-        }
-      } else if (variable == "%<cquup>") {
-        // The client request unmapped URL path. This field records a URL path
-        // before it is remapped (reverse proxy mode).
-        if (TSHttpTxnPristineUrlGet(res.txnp, &bufp, &url_loc) == TS_SUCCESS) {
-          int path_len;
-          const char *path = TSUrlPathGet(bufp, url_loc, &path_len);
-
-          if (path && path_len) {
-            resolved_variable.assign(path, path_len);
-          }
-          TSHandleMLocRelease(bufp, TS_NULL_MLOC, url_loc);
-        }
-      }
-
-      // TODO(SaveTheRbtz): Can be optimized
-      result.assign(first_part);
-      result.append(resolved_variable);
-      result.append(last_part);
-    }
-
-    return result;
-  }
-
-private:
-  std::string getIP(sockaddr const * s_sockaddr) {
-    const struct sockaddr_in *s_sockaddr_in;
-    const struct sockaddr_in6 *s_sockaddr_in6;
-
-    if (s_sockaddr == NULL)
-      return "";
-
-    char res[INET6_ADDRSTRLEN] = { '\0' };
-
-    switch (s_sockaddr->sa_family) {
-      case AF_INET:
-        s_sockaddr_in = reinterpret_cast<const struct sockaddr_in *>(s_sockaddr);
-        inet_ntop(s_sockaddr_in->sin_family, &s_sockaddr_in->sin_addr, res, INET_ADDRSTRLEN);
-        break;
-      case AF_INET6:
-        s_sockaddr_in6 = reinterpret_cast<const struct sockaddr_in6 *>(s_sockaddr);
-        inet_ntop(s_sockaddr_in6->sin6_family, &s_sockaddr_in6->sin6_addr, res,
INET6_ADDRSTRLEN);
-        break;
-    }
-
-    return std::string(res);
-  }
-
-  std::string getURL(const Resources& res) {
-    TSMBuffer bufp;
-    TSMLoc url_loc;
-
-    if (TSHttpTxnPristineUrlGet(res.txnp, &bufp, &url_loc) != TS_SUCCESS)
-      return "";
-
-    int url_len;
-    char *url = TSUrlStringGet(bufp, url_loc, &url_len);
-    std::string ret;
-    if (url && url_len)
-      ret.assign(url, url_len);
-    TSfree(url);
-    TSHandleMLocRelease(bufp, TS_NULL_MLOC, url_loc);
-
-    return ret;
-  }
-};
-
 #endif // __OPERATOR_H

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e8f18ea0/plugins/header_rewrite/operators.cc
----------------------------------------------------------------------
diff --git a/plugins/header_rewrite/operators.cc b/plugins/header_rewrite/operators.cc
index 9aa7ebe..d6a60e5 100644
--- a/plugins/header_rewrite/operators.cc
+++ b/plugins/header_rewrite/operators.cc
@@ -24,6 +24,7 @@
 #include <string.h>
 
 #include "operators.h"
+#include "expander.h"
 
 // OperatorConfig
 void
@@ -386,8 +387,11 @@ OperatorAddHeader::exec(const Resources& res) const
 
   _value.append_value(value, res);
 
-  VariableExpander ve(value);
-  value = ve.expand(res);
+  if (_value.need_expansion()) {
+    VariableExpander ve(value);
+
+    value = ve.expand(res);
+  }
 
   // Never set an empty header (I don't think that ever makes sense?)
   if (value.empty()) {

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e8f18ea0/plugins/header_rewrite/value.h
----------------------------------------------------------------------
diff --git a/plugins/header_rewrite/value.h b/plugins/header_rewrite/value.h
index cbdd7f8..9fb5856 100644
--- a/plugins/header_rewrite/value.h
+++ b/plugins/header_rewrite/value.h
@@ -42,10 +42,10 @@ class Value : Statement
 {
 public:
   Value()
-    : _value(""), _int_value(0), _float_value(0.0), _cond_val(NULL)
+    : _need_expander(false), _value(""), _int_value(0), _float_value(0.0), _cond_val(NULL)
   {
     TSDebug(PLUGIN_NAME_DBG, "Calling CTOR for Value");
-  };
+  }
 
   void
   set_value(const std::string& val)
@@ -58,6 +58,10 @@ public:
       if (_cond_val) {
         _cond_val->initialize(parser);
       }
+    } else if (_value.find("%<") != std::string::npos) { // It has a Variable to expand
+      _need_expander = true; // And this is clearly not an integer or float ...
+      // TODO: This is still not optimal, we should pre-parse the _value string here,
+      // and perhaps populate a per-Value VariableExpander that holds state.
     } else {
       _int_value = strtol(_value.c_str(), NULL, 10);
       _float_value = strtod(_value.c_str(), NULL);
@@ -65,7 +69,8 @@ public:
   }
 
   void
-  append_value(std::string& s, const Resources& res) const {
+  append_value(std::string& s, const Resources& res) const
+  {
     if (_cond_val) {
       _cond_val->append_value(s, res);
     } else {
@@ -79,10 +84,12 @@ public:
   double get_float_value() const { return _float_value; }
 
   bool empty() const { return _value.empty(); }
+  bool need_expansion() const { return _need_expander; }
 
 private:
   DISALLOW_COPY_AND_ASSIGN(Value);
 
+  bool _need_expander;
   std::string _value;
   int _int_value;
   double _float_value;


Mime
View raw message