You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by persiaAziz <gi...@git.apache.org> on 2016/11/08 15:55:11 UTC
[GitHub] trafficserver pull request #1212: TS-1257: [ssl_cipher_table] replace TCL Ha...
GitHub user persiaAziz opened a pull request:
https://github.com/apache/trafficserver/pull/1212
TS-1257: [ssl_cipher_table] replace TCL HashTable by unordered_map
Replace the underlying TCL hash table implementation of ssl_cipher_name_table by STL unordered_map. This PR partially addresses TS-1257. There are many places where TCL hash table is being used. I plan to replace these one by one.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/persiaAziz/trafficserver TS-1257
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/1212.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 #1212
----
commit da93bfb47b205a9d732f56c480631bd927f17316
Author: Persia Aziz <pe...@yahoo-inc.com>
Date: 2016-11-04T18:18:06Z
TS-1257: [ssl_cipher_table] replace TCL HashTable by unordered_map
----
---
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 issue #1212: TS-1257: [ssl_cipher_table] replace TCL HashTable...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the issue:
https://github.com/apache/trafficserver/pull/1212
I think the goal is to remove all TCL dependencies, not necessarily to improve performance.
---
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 #1212: TS-1257: [ssl_cipher_table] replace TCL Ha...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1212#discussion_r87022320
--- Diff: iocore/net/SSLUtils.cc ---
@@ -141,8 +142,9 @@ static int ssl_vc_index = -1;
static ink_mutex *mutex_buf = nullptr;
static bool open_ssl_initialized = false;
-RecRawStatBlock *ssl_rsb = nullptr;
-static InkHashTable *ssl_cipher_name_table = nullptr;
+RecRawStatBlock *ssl_rsb = nullptr;
+typedef std::unordered_map<const char *, intptr_t> cipherTable;
--- End diff --
This hashes on the pointer value, not on the string value. I'm not sure that OpenSSL guarantees to give the same pointer values when you fish them out of the SSL context.
---
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 #1212: TS-1257: [ssl_cipher_table] replace TCL Ha...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1212#discussion_r87417967
--- Diff: iocore/net/SSLUtils.cc ---
@@ -141,8 +142,9 @@ static int ssl_vc_index = -1;
static ink_mutex *mutex_buf = nullptr;
static bool open_ssl_initialized = false;
-RecRawStatBlock *ssl_rsb = nullptr;
-static InkHashTable *ssl_cipher_name_table = nullptr;
+RecRawStatBlock *ssl_rsb = nullptr;
+typedef std::unordered_map<const char *, intptr_t> cipherTable;
--- End diff --
That's what I thought too (we provide custom hashing functions for this in a few places), but then I tested this on a modern compiler, and it seemed to do it right. I assume C++11 had added support for hashing the char*, but now that you say this, it's quite possible my tests were wrong.
---
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.
---