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

Reduce the `DepNode` pre-allocation ratio. #59626

Merged
merged 1 commit into from Apr 15, 2019

Conversation

@nnethercote
Copy link
Contributor

commented Apr 2, 2019

A code size of increase of 15% is overly generous. 2% is more realistic.

This change reduces peak memory size by 20+ MiB on some workloads.

r? @Zoxc

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

I'll do a perf run, but max-rss is variable enough in rustc-perf that we may not get a clear result. Nonetheless, I've used DHAT to confirm that this changed has the intended effect.

@bors try

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

⌛️ Trying commit 6c9867d with merge 15dbaaa...

bors added a commit that referenced this pull request Apr 2, 2019

Auto merge of #59626 - nnethercote:DepGraph-1.02x, r=<try>
Reduce the `DepNode` pre-allocation ratio.

A code size of increase of 15% is overly generous. 2% is more realistic.

This change reduces peak memory size by 20+ MiB on some workloads.

r? @Zoxc
@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

☀️ Try build successful - checks-travis
Build commit: 15dbaaa

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

@rust-timer

This comment has been minimized.

Copy link

commented Apr 2, 2019

Success: Queued 15dbaaa with parent f694222, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented Apr 2, 2019

Finished benchmarking try commit 15dbaaa

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

The max-rss results are too noisy to see anything useful, but there is a small but clear instruction count improvement, lots of cases up to 0.5% better.

@Zoxc

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

@nnethercote How did you arrive at the 2% is realistic conclusion?

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

I looked at all the incremental "patched" cases in rustc-perf and they almost all fit within 0.5%, with many being quite a bit less. (There was one 20% case.) So I rounded up a bit to be safe. More generally, 15% seems really generous -- for the crate to increase in size that much would be very rare unless it's a very small crate.

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

@Zoxc: Anything else I can do to convince you that this is a reasonable change? :)

@Zoxc

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

I don't think the incremental "patched" cases in rustc-perf are very representative though. A low percentage doesn't seem great for small crates either. Maybe something like (prev_graph_node_count * 102) / 100 + constant would be better? What would be required to catch the 20% case (assuming that was a smaller crate)?

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

I think trying to catch the 20% case is a bad idea. It was a single exceptional case out of dozens; this should be tuned to work well with common cases. And small crates are less important, because they're already fast to compile. If the size is too small we'll just have to reallocate the table once, it's not that big a deal...

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Apr 5, 2019

FWIW, here are the sizes and ratios seen in rustc-perf, excluding all the ones with ratio 1 or infinity:

sizes 100094, 100177 (1.0008292205326992)
sizes 104936, 104981 (1.000428832812381)
sizes 106593, 106614 (1.0001970110607639)
sizes 106614, 106821 (1.0019415836569305)
sizes 113095, 113199 (1.0009195808833282)
sizes 113199, 113890 (1.006104294207546)
sizes 113425, 113493 (1.0005995150980824)
sizes 113461, 113859 (1.003507813257419)
sizes 113493, 114055 (1.0049518472504912)
sizes 113859, 113927 (1.000597229907166)
sizes 113890, 113425 (0.9959171130037756)
sizes 113927, 114077 (1.001316632580512)
sizes 114055, 114057 (1.000017535399588)
sizes 114057, 113461 (0.9947745425532848)
sizes 12143, 12147 (1.000329407889319)
sizes 12143, 12147 (1.000329407889319)
sizes 1325009, 1326730 (1.001298859102089)
sizes 1326730, 1328011 (1.0009655317962207)
sizes 1327208, 1327491 (1.0002132295766752)
sizes 1328011, 1327208 (0.9993953363338105)
sizes 151448, 151502 (1.0003565580265175)
sizes 151502, 151734 (1.0015313329196975)
sizes 16072, 16074 (1.0001244400199103)
sizes 161699, 161834 (1.000834884569478)
sizes 164375, 164414 (1.0002372623574145)
sizes 164414, 164582 (1.001021810794701)
sizes 1663, 2004 (1.2050511124473842)
sizes 17082, 17095 (1.0007610350076104)
sizes 1727, 1776 (1.0283729009843658)
sizes 17466, 17468 (1.0001145081873355)
sizes 18354, 18416 (1.0033780102429988)
sizes 18546, 18552 (1.0003235198964737
sizes 187902, 188065 (1.0008674734702132)
sizes 189404, 189641 (1.0012512935312876)
sizes 19379, 19390 (1.000567624748439)
sizes 19919, 20025 (1.0053215522867613)
sizes 19992, 20001 (1.000450180072029)
sizes 2004, 2026 (1.0109780439121756)
sizes 203192, 203199 (1.0000344501752036)
sizes 203199, 204350 (1.0056643979547144)
sizes 20708, 20768 (1.0028974309445624)
sizes 210877, 210910 (1.000156489327902)
sizes 210910, 213067 (1.0102271110900385)
sizes 2357216, 2359431 (1.00093966781152)
sizes 2359431, 2377340 (1.007590389377778)
sizes 2377340, 2376601 (0.9996891483759159)
sizes 25108, 25110 (1.00007965588657)
sizes 25110, 25332 (1.0088410991636798)
sizes 254604, 254655 (1.0002003110713107)
sizes 259509, 259717 (1.0008015136276585)
sizes 27015, 27141 (1.0046640755136036)
sizes 28057, 28078 (1.0007484763160708)
sizes 28078, 28356 (1.00990099009901)
sizes 281349, 281382 (1.0001172920465329)
sizes 29166, 29308 (1.0048686827127478)
sizes 31157, 31182 (1.0008023879064094)
sizes 31236, 31268 (1.001024458957613)
sizes 3270, 3311 (1.0125382262996943)
sizes 336017, 336224 (1.0006160402598678)
sizes 336224, 337513 (1.0038337536880175)
sizes 34269, 34290 (1.0006127987393854)
sizes 34290, 34788 (1.0145231846019247)
sizes 34644, 34770 (1.0036369934187739)
sizes 35379, 35370 (0.9997456118036123)
sizes 35642, 35633 (0.9997474889175691)
sizes 35911, 35949 (1.0010581715908775)
sizes 36004, 36019 (1.0004166203755138)
sizes 3713, 3752 (1.0105036358739563)
sizes 3714, 3713 (0.9997307485191168)
sizes 38355, 38710 (1.0092556381175857)
sizes 40393, 40748 (1.008788651499022)
sizes 406575, 406961 (1.0009493943306893)
sizes 41078, 41079 (1.0000243439310579)
sizes 4250747, 4251364 (1.0001451509581727)
sizes 4251364, 4251723 (1.0000844434868432)
sizes 4251654, 4257228 (1.0013110191939418)
sizes 4251723, 4251654 (0.9999837712851942)
sizes 42547, 42920 (1.008766775565845)
sizes 42547, 42920 (1.008766775565845)
sizes 42593, 42610 (1.0003991266170498)
sizes 439973, 440219 (1.0005591252190476)
sizes 440219, 441759 (1.0034982588211776)
sizes 44482, 44494 (1.000269772042624)
sizes 445213, 445627 (1.0009298919842862)
sizes 46658, 46980 (1.0069012816665952)
sizes 466932, 467571 (1.0013685076199532)
sizes 467571, 468684 (1.002380387149759)
sizes 4787, 4993 (1.0430332149571757)
sizes 48088, 48127 (1.000811013142572)
sizes 48833, 49321 (1.0099932422746913)
sizes 49074, 49099 (1.0005094347312222)
sizes 49333, 49597 (1.0053513875093751)
sizes 49416, 49677 (1.005281690140845)
sizes 4993, 5016 (1.0046064490286402)
sizes 51791, 51810 (1.0003668591067947)
sizes 52244, 52562 (1.0060868233672766)
sizes 52901, 52950 (1.0009262584828265)
sizes 53159, 53167 (1.0001504919204651)
sizes 54384, 54676 (1.0053692262430127)
sizes 56855, 56867 (1.0002110632310264)
sizes 57378, 57398 (1.0003485656523405)
sizes 57398, 58473 (1.0187288755705775)
sizes 57712, 57769 (1.0009876628777377)
sizes 58473, 58623 (1.0025652865425068)
sizes 58595, 58962 (1.006263333048895)
sizes 61467, 61533 (1.0010737468885744)
sizes 61533, 61846 (1.0050867014447533)
sizes 63399, 63435 (1.000567832300194)
sizes 66657, 66652 (0.9999249891234229)
sizes 6803, 6805 (1.0002939879464943)
sizes 68459, 68489 (1.0004382184957419)
sizes 70303, 70506 (1.0028875012446126)
sizes 70751, 70805 (1.0007632400955464)
sizes 71103, 71141 (1.0005344359591015)
sizes 71143, 71211 (1.0009558213738527)
sizes 72737, 72734 (0.9999587555164496)
sizes 72853, 72881 (1.0003843355798663)
sizes 72881, 72887 (1.0000823259834524)
sizes 74596, 75273 (1.0090755536489893)
sizes 76973, 77010 (1.0004806880334662)
sizes 77997, 78021 (1.000307704142467)
sizes 81043, 81744 (1.008649729156127)
sizes 8533, 8561 (1.003281378178835)
sizes 8572, 8641 (1.0080494633691088)
sizes 8601, 8603 (1.0002325311010347)
sizes 8603, 8694 (1.0105777054515868)
sizes 8641, 8533 (0.9875014465918297)
sizes 8694, 8572 (0.9859673337934207)
sizes 87932, 88140 (1.0023654642223536)
sizes 88140, 88531 (1.0044361243476287)
sizes 883931, 884381 (1.0005090895103803)
sizes 93002, 93047 (1.0004838605621384)
sizes 95433, 95583 (1.0015717833453837)
sizes 95859, 95935 (1.0007928311373997)
sizes 95935, 96306 (1.0038672017511856)
@Zoxc

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2019

If we had a +200 constant we could catch the sizes 4787, 4993 (1.0430332149571757) case. I can't tell if that's typical change from just the numbers though =P

Reduce the `DepNode` pre-allocation ratio.
A code size of increase of 15% is overly generous. 2% is more realistic.

This change reduces peak memory size by 20+ MiB on some workloads.

@nnethercote nnethercote force-pushed the nnethercote:DepGraph-1.02x branch from 6c9867d to 304a7be Apr 15, 2019

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Apr 15, 2019

@Zoxc: I added the +200 you requested.

@Zoxc

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

@bors rollup

@Zoxc

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

📌 Commit 304a7be has been approved by Zoxc

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

⌛️ Testing commit 304a7be with merge 1edb01b...

bors added a commit that referenced this pull request Apr 15, 2019

Auto merge of #59626 - nnethercote:DepGraph-1.02x, r=Zoxc
Reduce the `DepNode` pre-allocation ratio.

A code size of increase of 15% is overly generous. 2% is more realistic.

This change reduces peak memory size by 20+ MiB on some workloads.

r? @Zoxc
@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: Zoxc
Pushing 1edb01b to master...

@bors bors added the merged-by-bors label Apr 15, 2019

@bors bors merged commit 304a7be into rust-lang:master Apr 15, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.