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

Fix confusing indexing name #4933

Closed
wants to merge 2 commits into from
Closed

Conversation

ami-GS
Copy link
Contributor

@ami-GS ami-GS commented Aug 5, 2021

absolute index could make sense, but Indexed Field Line and Literal Field Line With Name Reference are better to use relative index, because these are said as relative index when T=0 at line 1043 and 1098.
post base index as well. it would be better than absolute index.
Line 585 said this index as Unlike in encoder instructions, relative indices in field line representations are relative to the Base at the beginning of the encoded field section

@MikeBishop MikeBishop added -qpack editorial An issue that does not affect the design of the protocol; does not require consensus. labels Aug 5, 2021
Copy link
Contributor

@MikeBishop MikeBishop left a comment

Choose a reason for hiding this comment

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

None of these changes are correct. The point of the absolute index is that it's not changing -- these are the definitions of how other types of references map to an absolute index, and which absolute indices can be expressed by each reference type.

draft-ietf-quic-qpack.md Outdated Show resolved Hide resolved
draft-ietf-quic-qpack.md Outdated Show resolved Hide resolved
@ami-GS
Copy link
Contributor Author

ami-GS commented Aug 10, 2021

How about this? should make sense more

@afrind
Copy link
Contributor

afrind commented Aug 10, 2021

The opening text for each representation is trying to state which dynamic table entries are eligible for for that representation. All three representations state that they include either a relative or post-base index in the paragraph following the diagram.

The eligibility condition is awkward because "Base" really means "Base Insert Count". Comparing indexes and counts can get confusing - eg: Absolute Index = 0 can be used in Indexed Field Line when Base (count) is 1 or greater.

So the explanation in the draft works better for me than the proposed text, but I don't love it. I'm inclined not to merge this PR.

@ami-GS
Copy link
Contributor Author

ami-GS commented Aug 11, 2021

Hmm, I don't come up with any other representation.
Simply I felt using absolute and relative in same section looks confusing.
eg: Line 1028 and 1024 explain same part in wire, but using opposite adjective.

@MikeBishop
Copy link
Contributor

1024 is a blank line, so I'm guessing you meant a different line number. 1043, maybe?

I think the confusion is that they're not describing the same thing with opposite adjectives; they're describing related but different concepts.

  • Every entry that has ever been in the table has an absolute index; see 3.2.4. Because this number grows over time, it's never directly used on the wire. It's an internal reference in each implementation.
  • In order to compress this potentially-large value, it's expressed on the wire as a relative index or a post-base index; see 3.2.5 and 3.2.6.
  • To turn a relative or post-base index into an absolute index and look up the appropriate item, you start from the most recent entry (on the Encoder stream) or the Base (within encoded field sections) and count one direction or the other.
  • The different representation types can only refer to a certain region of absolute indices relative to the Base; each representation describes the range it is able to reference at the beginning of its description. (e.g. an entry ... with an absolute index less than the value of the Base)

So every entry has an absolute index, but no instructions use absolute indices. Instead, they use relative indices. A decoder will first resolve a relative index into an absolute index, then look up that location in the table.

@ami-GS
Copy link
Contributor Author

ami-GS commented Aug 18, 2021

Maybe I get the point (Yes, line 1043 is correct)
The absolute index in absolute index ... the value of the Base. doesn't talk about index written in wire, right?
Just a general concept about absolute index used to refer entry and how is it relative Base for each field section type.

@MikeBishop
Copy link
Contributor

Right -- the absolute index is a concept used to describe how things work, but is never used directly on the wire.

@ami-GS ami-GS closed this Aug 18, 2021
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.

4 participants