maskit commented on a change in pull request #7405:
URL: https://github.com/apache/trafficserver/pull/7405#discussion_r562264779
##########
File path: iocore/net/SSLSessionCache.cc
##########
@@ -129,51 +129,50 @@ SSLSessionBucket::insertSession(const SSLSessionID &id,
SSL_SESSION *sess, SSL *
Debug("ssl.session_cache", "Inserting session '%s' to bucket %p.", buf,
this);
}
- MUTEX_TRY_LOCK(lock, mutex, this_ethread());
- if (!lock.is_locked()) {
+ Ptr<IOBufferData> buf;
+ Ptr<IOBufferData> buf_exdata;
+ size_t len_exdata = sizeof(ssl_session_cache_exdata);
+ buf = new_IOBufferData(buffer_size_to_index(len,
MAX_BUFFER_SIZE_INDEX), MEMALIGNED);
+ ink_release_assert(static_cast<size_t>(buf->block_size()) >= len);
+ unsigned char *loc = reinterpret_cast<unsigned char *>(buf->data());
+ i2d_SSL_SESSION(sess, &loc);
+ buf_exdata = new_IOBufferData(buffer_size_to_index(len,
MAX_BUFFER_SIZE_INDEX), MEMALIGNED);
+ ink_release_assert(static_cast<size_t>(buf_exdata->block_size()) >=
len_exdata);
+ ssl_session_cache_exdata *exdata = reinterpret_cast<ssl_session_cache_exdata
*>(buf_exdata->data());
+ // This could be moved to a function in charge of populating exdata
+ exdata->curve = (ssl == nullptr) ? 0 : SSLGetCurveNID(ssl);
+ ink_hrtime now = Thread::get_hrtime_updated();
+
+ ats_scoped_obj<SSLSession> ssl_session(new SSLSession(id, buf, len,
buf_exdata, now));
Review comment:
I mean we could use ClassAllocator or some memory pool.
##########
File path: iocore/net/SSLSessionCache.h
##########
@@ -134,18 +166,19 @@ class SSLSessionBucket
public:
SSLSessionBucket();
~SSLSessionBucket();
- void insertSession(const SSLSessionID &, SSL_SESSION *ctx, SSL *ssl);
- bool getSession(const SSLSessionID &, SSL_SESSION **ctx,
ssl_session_cache_exdata **data);
- int getSessionBuffer(const SSLSessionID &, char *buffer, int &len);
- void removeSession(const SSLSessionID &);
+ void insertSession(const SSLSessionID &sid, SSL_SESSION *sess, SSL *ssl);
+ bool getSession(const SSLSessionID &sid, SSL_SESSION **sess,
ssl_session_cache_exdata **data);
+ int getSessionBuffer(const SSLSessionID &sid, char *buffer, int &len);
+ void removeSession(const SSLSessionID &sid);
private:
/* these method must be used while hold the lock */
void print(const char *) const;
- void removeOldestSession();
+ void removeOldestSession(const std::unique_lock<std::shared_mutex> &lock);
- Ptr<ProxyMutex> mutex;
- CountQueue<SSLSession> queue;
+ mutable std::shared_mutex mutex;
+ std::unordered_map<SSLSessionID, SSLSession *, SSLSessionIDHash> bucket_data;
+ std::map<ink_hrtime, SSLSession *> bucket_data_ts;
Review comment:
I see, then std::multimap would be another option. You can keep the
number of entries as the same as the unordered_map. It still needs linear
search but should be better than deque, and it doesn't require accurate time.
So it's get_hrtime_updated vs linear search against entries that have the same
timestamp. I'm not sure which is faster.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
