[ https://issues.apache.org/jira/browse/TS-2503?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14174550#comment-14174550 ]
Bryan Call edited comment on TS-2503 at 10/17/14 1:13 AM: ---------------------------------------------------------- 1. It was hard to see the do while from the patch. I would then put a conditional around most of the code at the top (SSLConfigParams::ssl_maxrecord == -1) and comment it. I would like to avoid calling ink_get_hrtime_internal if the code is not going to use now. 2. Comment the l -= (l % SSL_MAX_TLS_RECORD_SIZE); since 2 of us didn't know what you were doing. 3. sslTotalBytesSent should be incremented by r not total_wrote. The value would be incremented by too many bytes. 4. indentation is sometimes 4 spaces and sometimes 2 spaces example of how the code at the top can be cleaned up: {code} // dynamic tls record size uint32_t dynamic_tls_record_size = 0; ink_hrtime now = ink_get_hrtime_internal(); int msec_since_last_write = 0; if (SSLConfigParams::ssl_maxrecord == -1) { ink_hrtime_to_msec(now - sslLastWriteTime); if (msec_since_last_write > SSL_DEF_TLS_RECORD_MSEC_THRESHOLD) { // reset sslTotalBytesSent after 1 sec inactivity sslTotalBytesSent = 0; Debug("ssl", "SSLNetVConnection::loadBufferAndCallWrite, now %" PRId64 ",lastwrite %" PRId64 " ,msec_since_last_write %d", now, sslLastWriteTime, msec_since_last_write); } {code} was (Author: bcall): 1. It was hard to see the do while from the patch. I would then put a conditional around most of the code at the top (SSLConfigParams::ssl_maxrecord == -1) and comment it. I would like to avoid calling ink_get_hrtime_internal if the code is not going to use now. 2. Comment the l -= (l % SSL_MAX_TLS_RECORD_SIZE); since 2 of us didn't know what you were doing. 3. sslTotalBytesSent should be incremented by r not total_wrote. The value would be incremented by too many bytes. 4. indentation is sometimes 4 spaces and sometimes 2 spaces > dynamic TLS record size tuning > ------------------------------ > > Key: TS-2503 > URL: https://issues.apache.org/jira/browse/TS-2503 > Project: Traffic Server > Issue Type: Improvement > Components: Performance, SSL > Reporter: James Peach > Assignee: Sudheer Vinukonda > Fix For: 5.2.0 > > Attachments: TS-2503.diff > > > From [~igrigorik] in TS-2365: > {quote} > FWIW, I think you may be interested in this discussion: > - http://mailman.nginx.org/pipermail/nginx-devel/2013-December/004703.html > - http://mailman.nginx.org/pipermail/nginx-devel/2014-January/004748.html > In a nutshell, static record size introduces an inherent tradeoff between > latency and throughput -- smaller records are good for latency, but hurt > server throughput by adding bytes and CPU overhead. It would be great if we > could implement a smarter strategy in ATS. The extra benefit is that it's one > less knob to tune: the out-of-the-box experience would be better optimized > for all ATS users, regardless of mix/type of traffic being proxies. > {quote} -- This message was sent by Atlassian JIRA (v6.3.4#6332)