> On 16 Nov 2021, at 15:56, Roman Arutyunyan <arut@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@nginx.com> wrote:
>>>
>>> # HG changeset patch
>>> # User Roman Arutyunyan <arut@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

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel