Serialize dependency graph directly from DepGraph

[tgnottingham]

Reduce memory usage by serializing dep graph directly from DepGraph,
rather than copying it into SerializedDepGraph and serializing that.

[rust-highfive]

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

[tgnottingham]

@rustbot label T-compiler A-incr-comp I-compiletime I-compilemem

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

[tgnottingham]

Hm, I think the try request was dropped, maybe because the regular CI checks hadn't passed yet.

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

[bors]

⌛ Trying commit 3ae463ae9db28561c67bbbad2735c751508ef9e0 with merge 140e2d824700dd32cc8a5a4fee0ca320c3d12360...

[bors]

☀️ Try build successful - checks-actions
Build commit: 140e2d824700dd32cc8a5a4fee0ca320c3d12360 (140e2d824700dd32cc8a5a4fee0ca320c3d12360)

[rust-timer]

Queued 140e2d824700dd32cc8a5a4fee0ca320c3d12360 with parent 7a9b552, future comparison URL.

@rustbot label: +S-waiting-on-perf

[rust-timer]

Finished benchmarking try commit (140e2d824700dd32cc8a5a4fee0ca320c3d12360): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

[lcnr]

r? @michaelwoerister maybe

[michaelwoerister]

Thanks for the PR, @tgnottingham! This looks good to me in general. I'll review properly next week.

[michaelwoerister]

Thanks for the PR, @tgnottingham! This looks like a step in the right direction.

One thing I noticed while reading the code was that quite a bit of this data could be mapped from disk into memory as is (at least on little endian platforms) and already provides random access (DepNodes and Fingerprints don't use leb128 because that would actually waste space). So in the future, we might want to just define a proper binary file format for the dep-graph and make decoding lazy -- or partially lazy, depending on whether we can afford the additional disk space for encoding edge indices with 4 bytes instead of using leb128.

Anyway, getting rid of the memory spike is the right thing to do in any case.

@bors r+

[bors]

📌 Commit 09067db has been approved by michaelwoerister

[tgnottingham]

Thanks @michaelwoerister!

we might want to just define a proper binary file format for the dep-graph and make decoding lazy -- or partially lazy, depending on whether we can afford the additional disk space for encoding edge indices with 4 bytes instead of using leb128.

As a reference point, the style-servo-opt incr-full benchmark creates almost 16 million edges. The LEB128 encoding of the u32 numbers 0 to that number of edges is ~58MB, and the raw encoding is ~60MB. I was really surprised to see that, but it checks out. The majority (~13 million) of those edges use 4 byte LEB128 encodings, so the savings aren't substantial.

Edit: realized the above is a bad comparison, as the edges have values from 0 to number of nodes, not number of edges. But, if you assume the ~16 million edges have values equally distributed between 0 and 2.75 million (number of nodes in same benchmark), then the LEB128 encoding is ~48MB. So the savings are a bit more significant.

This isn't representative of the actual edge data space requirements, but it suggests that raw-encoding might not be so bad.

[bors]

⌛ Testing commit 09067db with merge c5a96fb...

[bors]

☀️ Test successful - checks-actions
Approved by: michaelwoerister
Pushing c5a96fb to master...

[michaelwoerister]

It would be awesome if we added -Zself-profile events for tracking this kind of data (i.e. file sizes) and made perf.rlo show them. That would make experimenting with things like this more convenient.