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

QCRAM opcodes with non-byte-aligned string literals #1144

Merged
merged 5 commits into from
Feb 28, 2018

Conversation

MikeBishop
Copy link
Contributor

@MikeBishop MikeBishop commented Feb 28, 2018

Based on #1141 and sparked by a comment on that PR that I'd like to discuss separately. There are effectively four instructions in a table update:

  1. Insert with static table name reference
  2. Insert with dynamic table name reference
  3. Insert with no reference
  4. Duplicate (a.k.a. insert with name+value reference)

#1141 follows HPACK's lead in treating insert as a single instruction with internal flags describing different possible references. That leads to effectively using two-bit prefixes for (1)/(2-3), a sentinel value in the index to differentiate (2)/(3), and (4) gets a single-bit instruction code even though it's the least common. This PR treats them as four instructions and uses two-bit prefixes for each.

The impact of this is break-even for literals which reference a name from the header table. The benefit is that you save a byte on almost every header which is inserted without a name reference.

The cost is:

  • "Roughly 15 minutes of hacking (to add the extra argument)" in @martinthomson's estimation
  • An extra byte required to duplicate entries which are at indexes 64-127, modulo Duplication can use different indexing #1128.
  • It's no longer as simple to look at one bit and say that bit has a consistent meaning

@MikeBishop MikeBishop changed the title QCRAM opcodes with non-byte QCRAM opcodes with non-byte-aligned string literals Feb 28, 2018
5.2 of [RFC7541]). A value 0 is used in place of the table reference, followed
by the header field name.

### Insert Without Reference
Copy link
Member

Choose a reason for hiding this comment

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

Without Name Reference

+---+---+-----------------------+
| H | Name Length (7+) |
+---+---------------------------+
| 0 | 0 | H | Name Length (5+) |
Copy link
Member

Choose a reason for hiding this comment

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

Almost a bike shed: invert the second bit here. Then you can reasonably say that you are describing 11 (insert with static name reference), 10 (insert with dynamic name reference), 01 (insert with new name), 00 (duplication).

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'm not wholly sure that makes a difference, but I suppose there's an argument to be made that '00' is the "unique" one.

@martinthomson martinthomson mentioned this pull request Feb 28, 2018
@MikeBishop
Copy link
Contributor Author

Having separately discussed bringing back table size change, there are now five instructions, so "00" represents "uncommon things" with a third bit to differentiate duplicate versus size change. Purely by coincidence, that means size change is now identical to the HPACK instruction.

@MikeBishop MikeBishop merged commit f9083c8 into qcram_instructions Feb 28, 2018
@MikeBishop MikeBishop deleted the qcram_instructions_opcodes branch February 28, 2018 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants