[PATCH 2 of 2] HTTP/3: delayed Insert Count Increment instruction

[Sergey Kandaurov]

> On 16 Nov 2021, at 15:56, Roman Arutyunyan <arut at nginx.com> wrote:
> 
> On Thu, Nov 11, 2021 at 06:20:59PM +0300, Sergey Kandaurov wrote:
>> 
>>> On 21 Oct 2021, at 16:41, Roman Arutyunyan <arut at nginx.com> wrote:
>>> 
>>> # HG changeset patch
>>> # User Roman Arutyunyan <arut at nginx.com>
>>> # Date 1634804424 -10800
>>> #      Thu Oct 21 11:20:24 2021 +0300
>>> # Branch quic
>>> # Node ID e2d65b59ccb9035cbd619358a121ba5bcca3404a
>>> # Parent  8b049432ef2dcdb8d1a8ec1a5e41c0a340285b65
>>> HTTP/3: delayed Insert Count Increment instruction.
>>> 
>>> Sending the instruction is delayed until the end of the current event cycle.
>>> Delaying the instruction is allowed by quic-qpack-21, section 2.2.2.3.
>>> The goal is to reduce the amount of data sent back to client by accumulating
>>> inserts.
>> 
>> Or it may be not sent at all if there are no blocked streams.
> 
> Sure.
> 
> HTTP/3: delayed Insert Count Increment instruction.
> 
> Sending the instruction is delayed until the end of the current event cycle.
> Delaying the instruction is allowed by quic-qpack-21, section 2.2.2.3.
> The goal is to reduce the amount of data sent back to client by accumulating
> several inserts in one instruction and sometimes not sending the instruction at
> all, if Section Acknowledgement was sent just before it.
> 

Looks fine.

>>> 
>>> diff --git a/src/http/v3/ngx_http_v3.c b/src/http/v3/ngx_http_v3.c
>>> --- a/src/http/v3/ngx_http_v3.c
>>> +++ b/src/http/v3/ngx_http_v3.c
>>> @@ -47,6 +47,10 @@ ngx_http_v3_init_session(ngx_connection_
>>>    h3c->keepalive.handler = ngx_http_v3_keepalive_handler;
>>>    h3c->keepalive.cancelable = 1;
>>> 
>>> +    h3c->table.send_insert_count.log = pc->log;
>>> +    h3c->table.send_insert_count.data = pc;
>>> +    h3c->table.send_insert_count.handler = ngx_http_v3_inc_insert_count_handler;
>>> +
>>>    cln = ngx_pool_cleanup_add(pc->pool, 0);
>>>    if (cln == NULL) {
>>>        return NGX_ERROR;
>>> @@ -85,6 +89,10 @@ ngx_http_v3_cleanup_session(void *data)
>>>    if (h3c->keepalive.timer_set) {
>>>        ngx_del_timer(&h3c->keepalive);
>>>    }
>>> +
>>> +    if (h3c->table.send_insert_count.posted) {
>>> +        ngx_delete_posted_event(&h3c->table.send_insert_count);
>>> +    }
>>> }
>>> 
>>> 
>>> diff --git a/src/http/v3/ngx_http_v3_parse.c b/src/http/v3/ngx_http_v3_parse.c
>>> --- a/src/http/v3/ngx_http_v3_parse.c
>>> +++ b/src/http/v3/ngx_http_v3_parse.c
>>> @@ -395,6 +395,8 @@ done:
>>>        if (ngx_http_v3_send_ack_section(c, c->quic->id) != NGX_OK) {
>>>            return NGX_ERROR;
>>>        }
>>> +
>>> +        ngx_http_v3_ack_insert_count(c, st->prefix.insert_count);
>>>    }
>>> 
>>>    st->state = sw_start;
>>> diff --git a/src/http/v3/ngx_http_v3_tables.c b/src/http/v3/ngx_http_v3_tables.c
>>> --- a/src/http/v3/ngx_http_v3_tables.c
>>> +++ b/src/http/v3/ngx_http_v3_tables.c
>>> @@ -232,11 +232,9 @@ ngx_http_v3_insert(ngx_connection_t *c, 
>>>    dt->elts[dt->nelts++] = field;
>>>    dt->size += size;
>>> 
>>> -    /* TODO increment can be sent less often */
>>> +    dt->insert_count++;
>>> 
>>> -    if (ngx_http_v3_send_inc_insert_count(c, 1) != NGX_OK) {
>>> -        return NGX_ERROR;
>>> -    }
>>> +    ngx_post_event(&dt->send_insert_count, &ngx_posted_events);
>>> 
>>>    if (ngx_http_v3_new_entry(c) != NGX_OK) {
>>>        return NGX_ERROR;
>>> @@ -246,6 +244,34 @@ ngx_http_v3_insert(ngx_connection_t *c, 
>>> }
>>> 
>>> 
>>> +void
>>> +ngx_http_v3_inc_insert_count_handler(ngx_event_t *ev)
>>> +{
>>> +    ngx_connection_t             *c;
>>> +    ngx_http_v3_session_t        *h3c;
>>> +    ngx_http_v3_dynamic_table_t  *dt;
>>> +
>>> +    c = ev->data;
>>> +
>>> +    ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0,
>>> +                   "http3 inc insert count handler");
>>> +
>>> +    h3c = ngx_http_v3_get_session(c);
>>> +    dt = &h3c->table;
>>> +
>>> +    if (dt->insert_count > dt->ack_insert_count) {
>>> +        if (ngx_http_v3_send_inc_insert_count(c,
>>> +                                       dt->insert_count - dt->ack_insert_count)
>>> +            != NGX_OK)
>>> +        {
>>> +            return;
>>> +        }
>>> +
>>> +        dt->ack_insert_count = dt->insert_count;
>>> +    }
>>> +}
>>> +
>>> +
>>> ngx_int_t
>>> ngx_http_v3_set_capacity(ngx_connection_t *c, ngx_uint_t capacity)
>>> {
>>> @@ -603,6 +629,21 @@ ngx_http_v3_check_insert_count(ngx_conne
>>> }
>>> 
>>> 
>>> +void
>>> +ngx_http_v3_ack_insert_count(ngx_connection_t *c, uint64_t insert_count)
>>> +{
>>> +    ngx_http_v3_session_t        *h3c;
>>> +    ngx_http_v3_dynamic_table_t  *dt;
>>> +
>>> +    h3c = ngx_http_v3_get_session(c);
>>> +    dt = &h3c->table;
>>> +
>>> +    if (dt->ack_insert_count < insert_count) {
>>> +        dt->ack_insert_count = insert_count;
>>> +    }
>>> +}
>>> +
>>> +
>>> static void
>>> ngx_http_v3_unblock(void *data)
>>> {
>>> diff --git a/src/http/v3/ngx_http_v3_tables.h b/src/http/v3/ngx_http_v3_tables.h
>>> --- a/src/http/v3/ngx_http_v3_tables.h
>>> +++ b/src/http/v3/ngx_http_v3_tables.h
>>> @@ -26,9 +26,13 @@ typedef struct {
>>>    ngx_uint_t                    base;
>>>    size_t                        size;
>>>    size_t                        capacity;
>>> +    uint64_t                      insert_count;
>>> +    uint64_t                      ack_insert_count;
>>> +    ngx_event_t                   send_insert_count;
>>> } ngx_http_v3_dynamic_table_t;
>>> 
>>> 
>>> +void ngx_http_v3_inc_insert_count_handler(ngx_event_t *ev);
>>> void ngx_http_v3_cleanup_table(ngx_http_v3_session_t *h3c);
>>> ngx_int_t ngx_http_v3_ref_insert(ngx_connection_t *c, ngx_uint_t dynamic,
>>>    ngx_uint_t index, ngx_str_t *value);
>>> @@ -46,6 +50,7 @@ ngx_int_t ngx_http_v3_decode_insert_coun
>>>    ngx_uint_t *insert_count);
>>> ngx_int_t ngx_http_v3_check_insert_count(ngx_connection_t *c,
>>>    ngx_uint_t insert_count);
>>> +void ngx_http_v3_ack_insert_count(ngx_connection_t *c, uint64_t insert_count);
>>> ngx_int_t ngx_http_v3_set_param(ngx_connection_t *c, uint64_t id,
>>>    uint64_t value);
>>> 
>> 
>> Looks good.

-- 
Sergey Kandaurov