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

evaluate perf shifts caused by adopting ena in rustc #9

Open
nikomatsakis opened this issue Mar 13, 2018 · 1 comment
Open

evaluate perf shifts caused by adopting ena in rustc #9

nikomatsakis opened this issue Mar 13, 2018 · 1 comment

Comments

@nikomatsakis
Copy link
Contributor

In rust-lang/rust#47861, @sgrif recently switched rustc to use ena. This caused a huge perf regression, but for reasons unrelated to ena -- it was followed up by rust-lang/rust#48666 which reverted the troublesome commit. Still, looking at the combined scores here shows a small (but measurable) regression of a few percent on certain tests:

perf comparison showing combined results of 47861 and 48666

It's a bit messy because that diff includes some other commits. These are the perf results we did before landing, in which that troublesome commit was excluded:

perf comparison from before landing

You can see that the changes there are all in the 1-3% range. Some things worth investigating:

  • Was the 11% clap-rs regression caused by these PRs or not? It wasn't measured before landing
  • In order to make things faster, we also removed congruence closure from ena before landing. However, that didn't seem to make as much affect as I was hoping. Was it necessary?
  • Are there improvements we can make to ease the smaller regressions?

On the last point: there were a few distinct steps that we changed in rustc in the course of adopting ena. For example, we moved the types into the unification table directly. It might be that this caused a slight shift in cache misses or other things. Worth evaluating. I was wondering if [reodering the fields in the VarValue struct might be a win]:

https://github.com/rust-lang-nursery/ena/blob/7768e8f2f3d942580c7d3cb62d267af487e67865/src/unify/mod.rs#L158-L161

Or potentially separating the value field into a separate vector.

@Ten0
Copy link
Contributor

Ten0 commented Jun 7, 2019

Any news about the congruence closure's performance impact ? I'd be interested by that feature (more specifically the feature that came with it to iterate over the unioned keys).

EDIT: In the meantime I've got a fork here with this feature and more.

nikomatsakis added a commit to nikomatsakis/ena that referenced this issue Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants