You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by jacksontj <gi...@git.apache.org> on 2016/05/13 02:57:10 UTC
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
GitHub user jacksontj opened a pull request:
https://github.com/apache/trafficserver/pull/634
TS-4439 Reimplement TSHttpTxnConfigFind
This needs some cleanup (probably better names, and probably should move the PAIR struct out to the header file), but I want to get some feedback on switching to something more sane.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jacksontj/trafficserver TS-4439
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/634.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #634
----
commit 75ef9bbecc1bf4ee8950f5486493c5fb2971e306
Author: Thomas Jackson <ja...@gmail.com>
Date: 2016-05-13T02:54:16Z
TS-4439 Reimplement TSHttpTxnConfigFind
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-218945006
Test response (manually).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/634#discussion_r63218410
--- Diff: proxy/InkAPI.cc ---
@@ -8175,471 +8177,163 @@ TSHttpTxnConfigStringGet(TSHttpTxn txnp, TSOverridableConfigKey conf, const char
return TS_SUCCESS;
}
+struct TXN_CONFIG_PAIR {
+ TSOverridableConfigKey cnf;
+ TSRecordDataType typ;
+ TXN_CONFIG_PAIR(TSOverridableConfigKey cnf = TS_CONFIG_NULL, TSRecordDataType typ = TS_RECORDDATATYPE_INT)
+ {
+ this->cnf = cnf;
+ this->typ = typ ? typ : TS_RECORDDATATYPE_INT;
+ };
+};
+
+std::unordered_map<std::string, TXN_CONFIG_PAIR>
+create_map()
+{
+ std::unordered_map<std::string, TXN_CONFIG_PAIR> m;
+
+ m["proxy.config.body_factory.template_base"] = TXN_CONFIG_PAIR(TS_CONFIG_BODY_FACTORY_TEMPLATE_BASE, TS_RECORDDATATYPE_STRING);
+ m["proxy.config.http.accept_encoding_filter_enabled"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_ACCEPT_ENCODING_FILTER_ENABLED);
+ m["proxy.config.http.anonymize_insert_client_ip"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_ANONYMIZE_INSERT_CLIENT_IP);
+ m["proxy.config.http.anonymize_remove_client_ip"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_ANONYMIZE_REMOVE_CLIENT_IP);
+ m["proxy.config.http.anonymize_remove_cookie"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_ANONYMIZE_REMOVE_COOKIE);
+ m["proxy.config.http.anonymize_remove_from"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_ANONYMIZE_REMOVE_FROM);
+ m["proxy.config.http.anonymize_remove_referer"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_ANONYMIZE_REMOVE_REFERER);
+ m["proxy.config.http.anonymize_remove_user_agent"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_ANONYMIZE_REMOVE_USER_AGENT);
+ m["proxy.config.http.attach_server_session_to_client"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_ATTACH_SERVER_SESSION_TO_CLIENT);
+ m["proxy.config.http.auth_server_session_private"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_AUTH_SERVER_SESSION_PRIVATE);
+ m["proxy.config.http.background_fill_active_timeout"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_BACKGROUND_FILL_ACTIVE_TIMEOUT);
+ m["proxy.config.http.background_fill_completed_threshold"] =
+ TXN_CONFIG_PAIR(TS_CONFIG_HTTP_BACKGROUND_FILL_COMPLETED_THRESHOLD, TS_RECORDDATATYPE_FLOAT);
+ m["proxy.config.http.cache.cache_responses_to_cookies"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_CACHE_RESPONSES_TO_COOKIES);
+ m["proxy.config.http.cache.cache_urls_that_look_dynamic"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_CACHE_URLS_THAT_LOOK_DYNAMIC);
+ m["proxy.config.http.cache.cluster_cache_local"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_CLUSTER_CACHE_LOCAL);
+ m["proxy.config.http.cache.fuzz.min_time"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_FUZZ_MIN_TIME);
+ m["proxy.config.http.cache.fuzz.probability"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_FUZZ_PROBABILITY, TS_RECORDDATATYPE_FLOAT);
+ m["proxy.config.http.cache.fuzz.time"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_FUZZ_TIME);
+ m["proxy.config.http.cache.generation"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_GENERATION);
+ m["proxy.config.http.cache.guaranteed_max_lifetime"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_GUARANTEED_MAX_LIFETIME);
+ m["proxy.config.http.cache.guaranteed_min_lifetime"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_GUARANTEED_MIN_LIFETIME);
+ m["proxy.config.http.cache.heuristic_lm_factor"] =
+ TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_HEURISTIC_LM_FACTOR, TS_RECORDDATATYPE_FLOAT);
+ m["proxy.config.http.cache.heuristic_max_lifetime"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_HEURISTIC_MAX_LIFETIME);
+ m["proxy.config.http.cache.heuristic_min_lifetime"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_HEURISTIC_MIN_LIFETIME);
+ m["proxy.config.http.cache.http"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_HTTP);
+ m["proxy.config.http.cache.ignore_authentication"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_IGNORE_AUTHENTICATION);
+ m["proxy.config.http.cache.ignore_client_cc_max_age"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_IGNORE_CLIENT_CC_MAX_AGE);
+ m["proxy.config.http.cache.ignore_client_no_cache"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_IGNORE_CLIENT_NO_CACHE);
+ m["proxy.config.http.cache.ignore_server_no_cache"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_IGNORE_SERVER_NO_CACHE);
+ m["proxy.config.http.cache.ims_on_client_no_cache"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_IMS_ON_CLIENT_NO_CACHE);
+ m["proxy.config.http.cache.max_open_read_retries"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_MAX_OPEN_READ_RETRIES);
+ m["proxy.config.http.cache.max_open_write_retries"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_MAX_OPEN_WRITE_RETRIES);
+ m["proxy.config.http.cache.max_stale_age"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_MAX_STALE_AGE);
+ m["proxy.config.http.cache.open_read_retry_time"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_OPEN_READ_RETRY_TIME);
+ m["proxy.config.http.cache.open_write_fail_action"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_OPEN_WRITE_FAIL_ACTION);
+ m["proxy.config.http.cache.range.lookup"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_RANGE_LOOKUP);
+ m["proxy.config.http.cache.range.write"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_RANGE_WRITE);
+ m["proxy.config.http.cache.required_headers"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_REQUIRED_HEADERS);
+ m["proxy.config.http.cache.when_to_revalidate"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CACHE_WHEN_TO_REVALIDATE);
+ m["proxy.config.http.chunking_enabled"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CHUNKING_ENABLED);
+ m["proxy.config.http.chunking.size"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CHUNKING_SIZE);
+ m["proxy.config.http.connect_attempts_max_retries_dead_server"] =
+ TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CONNECT_ATTEMPTS_MAX_RETRIES_DEAD_SERVER);
+ m["proxy.config.http.connect_attempts_max_retries"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CONNECT_ATTEMPTS_MAX_RETRIES);
+ m["proxy.config.http.connect_attempts_rr_retries"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CONNECT_ATTEMPTS_RR_RETRIES);
+ m["proxy.config.http.connect_attempts_timeout"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_CONNECT_ATTEMPTS_TIMEOUT);
+ m["proxy.config.http.default_buffer_size"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_DEFAULT_BUFFER_SIZE);
+ m["proxy.config.http.default_buffer_water_mark"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_DEFAULT_BUFFER_WATER_MARK);
+ m["proxy.config.http.doc_in_cache_skip_dns"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_DOC_IN_CACHE_SKIP_DNS);
+ m["proxy.config.http.down_server.abort_threshold"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_DOWN_SERVER_ABORT_THRESHOLD);
+ m["proxy.config.http.down_server.cache_time"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_DOWN_SERVER_CACHE_TIME);
+ m["proxy.config.http.flow_control.enabled"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_FLOW_CONTROL_ENABLED);
+ m["proxy.config.http.flow_control.high_water"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_FLOW_CONTROL_HIGH_WATER_MARK);
+ m["proxy.config.http.flow_control.low_water"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_FLOW_CONTROL_LOW_WATER_MARK);
+ m["proxy.config.http.forward.proxy_auth_to_parent"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_FORWARD_PROXY_AUTH_TO_PARENT);
+ m["proxy.config.http.global_user_agent_header"] =
+ TXN_CONFIG_PAIR(TS_CONFIG_HTTP_GLOBAL_USER_AGENT_HEADER, TS_RECORDDATATYPE_STRING);
+ m["proxy.config.http.insert_age_in_response"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_INSERT_AGE_IN_RESPONSE);
+ m["proxy.config.http.insert_request_via_str"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_INSERT_REQUEST_VIA_STR);
+ m["proxy.config.http.insert_response_via_str"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_INSERT_RESPONSE_VIA_STR);
+ m["proxy.config.http.insert_squid_x_forwarded_for"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_INSERT_SQUID_X_FORWARDED_FOR);
+ m["proxy.config.http.keep_alive_enabled_in"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_KEEP_ALIVE_ENABLED_IN);
+ m["proxy.config.http.keep_alive_enabled_out"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_KEEP_ALIVE_ENABLED_OUT);
+ m["proxy.config.http.keep_alive_no_activity_timeout_in"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_KEEP_ALIVE_NO_ACTIVITY_TIMEOUT_IN);
+ m["proxy.config.http.keep_alive_no_activity_timeout_out"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_KEEP_ALIVE_NO_ACTIVITY_TIMEOUT_OUT);
+ m["proxy.config.http.keep_alive_post_out"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_KEEP_ALIVE_POST_OUT);
+ m["proxy.config.http.negative_caching_enabled"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_NEGATIVE_CACHING_ENABLED);
+ m["proxy.config.http.negative_caching_lifetime"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_NEGATIVE_CACHING_LIFETIME);
+ m["proxy.config.http.negative_revalidating_enabled"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_NEGATIVE_REVALIDATING_ENABLED);
+ m["proxy.config.http.negative_revalidating_lifetime"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_NEGATIVE_REVALIDATING_LIFETIME);
+ m["proxy.config.http.normalize_ae_gzip"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_NORMALIZE_AE_GZIP);
+ m["proxy.config.http.number_of_redirections"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_NUMBER_OF_REDIRECTIONS);
+ m["proxy.config.http.origin_max_connections_queue"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_ORIGIN_MAX_CONNECTIONS_QUEUE);
+ m["proxy.config.http.origin_max_connections"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_ORIGIN_MAX_CONNECTIONS);
+ m["proxy.config.http.parent_proxy.total_connect_attempts"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_PARENT_PROXY_TOTAL_CONNECT_ATTEMPTS);
+ m["proxy.config.http.post.check.content_length.enabled"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_POST_CHECK_CONTENT_LENGTH_ENABLED);
+ m["proxy.config.http.post_connect_attempts_timeout"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_POST_CONNECT_ATTEMPTS_TIMEOUT);
+ m["proxy.config.http.redirection_enabled"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_ENABLE_REDIRECTION);
+ m["proxy.config.http.redirect_use_orig_cache_key"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_REDIRECT_USE_ORIG_CACHE_KEY);
+ m["proxy.config.http.request_header_max_size"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_REQUEST_HEADER_MAX_SIZE);
+ m["proxy.config.http.response_header_max_size"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_RESPONSE_HEADER_MAX_SIZE);
+ m["proxy.config.http.response_server_enabled"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_RESPONSE_SERVER_ENABLED);
+ m["proxy.config.http.response_server_str"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_RESPONSE_SERVER_STR, TS_RECORDDATATYPE_STRING);
+ m["proxy.config.http.send_http11_requests"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_SEND_HTTP11_REQUESTS);
+ m["proxy.config.http.server_session_sharing.match"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_SERVER_SESSION_SHARING_MATCH);
+ m["proxy.config.http.server_tcp_init_cwnd"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_SERVER_TCP_INIT_CWND);
+ m["proxy.config.http.slow.log.threshold"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_SLOW_LOG_THRESHOLD);
+ m["proxy.config.http.transaction_active_timeout_in"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_TRANSACTION_ACTIVE_TIMEOUT_IN);
+ m["proxy.config.http.transaction_active_timeout_out"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_TRANSACTION_ACTIVE_TIMEOUT_OUT);
+ m["proxy.config.http.transaction_no_activity_timeout_in"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_TRANSACTION_NO_ACTIVITY_TIMEOUT_IN);
+ m["proxy.config.http.transaction_no_activity_timeout_out"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_TRANSACTION_NO_ACTIVITY_TIMEOUT_OUT);
+ m["proxy.config.http.uncacheable_requests_bypass_parent"] = TXN_CONFIG_PAIR(TS_CONFIG_HTTP_UNCACHEABLE_REQUESTS_BYPASS_PARENT);
+ m["proxy.config.net.sock_option_flag_out"] = TXN_CONFIG_PAIR(TS_CONFIG_NET_SOCK_OPTION_FLAG_OUT);
+ m["proxy.config.net.sock_packet_mark_out"] = TXN_CONFIG_PAIR(TS_CONFIG_NET_SOCK_PACKET_MARK_OUT);
+ m["proxy.config.net.sock_packet_tos_out"] = TXN_CONFIG_PAIR(TS_CONFIG_NET_SOCK_PACKET_TOS_OUT);
+ m["proxy.config.net.sock_recv_buffer_size_out"] = TXN_CONFIG_PAIR(TS_CONFIG_NET_SOCK_RECV_BUFFER_SIZE_OUT);
+ m["proxy.config.net.sock_send_buffer_size_out"] = TXN_CONFIG_PAIR(TS_CONFIG_NET_SOCK_SEND_BUFFER_SIZE_OUT);
+ m["proxy.config.ssl.hsts_include_subdomains"] = TXN_CONFIG_PAIR(TS_CONFIG_SSL_HSTS_INCLUDE_SUBDOMAINS);
+ m["proxy.config.ssl.hsts_max_age"] = TXN_CONFIG_PAIR(TS_CONFIG_SSL_HSTS_MAX_AGE);
+ m["proxy.config.url_remap.pristine_host_hdr"] = TXN_CONFIG_PAIR(TS_CONFIG_URL_REMAP_PRISTINE_HOST_HDR);
+ m["proxy.config.websocket.active_timeout"] = TXN_CONFIG_PAIR(TS_CONFIG_WEBSOCKET_ACTIVE_TIMEOUT);
+ m["proxy.config.websocket.no_activity_timeout"] = TXN_CONFIG_PAIR(TS_CONFIG_WEBSOCKET_NO_ACTIVITY_TIMEOUT);
+
+ // m[""] = TXN_CONFIG_PAIR();
+
+ return m;
+};
+
+std::unordered_map<std::string, TXN_CONFIG_PAIR> TXN_CONFIG_LOOKUP_TABLE = create_map();
+
+TXN_CONFIG_PAIR *
+TXN_CONFIG_LOOKUP(std::string name)
--- End diff --
If you want to keep this, it should be static. I think that this is only used in one place and small enough just to inline it into ``TSHttpTxnConfigFind``.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219077066
That seems like something we should include in some sort of "contributor" documentation in the root of the repo :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/634#discussion_r63216041
--- Diff: proxy/InkAPI.cc ---
@@ -8175,471 +8177,163 @@ TSHttpTxnConfigStringGet(TSHttpTxn txnp, TSOverridableConfigKey conf, const char
return TS_SUCCESS;
}
+struct TXN_CONFIG_PAIR {
--- End diff --
We don't use all-caps for types or functions, just macros.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219070741
@zwoop We do have unordered_map in the core (https://github.com/apache/trafficserver/blob/master/proxy/logstats.cc#L55).
I also guess I don't quite understand the reasoning for not wanting STL-- as long as its used correctly (meaning overriden allocators etc. or static things). Similarly a STL map is used in hostdb for the hosts file implementation (and I seem to remember having a similar discussion there).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219104889
Well, static / no-static, it still needs a container from lib/ts.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-218946224
[approve ci]
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by Thomas Jackson <ja...@gmail.com>.
We already pull stl in the core in a couple other places. And to Brian's
point since this is effectively a constant (never mutates) the allocations
don't matter.
On May 13, 2016 6:21 AM, "zwoop" <gi...@git.apache.org> wrote:
> Github user zwoop commented on the pull request:
>
>
> https://github.com/apache/trafficserver/pull/634#issuecomment-219040807
>
> Sudheer brings up a valid point. We should use the hash tables that we
> have in lib/ts for core code.
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219040807
Sudheer brings up a valid point. We should use the hash tables that we have in lib/ts for core code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219066268
Adding comment from email (apparently Githib's auto-comment email isn't in our emails to dev@):
We already pull stl in the core in a couple other places. And to @bgaff 's point since this is effectively a constant (never mutates) the allocations don't matter.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219107725
The recipe in `docs/developer/config-vars.en.rst` needs to be updated for this new structure.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-218945356
Yeah, this seems nicer.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219075424
@SolidWallOfCode Seems like you are the keeper of the maps-- do you have docs or examples I can follow to use different maps?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by bgaff <gi...@git.apache.org>.
Github user bgaff commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-218960894
If you're going to use STL you should probably incorporate the stl allocators, or if we're going to settle on jemalloc then it doesn't matter
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by sudheerv <gi...@git.apache.org>.
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-218950857
Are we now okay with letting the use of STL in the core?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/634#discussion_r63216086
--- Diff: proxy/InkAPI.cc ---
@@ -8175,471 +8177,163 @@ TSHttpTxnConfigStringGet(TSHttpTxn txnp, TSOverridableConfigKey conf, const char
return TS_SUCCESS;
}
+struct TXN_CONFIG_PAIR {
+ TSOverridableConfigKey cnf;
+ TSRecordDataType typ;
+ TXN_CONFIG_PAIR(TSOverridableConfigKey cnf = TS_CONFIG_NULL, TSRecordDataType typ = TS_RECORDDATATYPE_INT)
+ {
+ this->cnf = cnf;
+ this->typ = typ ? typ : TS_RECORDDATATYPE_INT;
+ };
+};
+
+std::unordered_map<std::string, TXN_CONFIG_PAIR>
--- End diff --
This should be ``static``.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219068283
No, we do not use unordered_map in our core. Until we decide otherwise (on mailing lists), no STL in the core. The STL that is in there was put in for no good reason, and we have Jira's to remove them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by sudheerv <gi...@git.apache.org>.
Github user sudheerv commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219075656
Just fyi -
https://mail-archives.apache.org/mod_mbox/trafficserver-dev/201507.mbox/%3CA16410D3-9E7B-4042-A819-F361BFD5002C%40apache.org%3E
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219071582
Okay, I guess I'm misunderstanding what "core" is, that is in proxy (same dir as this) and its also in "iocore" (which has core right in the name). Where exactly are we not allowed to use STL (and is that documented somewhere?)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219070850
That is not in the core.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-218945321
Yeah, this seems nicer.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219098651
Basically anything that is in trafficserver which would be considered the critical path. The big exception here are plug-ins, because in many cases there is no better option. But we should avoid STL there too when possible. At least until we can prove that STL is not going to impact performance. And it's more than just allocation, it's really easy to write slow STL based code :).
Fwiw, your use case is likely fine from performance but sets a bad precedence.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-219063157
[approve ci]
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/634#issuecomment-218944885
Just a test [approve ci]
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/634#discussion_r63216314
--- Diff: proxy/InkAPI.cc ---
@@ -8175,471 +8177,163 @@ TSHttpTxnConfigStringGet(TSHttpTxn txnp, TSOverridableConfigKey conf, const char
return TS_SUCCESS;
}
+struct TXN_CONFIG_PAIR {
+ TSOverridableConfigKey cnf;
+ TSRecordDataType typ;
+ TXN_CONFIG_PAIR(TSOverridableConfigKey cnf = TS_CONFIG_NULL, TSRecordDataType typ = TS_RECORDDATATYPE_INT)
+ {
+ this->cnf = cnf;
+ this->typ = typ ? typ : TS_RECORDDATATYPE_INT;
+ };
+};
+
+std::unordered_map<std::string, TXN_CONFIG_PAIR>
--- End diff --
Why static? Do you mean in the anonymous namespace to avoid linkage visibility?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4439 Reimplement TSHttpTxnConfigFin...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/634#discussion_r63218243
--- Diff: proxy/InkAPI.cc ---
@@ -8175,471 +8177,163 @@ TSHttpTxnConfigStringGet(TSHttpTxn txnp, TSOverridableConfigKey conf, const char
return TS_SUCCESS;
}
+struct TXN_CONFIG_PAIR {
+ TSOverridableConfigKey cnf;
+ TSRecordDataType typ;
+ TXN_CONFIG_PAIR(TSOverridableConfigKey cnf = TS_CONFIG_NULL, TSRecordDataType typ = TS_RECORDDATATYPE_INT)
+ {
+ this->cnf = cnf;
+ this->typ = typ ? typ : TS_RECORDDATATYPE_INT;
+ };
+};
+
+std::unordered_map<std::string, TXN_CONFIG_PAIR>
+create_map()
+{
+ std::unordered_map<std::string, TXN_CONFIG_PAIR> m;
--- End diff --
It think this would be clearer and more readable if you defined a static const array of entries.
```C
struct entry {
const char * name;
TSOverridableConfigKey key;
TSRecordDataType dtype;
} values[] = {
{ "proxy.config.body_factory.template_base", TS_CONFIG_BODY_FACTORY_TEMPLATE_BASE, TS_RECORDDATATYPE_STRING},
...
};
```
When you create the map, just loop over the array and insert pointer without copying any values.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---