Refactor task system for efficiency

[Zoxc]

Split from #56509

r? @michaelwoerister

[michaelwoerister]

@bors try

[bors]

⌛ Trying commit 88531f4 with merge 3bb0bed84e5d690aeee6787e6e0978774aee2789...

[bors]

☀️ Test successful - status-travis
State: approved= try=True

[michaelwoerister]

@rust-timer build 3bb0bed84e5d690aeee6787e6e0978774aee2789

[rust-timer]

Success: Queued 3bb0bed84e5d690aeee6787e6e0978774aee2789 with parent a64cdec, comparison URL.

[rust-timer]

Finished benchmarking try commit 3bb0bed84e5d690aeee6787e6e0978774aee2789

[michaelwoerister]

The perf results look like there's no real improvement here. I see -0.4-7% in some cases but also +0.8% in others. As such, I'd prefer not to do these changes.

[michaelwoerister]

@Zoxc, would you be OK with closing this PR? It changes quite a bit of code but the benefit of that is doubtful.

[Zoxc]

Could you make your objections a bit more concrete? I like that this splits up with_task_impl which is rather large currently, and that ImplicitCtxt is only changed once when executing a query.

[michaelwoerister]

My objections are:

• Changing code in a multi-user codebase comes at a cost. People who understood the code before have to invest time into understanding it again. A change also needs to be reviewed, which costs time and energy, especially if the change affects complicated logic. It might also introduce new bugs that are not spotted during review, causing follow-up work. So any (non-trivial) change starts out with a kind of a debt that it has to make up for in improved code quality/readability, improved runtime efficiency, or some other improvement. The changes in this PR don't make the code more efficient in practice, increase code duplication, and makes already too complicated logic a bit more complicated, so in my estimation it doesn't offset the debt it introduces.
• The current version of the code follows a kind of template pattern with with_task_impl giving a skeleton for the logic that needs to be executed for all kinds of task. This reduces code duplication and thus the risk of forgetting to update other the kinds of tasks, when fixing a bug in one.
• It also gives the information that different kinds of tasks basically follow the same kind of logic, which is quite a big win for making the code more readable, I think.
• The version in this PR increases the number of public DepGraph::with_* methods from 5 to 7, making it even harder to understand which one to use. I'd rather see the number reduced, if possible.
• The interaction between query system and task system becomes more circular than before, increasing the coupling of the two systems.
• It breaks the ProfileQueriesMsgcalls, which to me seems like an indication that there is too much duplication of logic.

[Dylan-DPC-zz]

ping from triage @michaelwoerister @Zoxc what's the update on this?