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.

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

[qpack] Add comment explaining static table #3576

Merged
merged 2 commits into from
Apr 28, 2020
Merged

Conversation

afrind
Copy link
Contributor

@afrind afrind commented Apr 13, 2020

Fixes #3562

@ianswett ianswett requested a review from bencebeky April 13, 2020 22:15
Comment on lines 1204 to 1206
this methodology, some of the entries may be inconsistent. Notably,
"content-type: text/plain; charset=utf-8" appears twice with two different
whitespace encodings (entries 52 and 54).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this methodology, some of the entries may be inconsistent. Notably,
"content-type: text/plain; charset=utf-8" appears twice with two different
whitespace encodings (entries 52 and 54).
this methodology, some of the entries may be inconsistent.

It's probably enough to observe that there are inconsistencies that derive from the source data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like we've gotten a couple questions about this one in particular, but I can remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to support @martinthomson because text/plain; charsert=utf-8- does not appear twice. The other is text/html.

@@ -1199,6 +1199,12 @@ are registered in the "HTTP/3 Error Code" registry established in {{HTTP3}}.

# Static Table

This table was generated by analyzing actual internet traffic in 2018 and
manually crafting the table for optimal compression of common headers. Due to
Copy link
Member

Choose a reason for hiding this comment

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

What does "manually crafting" mean? I think that you mean manually removing obvious redundancies and (other stuff we/Mike did).

Copy link
Member

Choose a reason for hiding this comment

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

maybe you want to include some info from #904 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

We started from an automated query for data, then:

  • Removed vendor-specific artifacts and anti-patterns as best we could
  • Sorted the entries into the three tranches described in the comment; the sizes are dictated by the QPACK instruction space, but the selection of which things go in each tranche is a mix of data and judgement.

@martinthomson martinthomson added -qpack editorial An issue that does not affect the design of the protocol; does not require consensus. labels Apr 15, 2020
@afrind afrind merged commit 5faae5c into master Apr 28, 2020
@afrind afrind deleted the qpack-static-comment branch April 28, 2020 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-qpack editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistency of QPACK static table
6 participants