Group dep node data into a single structure#57061
Conversation
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@bors try |
|
⌛ Trying commit 775d3e0861c93f8e4794264948ba7ed0956d5456 with merge 2f088a6151d06e930ecb997ba0859a9c32fc3b8a... |
|
☀️ Test successful - status-travis |
|
@rust-timer build 2f088a6151d06e930ecb997ba0859a9c32fc3b8a |
|
Success: Queued 2f088a6151d06e930ecb997ba0859a9c32fc3b8a with parent 9689ada, comparison URL. |
|
Finished benchmarking try commit 2f088a6151d06e930ecb997ba0859a9c32fc3b8a |
|
Looks like pretty solid wins! I don't have time to review this in detail right now. One thing I noticed: It seems like the PR undoes #49069 which @wesleywiser implemented a while ago per my request. It would be good to check if the wins here are due to the struct-of-arrays approach being a bad idea in this scenario. Maybe it's a bad trade-off, i.e. we trade faster serialization for bad cache locality during tracking. |
michaelwoerister
left a comment
There was a problem hiding this comment.
Thanks again for the PR, @Zoxc! r=me with the nits addressed.
I'm not quite happy with factoring of functions in the hir collector. It seems that it should be possible to make this a little less complicated. But I couldn't come up with something better either.
src/librustc/hir/map/collector.rs
Outdated
There was a problem hiding this comment.
Can you rename fingerprint to something like combined_fingerprint and hash to fingerprint here? That should make it easier to understand what's going on.
src/librustc/hir/map/collector.rs
Outdated
There was a problem hiding this comment.
This should be dep_node I guess?
src/librustc/dep_graph/graph.rs
Outdated
There was a problem hiding this comment.
Can you rename f to fingerprint here and at the other occurrences?
src/librustc/hir/map/collector.rs
Outdated
There was a problem hiding this comment.
Could you please rename this to alloc_hir_dep_nodes?
|
@bors r=michaelwoerister |
|
📌 Commit 2738f2c has been approved by |
|
@bors r- |
|
@bors r=michaelwoerister |
|
📌 Commit 2738f2c has been approved by |
Group dep node data into a single structure r? @michaelwoerister
|
☀️ Test successful - status-appveyor, status-travis |
r? @michaelwoerister