[ 
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:14 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



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


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}

> 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)

Reply via email to