Skip to content

Conversation

@jonas-schievink
Copy link
Contributor

This is mostly just for testing/fun, but it looks like type inference can be sped up massively with little to no effort (since it runs after the serial phases are already done).

Without --parallel:

Item Collection: 16.43597698s, 683mb allocated 720mb resident
Inference: 25.429774879s, 1720mb allocated 1781mb resident
Total: 41.865866352s, 1720mb allocated 1781mb resident

With --parallel:

Item Collection: 16.380369815s, 683mb allocated 735mb resident
Parallel Inference: 7.449166445s, 1721mb allocated 1812mb resident
Inference: 143.437157ms, 1721mb allocated 1812mb resident
Total: 23.973303611s, 1721mb allocated 1812mb resident

@flodiebold
Copy link
Member

That's pretty cool! Is there any reason except comparison purposes to make this optional?

@bjorn3
Copy link
Member

bjorn3 commented Jun 30, 2020

I don't think the lsp server uses parallel type inference.

@jonas-schievink
Copy link
Contributor Author

Is there any reason except comparison purposes to make this optional?

Single-threaded inference is deterministic (I think?), while multi-threaded inference isn't. This can be a helpful property when trying to narrow down bugs, and also makes benchmarks more reproducible.

I don't think the lsp server uses parallel type inference.

Yeah, this is just an experiment for analysis-stats. Interactive use is unlikely to benefit from this as types are only inferred for function bodies visible in the editor.

@lnicola
Copy link
Member

lnicola commented Jun 30, 2020

Single-threaded inference is deterministic

Nope: #1552

@jonas-schievink
Copy link
Contributor Author

Ah, that's unfortunate. But adding threads on top makes it even more noisy.

@flodiebold
Copy link
Member

Well, it's supposed to be deterministic, yeah. And analysis-stats is a good tool for finding non-determinism (which I should do sometime again...), you're right.

Copy link
Contributor

@matklad matklad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

serde = { version = "1.0.106", features = ["derive"] }
serde_json = "1.0.48"
threadpool = "1.7.1"
rayon = "1.3.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just seeing rayon in Cargo.toml creates an impression of speed 👍 !

@bors
Copy link
Contributor

bors bot commented Jun 30, 2020

@bors bors bot merged commit b1a2d01 into rust-lang:master Jun 30, 2020
@jonas-schievink jonas-schievink deleted the parallelize-anastats branch June 30, 2020 21:31
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

Successfully merging this pull request may close these issues.

5 participants