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 SC: No encoder memory for blocked #4008

Merged
merged 3 commits into from Aug 18, 2020
Merged

Conversation

MikeBishop
Copy link
Contributor

Per discussion on Slack, expanding this discussion in the Security Considerations to be more accurate. Editorial, since these are purely statements about the properties of the design.

@MikeBishop MikeBishop added editorial An issue that does not affect the design of the protocol; does not require consensus. -qpack labels Aug 17, 2020
@dtikhonov
Copy link
Member

An encoder allocates memory to track all dynamic table references in unacknowledged representations. Implementations can directly limit the amount of state memory by only using as many references to the dynamic table as it wishes to track; no signaling to the decoder is required.

This is true, but this is also obvious, at least once you start implementing the encoder. Too, there is the natural limit on dynamic references: it is the function of the product of the number of allowed streams and the number of entries in the dynamic table (which itself is a function of the table size).

I am concerned that this new verbiage puts the wrong thought into the head of the implementer; we should not encourage this. Limiting the number of dynamic table references (done out of memory conservation zeal) affects compression performance directly -- it worsens it.

Perhaps we can tack on the following sentence to the end of the paragraph:

Note that limiting the number of references is likely to affect performance negatively.

@martinthomson
Copy link
Member

I'm OK with that, though I would qualify with "compression performance", as performance has other dimensions that the extra state might impinge upon.

@dtikhonov
Copy link
Member

Another thing to note is that sometimes dynamic table references do not have to have their own objects in the encoder. For examples, they are virtually free in ls-qpack.

(Everyone reading this in 2022: you're welcome)

@dtikhonov
Copy link
Member

Yeah, compression performance qualification is a good suggestion.

additional state memory on the encoder.

An encoder allocates memory to track all dynamic table references in
unacknowledged representations. Implementations can directly limit the amount
Copy link
Contributor

Choose a reason for hiding this comment

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

representations are not acknowledged, encoded field section blocks are, right?


An encoder allocates memory to track all dynamic table references in
unacknowledged representations. Implementations can directly limit the amount
of state memory by only using as many references to the dynamic table as it
Copy link
Contributor

Choose a reason for hiding this comment

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

Subject/pronoun agreement: "Implementations can...as it wishes". Perhaps "An implementation can..."

@afrind afrind merged commit f1b8397 into master Aug 18, 2020
@afrind afrind deleted the qpack/no-blocked-state branch August 18, 2020 22:16
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.

None yet

4 participants