> On Nov 16, 2015, at 12:38 PM, Yann Ylavic <ylavic....@gmail.com> wrote:
>
> On Mon, Nov 16, 2015 at 5:53 PM, jean-frederic clere <jfcl...@gmail.com>
> wrote:
>> On 01/09/2015 09:37 PM, jaillet...@apache.org wrote:
>>>
>>> Modified: httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c?rev=1650655&r1=1650654&r2=1650655&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c (original)
>>> +++ httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c Fri Jan 9
>>> 20:37:50 2015
>>> @@ -1683,6 +1683,9 @@ PROXY_DECLARE(char *) ap_proxy_define_wo
>>>
>>> memset(wshared, 0, sizeof(proxy_worker_shared));
>>>
>>> + if (uri.port && uri.port == ap_proxy_port_of_scheme(uri.scheme)) {
>>> + uri.port = 0;
>>> + }
>>
>>
>> I must be doing something wrong but the above seems to hurt by 30% more
>> CPU one for the tests we are doing, any hints?
>
> Looks like you are quite heavy testing this path...
>
> Could you please try the following patch and see if it helps?
>
> Index: modules/proxy/proxy_util.c
> ===================================================================
> --- modules/proxy/proxy_util.c (revision 1714617)
> +++ modules/proxy/proxy_util.c (working copy)
> @@ -3663,35 +3663,51 @@ PROXY_DECLARE(int) ap_proxy_pass_brigade(apr_bucke
> return OK;
> }
>
> -/* Fill in unknown schemes from apr_uri_port_of_scheme() */
>
> -typedef struct proxy_schemes_t {
> - const char *name;
> - apr_port_t default_port;
> -} proxy_schemes_t ;
> -
> -static proxy_schemes_t pschemes[] =
> -{
> - {"fcgi", 8000},
> - {"ajp", AJP13_DEF_PORT},
> - {"scgi", SCGI_DEF_PORT},
> - { NULL, 0xFFFF } /* unknown port */
> -};
> -
> PROXY_DECLARE(apr_port_t) ap_proxy_port_of_scheme(const char *scheme)
> {
> if (scheme) {
> - apr_port_t port;
> - if ((port = apr_uri_port_of_scheme(scheme)) != 0) {
> - return port;
> - } else {
> - proxy_schemes_t *pscheme;
> - for (pscheme = pschemes; pscheme->name != NULL; ++pscheme) {
> - if (strcasecmp(scheme, pscheme->name) == 0) {
> - return pscheme->default_port;
> + /* Fill in unknown schemes from apr_uri_port_of_scheme(),
> + * and try a fast path for common schemes.
> + */
> + switch (ap_tolower(*scheme)) {
> + case 'a':
> + if (strcasecmp(scheme + 1, "jp") == 0) {
> + return AJP13_DEF_PORT;
> + }
> + break;
> + case 'f':
> + if (strcasecmp(scheme + 1, "cgi") == 0) {
> + return 8000;
> + }
> + if (strcasecmp(scheme + 1, "tp") == 0) {
> + return APR_URI_FTP_DEFAULT_PORT;
> + }
> + break;
> + case 'h':
> + if (strncasecmp(scheme + 1, "ttp", 4) == 0) {
Should be 3, shouldn't it?
> + int s4 = ap_tolower(scheme[4]);
> + if (!s4) {
> + return APR_URI_HTTP_DEFAULT_PORT;
> }
> + else if (s4 == 's' && !scheme[5]) {
> + return APR_URI_HTTPS_DEFAULT_PORT;
> + }
> }
If we are really trying to optimize the heck out of this, certainly
we should not ap_tolower s4 until we know it's not '\0'.
> + break;
> + case 'n':
> + if (strcasecmp(scheme + 1, "ntp") == 0) {
> + return APR_URI_NNTP_DEFAULT_PORT;
> + }
> + break;
> + case 's':
> + if (strcasecmp(scheme + 1, "cgi") == 0) {
> + return SCGI_DEF_PORT;
> + }
> + break;
> }
> +
> + return apr_uri_port_of_scheme(scheme);
> }
> return 0;
> }
> --
>
> Regards,
> Yann.
