Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use "Insert Count" rather than "Largest Reference" #2111

Merged
merged 14 commits into from Jan 10, 2019
Merged

Use "Insert Count" rather than "Largest Reference" #2111

Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
79f819f
Use "Insert Count" rather than "Largest Reference"
martinthomson Dec 11, 2018
3da4d1f
Some editorial changes
martinthomson Dec 31, 2018
3a43469
Base editorial changes
martinthomson Dec 31, 2018
9ccc5ca
Text format description for header block prefix
martinthomson Dec 31, 2018
c6296d4
Change absolute indices to 0-based too
martinthomson Dec 31, 2018
39f9930
>=Base
martinthomson Dec 31, 2018
7654602
Mike's comments
martinthomson Jan 7, 2019
21199c4
Drop the 'maybe later'
martinthomson Jan 8, 2019
bcca2eb
the Base, fix some off-by-one errors
martinthomson Jan 9, 2019
7281fc2
More off-by-one errors
martinthomson Jan 10, 2019
43a826c
Insert Count Delta for the Table State Synchronize instruction
martinthomson Jan 10, 2019
8d8f334
Merge branch 'master' into insert-count
martinthomson Jan 10, 2019
6077a99
Use Insert Count Increment rather than Table State Synchronize
martinthomson Jan 10, 2019
f35caef
a -> an
afrind Jan 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 23 additions & 23 deletions draft-ietf-quic-qpack.md
View file
Open in desktop
Expand Up @@ -481,14 +481,14 @@ the first entry added after the Base, see {{post-base}}.
~~~~~ drawing
Required
Insert
Count Base
| |
V V
+---+-----+-----+-----+-------+
| n | n-1 | n-2 | ... | d | Absolute Index
+---+-----+ - +-----+ - +
| 0 | ... | n-d-3 | Relative Index
+-----+-----+-------+
Count Base
| |
V V
+-----+-----+-----+-----+-------+
| n-1 | n-2 | n-3 | ... | d | Absolute Index
+-----+-----+ - +-----+ - +
| 0 | ... | n-d-3 | Relative Index
+-----+-----+-------+

n = count of entries inserted
d = count of entries dropped
Expand All @@ -507,14 +507,14 @@ as absolute indices, with the zero value being the first entry inserted after
the Base.

~~~~~ drawing
Base
|
V
+---+-----+-----+-----+-----+
| n | n-1 | n-2 | ... | d | Absolute Index
+---+-----+-----+-----+-----+
| 1 | 0 | Post-Base Index
+---+-----+
Base
|
V
+-----+-----+-----+-----+-----+
| n-1 | n-2 | n-3 | ... | d | Absolute Index
+-----+-----+-----+-----+-----+
| 1 | 0 | Post-Base Index
+-----+-----+

n = count of entries inserted
d = count of entries dropped
Expand Down Expand Up @@ -713,21 +713,21 @@ The Table State Synchronize instruction begins with the '00' two-bit pattern.
The instruction specifies the total number of dynamic table inserts and
duplications since the last Table State Synchronize or Header Acknowledgement
that increased the Known Received Count for the dynamic table (see
{{known-received-count}}). This is encoded as a 6-bit prefix integer. The
encoder uses this value to determine which table entries might cause a stream to
become blocked, as described in {{state-synchronization}}.
{{known-received-count}}). The Insert Count Delta is encoded as a 6-bit prefix
integer. The encoder uses this value to determine which table entries might
cause a stream to become blocked, as described in {{state-synchronization}}.

~~~~~~~~~~ drawing
0 1 2 3 4 5 6 7
+---+---+---+---+---+---+---+---+
| 0 | 0 | Insert Count (6+) |
| 0 | 0 |Insert Count Delta (6+)|
+---+---+-----------------------+
~~~~~~~~~~
{:#fig-size-sync title="Table State Synchronize"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


An encoder that receives an Insert Count equal to zero or one that increases
Known Received Count beyond what the encoder has sent MUST treat this as a
connection error of type `HTTP_QPACK_DECODER_STREAM_ERROR`.
An encoder that receives an Insert Count Delta equal to zero or one that
increases the Known Received Count beyond what the encoder has sent MUST treat
this as a connection error of type `HTTP_QPACK_DECODER_STREAM_ERROR`.

### Header Acknowledgement

Expand Down