Upgrade Chalk #2872
Upgrade Chalk #2872
Conversation
flodiebold
force-pushed
the
chalk-upgrade
branch
from
1cd5e84 to
dcd3538
Compare
|
Along with this, I would also suggest changing the |
flodiebold
force-pushed
the
chalk-upgrade
branch
from
dcd3538 to
339a11c
Compare
|
r? @matklad |
| "lalrpop-intern 0.15.1 (registry+https://github.com/rust-lang/crates.io-index)", | ||
| ] | ||
|
|
||
| [[package]] | ||
| name = "chalk-macros" | ||
| version = "0.1.1" | ||
| source = "git+https://github.com/rust-lang/chalk.git?rev=ff65b5ac9860f3c36bd892c865ab23d5ff0bbae5#ff65b5ac9860f3c36bd892c865ab23d5ff0bbae5" | ||
| source = "git+https://github.com/rust-lang/chalk.git?rev=af48f302a1f571b3ca418f7c5aa639a144a34f75#af48f302a1f571b3ca418f7c5aa639a144a34f75" | ||
| dependencies = [ | ||
| "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", |
There was a problem hiding this comment.
Should switch this to OnceCell :D
|
bors r=matklad |
2872: Upgrade Chalk r=matklad a=flodiebold This is just keeping track of the changes required to upgrade Chalk; currently we can't really merge it since it breaks opaque types. Now also makes use of the newly introduced `solve_limited` to implement fuel. Co-authored-by: Florian Diebold <[email protected]>
Build succeeded
|
|
I've tried running again on the substrate repo. It eventually OOMs, but looks like this is because we genuinely don't cleanup various caches, and not because some particular functions leads to O(infinity) allocations. |
|
Does the fuel not prevent that? |
|
I don't understand why you are asking this question :) Basically, after we type check each function, we seem to leave some stuff allocated behind (inside chalk caches, or maybe the |
|
Oh, I somehow thought the problem was one big Chalk query, not many small ones... |
|
It's still a bit weird though -- I don't think Chalk memory usage should scale directly with the number of functions, more with the number of types and traits 🤔 |
|
This is interesting. It doesn't really surprise me that memory scales with functions, considering each one would generate a new |
|
@jackh726 Shouldn't there be a table per unique goal? Although I didn't consider environments, which might lead to enough combinations to cause this 🤔 |
|
To be clear, I did zero analysis of where the memory goes, so it might be
something unrelated to chalk. Will try to dust off our sophisticated memory
profiling infra tomorrow
…On Tuesday, 28 January 2020, Florian Diebold ***@***.***> wrote:
@jackh726 <https://github.com/jackh726> Shouldn't there be a table per
*unique* goal? Although I didn't consider environments, which might lead
to enough combinations to cause this 🤔
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2872>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANB3M6BQ4RKXJI5QDZ32NTRACB2VANCNFSM4KIOHDSQ>
.
|
|
@flodiebold Yes, it is per unique goal. I sort of assumed that almost every goal would be different because of different signatures. But yeah, different environments too :/ (actually now that I think about it, that would be something interesting to try "delaying" until the root goal, so sub-goals are shared) |
|
Here's what I've got: TL;DR: it's mostly salsa's dependencies. And also 7.5 gigabytes of something else :) |
|
The huge leftower is due to the fact that salsa does not collect interned queries by default. Here's how it looks like with full collection Note that |
|
opened #2945 with a slightly bigger trace |
This is just keeping track of the changes required to upgrade Chalk; currently we can't really merge it since it breaks opaque types.
Now also makes use of the newly introduced
solve_limitedto implement fuel.