1255239 - Reduce size of HuffmanIncomingTables

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Nicholas Nethercote [inactive]]

Http2HuffmanIncoming.h has 15 HuffmanIncomingEntry arrays, each with 256
entries. On 64-bit platforms they take up 60 KiB, on 32-bit it's 30 KiB.
They are currently in the .data section which means they aren't shared and the
60/30 KiB is repeated for every process.

We can do much better. Each table has two kinds of entries:

- most/all entries: a value and a prefixLen;
- zero/a few entries: a pointer to another table;

We currently use a "fat" representation for each entry which has space for all
three fields. So every entry either looks like this:

> { nullptr, 33, 2 },

or like this:

> { &HuffmanIncoming_254, 0, 0 },

It's possible to split each table into two parts: a first part with all the
value/prefixLen entries, and a second, much smaller (possibly zero-sized) part
with all the pointer entries. This avoids the unused field(s) for each entry.

It's also possible to use bitfields to pack the value and prefixLen into 16
bits, instead of taking up 24 (which ends up as 32 once padding is added).

It's also possible to make all these tables |const|, which moves the entry 
tables out of the .data section into the .rodata section -- the removal of the
pointers was also necessary for this -- which means they will be shared between
processes instead of each process having its own copy.

So the overall saving from these changes on 64-bit is about (60*N - 7.5) KiB,
where N is the number of processes. On 32-bit it's about (30*N - 7.5) KiB.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Reduce size of HuffmanIncomingTables

I have been as careful as I could with this change, but I don't know how this
code is used, how well tested it is, and how any errors would manifest. I will
do a try push shortly.

Comment 2

[Patrick McManus [:mcmanus]]

Comment on attachment 8728726 [details] [diff] [review]
Reduce size of HuffmanIncomingTables

Review of attachment 8728726 [details] [diff] [review]:
-----------------------------------------------------------------

there are xpcshell tests that should exercise this.. thanks!

Comment 3

[u408661]

Comment on attachment 8728726 [details] [diff] [review]
Reduce size of HuffmanIncomingTables

Review of attachment 8728726 [details] [diff] [review]:
-----------------------------------------------------------------

This all looks good - just one request. Please also update make_incoming_tables.py in the same directory so that its output matches your modifications. The chances of having to re-generate the .h is pretty slim, but if we do something similar for future versions of HTTP, it would be nice to have an easier path towards auto-generation.

Also, just for safety's sake, I'd like to see a linux64-only, xpcshell-only try run before this lands - the huffman code has been known to be subtle in the past (not my finest coding, to be sure), so more safety = better :)

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: Reduce size of HuffmanIncomingTables

Note that I took advantage of the fact that C and C++ allow trailing commas in
struct initialization. E.g. see
http://stackoverflow.com/questions/7043372/int-a-1-2-weird-comma-allowed-any-particular-reason

Comment 5

[Nicholas Nethercote [inactive]]

And try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=365f98501bb4

Comment 6

[u408661]

Comment on attachment 8729238 [details] [diff] [review]
Reduce size of HuffmanIncomingTables

Review of attachment 8729238 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry - I reviewed this yesterday evening, but totally forgot to hit "publish". Thanks!

Comment 7

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/676fc640c532865c16cda8102b9c3ddc2e8357fa
Bug 1255239 - Reduce size of HuffmanIncomingTables. r=hurley.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/676fc640c532