Subject: Re: [quicwg/base-drafts] Use "Insert Count" rather than "Largest Reference" (#2111)

From: Mike Bishop <[email-masked]>
Date: Mon, 07 Jan 2019 15:18:53 -0800

MikeBishop commented on this pull request.

> @@ -784,29 +798,29 @@ streams emit the headers for an HTTP request or response.
 
 ### Header Data Prefix {#header-prefix}
 
-Header data is prefixed with two integers, `Largest Reference` and `Base Index`.
+Header data is prefixed with two integers, `Required Insert Count` and `Base`.

The guidance that I've previously received is that illustrations are helpful, but not normative, and the text therefore has to convey everything that's required to implement.

-- 
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/2111#discussion_r245831403

---

From: Mike Bishop <[email-masked]>
Date: Mon, 07 Jan 2019 15:42:09 -0800

MikeBishop approved this pull request.

In general, I think this is a good improvement.  You seem to switch from using "Base" to using "the Base" part-way through, so I'd pick one and switch the others.

> @@ -444,33 +452,40 @@ referenced by a given relative index will change while interpreting instructions
 on the encoder stream.
 
 ~~~~~ drawing
-    +---+---------------+-----------+
-    | n |      ...      |   d + 1   |  Absolute Index
-    + - +---------------+ - - - - - +
-    | 0 |      ...      | n - d - 1 |  Relative Index
-    +---+---------------+-----------+
-      ^                       |
-      |                       V
-Insertion Point         Dropping Point
+      +---+---------------+-----------+
+      | n |      ...      |     d     |  Absolute Index

If Absolute Index is zero-based, then the Absolute Index of the most recently inserted item would be n-1.

-Header data is prefixed with two integers, `Largest Reference` and `Base Index`.
+Each header block is prefixed with two integers.  The Required Insert Count
+encoded as an integer with an 8-bit prefix after the encoding described in

**is** encoded, presumably.

>  ~~~
 
-TotalNumberOfInserts is the total number of inserts into the decoder's
-dynamic table.  This encoding limits the length of the prefix on
-long-lived connections.
+Where TotalNumberOfInserts is the total number of inserts into the decoder's

I know that the actual intent is to continue the previous sentence (...using the following algorithm, where....), but this reads as a dangling phrase.  If you want the "where," maybe move the whole thing above the algorithm?

 A header block that does not reference the dynamic table can use any value for
-Base Index; setting both Largest Reference and Base Index to zero is the most
+Base; setting both Required Insert Count and Delta Base to zero is the most
 efficient encoding.

NYT but isn't it required to set a correct RIC?

> -Because frames from request streams can be delivered out of order with
-instructions on the encoder stream, relative indices are relative to the Base
-Index at the beginning of the header block (see {{header-prefix}}). The Base
-Index is an absolute index. When interpreting the rest of the frame, the entry
-identified by Base Index has a relative index of zero.  The relative indices of
-entries do not change while interpreting headers on a request or push stream.
+Unlike on the encoder stream, relative indices on push and request streams are
+relative to the Base at the beginning of the header block (see
+{{header-prefix}}). This ensures that references are stable even if the dynamic
+table is updated while decoding a header block.
+
+The Base is encoded as a value relative to the Required Insert Count. The Base
+identifies which dynamic table entries can be referenced using relative
+indexing, starting with 0 at the last entry added.
+
+Post-base references are used for entries inserted after base, starting at 0 for

"Base" is capitalized elsewhere

-- 
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/2111#pullrequestreview-190039015

---

From: Martin Thomson <[email-masked]>
Date: Mon, 07 Jan 2019 15:54:08 -0800

@martinthomson pushed 1 commit.

7654602d9122c45675f5b6e26000a8d20fb2fadc  Mike's comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2111/files/39f9930445e718bb0f34746a0e77c13328e989c2..7654602d9122c45675f5b6e26000a8d20fb2fadc

---

From: Martin Thomson <[email-masked]>
Date: Tue, 08 Jan 2019 14:17:44 -0800

21199c4c127ab1dc80e9b5d949a30bc4e1df845e  Drop the 'maybe later'

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2111/files/7654602d9122c45675f5b6e26000a8d20fb2fadc..21199c4c127ab1dc80e9b5d949a30bc4e1df845e

---

From: afrind <[email-masked]>
Date: Tue, 08 Jan 2019 16:47:33 -0800

afrind commented on this pull request.

I'm not crazy about just 'Base'.  Does 'Base Count' or 'Base Insert Count' read any better to you?

 When blocking references are permitted, the encoder uses header block
-acknowledgement to identify the Largest Known Received index, as described in
+acknowledgement to identify the Known Received Count, as described in

s/identify/maintain/ ?

 ~~~~~ drawing
-             Base Index
-                 |
-                 V
+ Required
+  Insert
+  Count      Base
+    |         |
+    V         V

I would still have these point at the boxes (where the numbers are) rather than the lines.

 ~~~~~ drawing
-             Base Index
-                 |
-                 V
+ Required
+  Insert
+  Count      Base
+    |         |
+    V         V

Also, not sure we need Required Insert Count in this drawing.  The section is describing relative indexing so it's not really adding anything?

-`Largest Reference` identifies the largest absolute dynamic index referenced in
-the block.  Blocking decoders use the Largest Reference to determine when it is
-safe to process the rest of the block.  If Largest Reference is greater than
-zero, the encoder transforms it as follows before encoding:
+#### Required Insert Count {#ric}
+
+Required Insert Count identifies the state of the dynamic table needed to
+process the header block successfully.  Blocking decoders use the Required

Does 'successfully' add much here?

> @@ -923,10 +961,9 @@ header field name matches the header field name of an entry stored in the static
 table or the dynamic table.
 
 If the entry is in the static table, or in the dynamic table with an absolute
-index less than or equal to Base Index, this representation starts with the '01'
-two-bit pattern.  If the entry is in the dynamic table with an absolute index
-greater than Base Index, the representation starts with the '0000' four-bit
-pattern.
+index less than the Base, this representation starts with the '01' two-bit
+pattern.  If the entry is in the dynamic table with an absolute index greater
+than the Base, the representation starts with the '0000' four-bit pattern.

Something is off here because it doesn't say what to do if Base and the Absolute Index of the entry are equal.  If Base is the Insert Count when encoding started, then we need to use post-base indexing if the entry is greater than or equal to base.

 ### Literal Header Field With Post-Base Name Reference
 
-For entries in the dynamic table with an absolute index greater than Base Index,
-the header field name is represented using the post-base index of that entry
-(see {{post-base-indexing}}) encoded as an integer with a 3-bit prefix.
+For entries in the dynamic table with an absolute index greater than or equal to
+the Base, the header field name is represented using the post-base index of that

or equal?  per above.

-- 
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/2111#pullrequestreview-190497904

---

From: Martin Thomson <[email-masked]>
Date: Tue, 08 Jan 2019 18:26:59 -0800

martinthomson commented on this pull request.

 ~~~~~ drawing
-             Base Index
-                 |
-                 V
+ Required
+  Insert
+  Count      Base
+    |         |
+    V         V

https://blog.nelhage.com/2015/08/indices-point-between-elements/ is what I'm thinking.  That meaning that you can get something like this:

```
| a | b | c | d |
        ^

c is index 0 normal
b is index 0 post-base
both use the same pointer
```

-- 
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/2111#discussion_r246242147

---

From: Martin Thomson <[email-masked]>
Date: Tue, 08 Jan 2019 18:35:13 -0800

bcca2eba0c0740f8255fee1cf373830c2fd5f5f0  the Base, fix some off-by-one errors

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2111/files/21199c4c127ab1dc80e9b5d949a30bc4e1df845e..bcca2eba0c0740f8255fee1cf373830c2fd5f5f0

---

From: Martin Thomson <[email-masked]>
Date: Tue, 08 Jan 2019 18:39:03 -0800

I think that I got all the "the Base" updated.

I don't know if this needs to be a more specific term.  Using "count" in the name is somewhat right, but it's a concept that probably doesn't need to be globally unique as it is only used in the scope of decoding header blocks.

I'll leave that to editorial discretion though, that's just my opinion.

-- 
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/2111#pullrequestreview-190554593

---

From: afrind <[email-masked]>
Date: Wed, 09 Jan 2019 16:37:21 -0800

>      +---+-----+-----+-----+-------+
-    | n | n-1 | n-2 | ... |  d+1  |  Absolute Index
+    | n | n-1 | n-2 | ... |   d   |  Absolute Index

If the absolute index is 0 based, then shouldn't the most recent entry be 'n - 1'?.  Eg: if Insert Count is 1 then the entry's absolute index is 0.

>      +---+-----+-----+-----+-----+
-    | n | n-1 | n-2 | ... | d+1 |  Absolute Index
+    | n | n-1 | n-2 | ... |  d  |  Absolute Index

Same here

> @@ -708,17 +726,17 @@ become blocked, as described in {{state-synchronization}}.
 {:#fig-size-sync title="Table State Synchronize"}

The above table still uses 'Insert Count', which might confuse some readers now that we've defined this term above to mean something else.  What this instruction sends is a Insert Count Delta, or something?

-- 
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/2111#pullrequestreview-191003748

---

From: Martin Thomson <[email-masked]>
Date: Wed, 09 Jan 2019 16:46:23 -0800

Yep, that's a good idea, though it pushes the limits of what fits in the little box :)  I wanted Insert Count Increment, but that very much doesn't fit.

-- 
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/2111#discussion_r246603394

---

From: Martin Thomson <[email-masked]>
Date: Wed, 09 Jan 2019 16:50:15 -0800

@martinthomson pushed 2 commits.

7281fc27a796dc00bc0e4a7bab277c06513ec894  More off-by-one errors
43a826cbbf9c32a7ebaed40da7ed7fba27cef56c  Insert Count Delta for the Table State Synchronize instruction

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2111/files/bcca2eba0c0740f8255fee1cf373830c2fd5f5f0..43a826cbbf9c32a7ebaed40da7ed7fba27cef56c

---

From: Martin Thomson <[email-masked]>
Date: Thu, 10 Jan 2019 00:50:30 +0000 (UTC)

Incidentally, does this work?  https://github.com/quicwg/base-drafts/compare/insert-count...insert-count-increment?expand=1

-- 
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/2111#discussion_r246604064

---

From: Martin Thomson <[email-masked]>
Date: Wed, 09 Jan 2019 16:51:48 -0800

8d8f334773da8d14b218defe3efb5a538677f451  Merge branch 'master' into insert-count

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2111/files/43a826cbbf9c32a7ebaed40da7ed7fba27cef56c..8d8f334773da8d14b218defe3efb5a538677f451

---

From: afrind <[email-masked]>
Date: Thu, 10 Jan 2019 01:36:11 +0000 (UTC)

Insert Count Increment is much more specific than Table State Synchronize.  The description is good.  Sadly, it doesn't have the same ring to it, but I suppose clarity is more important.  I don't really like Delta, Increment is better.  We could also keep TSS and just put Increment or IC Increment or something in the little box.

-- 
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/2111#discussion_r246611498

---

From: Martin Thomson <[email-masked]>
Date: Wed, 09 Jan 2019 17:46:19 -0800

6077a99452c0799812261e2f3729b14286e4e2d7  Use Insert Count Increment rather than Table State Synchronize

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2111/files/8d8f334773da8d14b218defe3efb5a538677f451..6077a99452c0799812261e2f3729b14286e4e2d7

---

From: Martin Thomson <[email-masked]>
Date: Wed, 09 Jan 2019 17:46:33 -0800

"Synchronize" is fancy word, yeah :)  But accurate >> fancy.  I'll make the proposed change.

-- 
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/2111#discussion_r246613048

---

From: Mike Bishop <[email-masked]>
Date: Thu, 10 Jan 2019 08:29:53 -0800

Same as above, yes.

-- 
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/2111#discussion_r246828927

---

From: afrind <[email-masked]>
Date: Thu, 10 Jan 2019 09:14:56 -0800

@afrind pushed 1 commit.

f35caef0210c8aeed99741cd4bc75347e1da4a22  a -> an

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2111/files/6077a99452c0799812261e2f3729b14286e4e2d7..f35caef0210c8aeed99741cd4bc75347e1da4a22

---

From: afrind <[email-masked]>
Date: Thu, 10 Jan 2019 17:53:22 +0000 (UTC)

Merged #2111 into master.

-- 
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/2111#event-2066563723