Subject: Re: [quicwg/base-drafts] QPACK [editorial] Misc minor editorial changes. (#2942)

From: Mike Bishop <[email-masked]>
Date: Fri, 16 Aug 2019 12:00:37 -0700

MikeBishop approved this pull request.

A few nitpicks, but these are all good improvements.  Thanks.

>  table entries which have been acknowledged, but this could mean using
 literals. Since literals make the header block larger, this can result in the
 encoder becoming blocked on congestion or flow control limits.
 
 ### Known Received Count
 
-In order to identify which dynamic table entries can be safely used without a
-stream becoming blocked, the encoder tracks the number of entries received by
-the decoder.  The Known Received Count tracks the total number of acknowledged
-insertions.
+The Known Received Count is the total number of dynamic table insertions and
+duplications acknowledged by the decoder.  The encoder tracks the Known Received
+Count in order to identify which dynamic table entries can be references without

```suggestion
Count in order to identify which dynamic table entries can be referenced without
```

> @@ -445,19 +443,23 @@ encoder sends a Set Dynamic Table Capacity instruction
 ({{set-dynamic-capacity}}) with a non-zero capacity to begin using the dynamic
 table.
 
-Before a new entry is added to the dynamic table, entries are evicted from the

Well, if you're running close to the table's maximum size, presumably that's an indicator that the other side doesn't ever want the table larger than that, which implies evict-then-add.  However, this is also a holdover from HPACK where adding something larger than the max table size evicted everything, then left you with an empty table when the add failed.  Since that's now a hard error, I'm comfortable framing this as purely implementation choice.

> @@ -445,19 +443,23 @@ encoder sends a Set Dynamic Table Capacity instruction
 ({{set-dynamic-capacity}}) with a non-zero capacity to begin using the dynamic
 table.
 
-Before a new entry is added to the dynamic table, entries are evicted from the
-end of the dynamic table until the size of the dynamic table is less than or
-equal to (table capacity - size of new entry). The encoder MUST NOT evict a
-blocking dynamic table entry (see {{blocked-insertion}}).  The entry is then
-added to the table.  It is an error if the encoder attempts to add an entry that
-is larger than the dynamic table capacity; the decoder MUST treat this as a
+When a new entry is added to the dynamic table, the minimum number of entries
+are evicted from the end of the dynamic table so that the final table size
+including the new entry does not exceed the table capacity.  This can be
+achieved by adding the new entry and then removing entries from the end until
+the table size is less than or equal to the table capacity.  Another option is
+to first evict entries until the size of the dynamic table is less than or equal
+to (table capacity - size of new entry), then add the new entry.  Note that a
+new entry can reference an entry in the dynamic table that will be evicted when
+adding this new entry.  Therefore implementations utilizing the latter option

```suggestion
adding this new entry.  Therefore, implementations utilizing the latter option
```

> @@ -477,28 +479,27 @@ it can choose to use a lower dynamic table capacity (see
 
 For clients using 0-RTT data in HTTP/3, the server's maximum table capacity is
 the remembered value of the setting, or zero if the value was not previously
-sent.  When the client's 0-RTT value of the SETTING is 0, the server MAY set it

The usual rule is to write out numbers less than ten.  However, I'm okay either way when we're talking about the literal value of something.  I defer to @afrind's style preferences.

-An endpoint MUST allow its peer to create both encoder and decoder streams

Would "an encoder stream and a decoder stream" make this clearer?

> @@ -830,10 +831,8 @@ modifying it.
 
 Each header block is prefixed with two integers.  The Required Insert Count is
 encoded as an integer with an 8-bit prefix after the encoding described in
-{{ric}}).  The Base is encoded as sign-and-modulus integer, using a single sign
-bit and a value with a 7-bit prefix (see {{base}}).
-
-These two values are followed by representations for compressed headers.
+{{ric}}).  The Base is encoded as a sign bit, and a Delta Base value with a

```suggestion
{{ric}}).  The Base is encoded as a sign bit and a Delta Base value with a
```
Comma makes it looks like Delta Base follows the sign-bit-encoded Base.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2942#pullrequestreview-276111748

---

From: Bence Béky <[email-masked]>
Date: Mon, 19 Aug 2019 07:43:34 -0700

bencebeky commented on this pull request.

Thank you for the review.

 ### Known Received Count

Thank you.

Thank you.

Thank you.

Just for the record, I do not feel strongly, and I'm happy to incorporate whatever @afrind suggests.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2942#pullrequestreview-276604747

---

From: Bence Béky <[email-masked]>
Date: Mon, 19 Aug 2019 07:50:44 -0700

@bencebeky pushed 1 commit.

0ca3bf7d6dddc32879371691c6eccc2666f88210  Apply suggestions from code review

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2942/files/7bc269a3b73015041ad2e8ea95164aff14d64f49..0ca3bf7d6dddc32879371691c6eccc2666f88210

---

From: Bence Béky <[email-masked]>
Date: Mon, 19 Aug 2019 07:54:25 -0700

d29333ebc1ac11a02c5835d821a1d8cc5a939e9a  MikeBishop's wording suggestion for allowing creation of an encoder stream and a decoder stream.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2942/files/0ca3bf7d6dddc32879371691c6eccc2666f88210..d29333ebc1ac11a02c5835d821a1d8cc5a939e9a

---

From: Bence Béky <[email-masked]>
Date: Mon, 19 Aug 2019 07:54:33 -0700

Yes, thank you for the suggestion.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2942#discussion_r315254933