Stream: t-compiler/wg-rls-2.0

Topic: Changing CHALK_SOLVER_MAX_SIZE


Jack Huey (Jan 05 2020 at 19:51, on Zulip):

@matklad @nikomatsakis So, I know it's been discussed already a few times, but I really think the SLG max_size should be changed from 4 to probably 10. (Relevant line here https://github.com/rust-analyzer/rust-analyzer/blob/ce07a2daa9e53aa86a769f8641b14c2878444fbc/crates/ra_hir_ty/src/traits.rs#L59)

Jack Huey (Jan 05 2020 at 19:52, on Zulip):

But, thinking about this made me think about how we would profile this

Jack Huey (Jan 05 2020 at 19:53, on Zulip):

Like, it would be simple enough to make the change. But the question is about those "edge cases" mentioned in the comment for that const

Jack Huey (Jan 05 2020 at 19:54, on Zulip):

However, I think changing it to 10 may mask/fix some errors, such as https://github.com/rust-lang/chalk/issues/302 and https://github.com/rust-lang/chalk/issues/301

Florian Diebold (Jan 05 2020 at 20:08, on Zulip):

I think last time I tried to change it back to 10, it still lead to analysis hanging seemingly forever when running RA on itself

Jack Huey (Jan 05 2020 at 20:12, on Zulip):

How long ago was that?

Jack Huey (Jan 05 2020 at 20:13, on Zulip):

Also, changing it to 5 or 6 might also solve some problems too

Jack Huey (Jan 05 2020 at 20:14, on Zulip):

(this is probably more of a discussion for a wg-traits topic, but) I think max_size is probably not the right way of limiting run time

Jack Huey (Jan 05 2020 at 20:14, on Zulip):

(obviously, this has been thought about before with the idea of fuel)

Jack Huey (Jan 05 2020 at 20:17, on Zulip):

Right now, as long as everything is "making progress" in terms of finding answers to strands, Chalk will keep going and won't yield with QuantumExceeded (except for PositiveCycles, which sort of have to continue currently). But once a table flounders, or there are no more solutions to a strand, it should yield with QuantumExceeded. The problem though right now, is that actually doesn't do anything, since ForestSolver just loops

Jack Huey (Jan 05 2020 at 20:18, on Zulip):

Probably want to think about allowing the caller to stop solving on QuantumExceeded, and if so, give some sort of "best guess" for an answer

Florian Diebold (Jan 05 2020 at 20:30, on Zulip):

https://github.com/rust-analyzer/rust-analyzer/pull/2290

Florian Diebold (Jan 05 2020 at 20:31, on Zulip):

I can try out what happens

Jack Huey (Jan 05 2020 at 20:35, on Zulip):

I don't think we've done anything since then that should make it not hang if it hung with 5 then

Jack Huey (Jan 05 2020 at 20:36, on Zulip):

I'm definitely interested to know if Chalk is yielding with QuantumExceeded during those hangs at any more

Jack Huey (Jan 05 2020 at 20:37, on Zulip):

Oh wait, maybe the hang for that issue is the old bug?

Jack Huey (Jan 05 2020 at 20:37, on Zulip):

maybe, maybe not

Florian Diebold (Jan 05 2020 at 20:49, on Zulip):

currently, with 4 it crashes with "New answer was not ambiguous whereas previous answer was", with 5 it goes through, with 6-8 it crashes, with 9-10 it hangs

Florian Diebold (Jan 05 2020 at 20:51, on Zulip):

('it hangs' doesn't necessarily mean it hangs forever, I only waited a few minutes)

Florian Diebold (Jan 05 2020 at 20:53, on Zulip):

you can try this yourself (and see the Chalk debug logs) by setting the solver size to 9 or 10 and doing cargo build --release -p ra_cli; CHALK_DEBUG=1 ./target/release/ra_cli analysis-stats . -o trait_solve_query

Florian Diebold (Jan 05 2020 at 20:53, on Zulip):

(kind of ironically, trait_solve_query is the function where the analysis hangs)

Jack Huey (Jan 05 2020 at 20:56, on Zulip):

It's actually kind of interesting that 5 is fine but 6-8 crashes

nikomatsakis (Jan 06 2020 at 15:09, on Zulip):

interesting

nikomatsakis (Jan 06 2020 at 15:10, on Zulip):

I agree that upping the limit is good; I've been contemplating whether we should consider an alternative design for the chalk engine itself, or at least experiment with a few more. But I guess we have to first identify where the problems arise. I see a lot of chatter in #wg-traits so probably best to take it to a topic over there

nikomatsakis (Jan 06 2020 at 19:47, on Zulip):

Right now, as long as everything is "making progress" in terms of finding answers to strands, Chalk will keep going and won't yield with QuantumExceeded (except for PositiveCycles, which sort of have to continue currently). But once a table flounders, or there are no more solutions to a strand, it should yield with QuantumExceeded. The problem though right now, is that actually doesn't do anything, since ForestSolver just loops

btw what I had expected to do was to propagate QunatumExceeded back out through ForestSolver, I think, so that rust-analyzer could choose how much to loop

nikomatsakis (Jan 06 2020 at 19:48, on Zulip):

not sure @Jack Huey if this is what you were thinking

Jack Huey (Jan 06 2020 at 19:51, on Zulip):

Yep, basically. Just right now that's not what is done

Last update: Jan 21 2020 at 09:20UTC