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

[Sergey Kandaurov]

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

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