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

Shink NodeId, CrateNum, Name and Mrk down to 32 bits on x64. #10670

Merged
merged 1 commit into from
Nov 27, 2013

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Nov 26, 2013

Rationale

There is no reason to support more than 2³² nodes or names at this moment, as compiling something that big (even without considering the quadratic space usage of some analysis passes) would take at least 64GB.
Meanwhile, some can't (or barely can) compile rustc because it requires almost 1.5GB.

Potential problems

Can someone confirm this doesn't affect metadata (de)serialization? I can't tell myself, I know nothing about it.

Results

Some structures have a size reduction of 25% to 50%: before - after.
Sadly, there isn't a massive change in the memory used for compiling stage2 librustc (it doesn't go over 1.4GB as before, but I can barely see the difference).
However, my own testcase (previously peaking at 1.6GB in typeck) shows a reduction of 200-400MB.

@emberian
Copy link
Member

👍, but this should be discussed (even if briefly) at the meeting I think.

@pnkfelix
Copy link
Member

I'll make sure its on the agenda.

@@ -285,7 +285,7 @@ fn resolve_crate(e: @mut Env,
// Claim this crate number and cache it
let cnum = e.next_crate_num;
e.crate_cache.push(cache_entry {
cnum: cnum,
cnum: cnum as int,
Copy link
Member

Choose a reason for hiding this comment

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

Why is this cast necessary, shouldn't cache_entry.cnum be type CrateNum?

bors added a commit that referenced this pull request Nov 27, 2013
### Rationale
There is no reason to support more than 2³² nodes or names at this moment, as compiling something that big (even without considering the quadratic space usage of some analysis passes) would take at least **64GB**.
Meanwhile, some can't (or barely can) compile rustc because it requires almost **1.5GB**.

### Potential problems
Can someone confirm this doesn't affect metadata (de)serialization? I can't tell myself, I know nothing about it.

### Results
Some structures have a size reduction of 25% to 50%: [before](https://gist.github.com/luqmana/3a82a51fa9c86d9191fa) - [after](https://gist.github.com/eddyb/5a75f8973d3d8018afd3).
Sadly, there isn't a massive change in the memory used for compiling stage2 librustc (it doesn't go over **1.4GB** as [before](http://huonw.github.io/isrustfastyet/mem/), but I can barely see the difference).
However, my own testcase (previously peaking at **1.6GB** in typeck) shows a reduction of **200**-**400MB**.
@bors bors closed this Nov 27, 2013
@bors bors merged commit 7ed27b5 into rust-lang:master Nov 27, 2013
@eddyb eddyb deleted the node-u32 branch November 27, 2013 07:18
bors added a commit that referenced this pull request Dec 1, 2013
**Note**: I only tested on top of my #10670 PR, size reductions come from both change sets.

With this, [more enums are shrinked](https://gist.github.com/eddyb/08fef0dfc6ff54e890bc), the most significant one being `ast_node`, from 104 bytes (master) to 96 (#10670) and now to 32 bytes.

My own testcase requires **200MB** less when compiling (not including the other **200MB** gained in #10670), and rustc-stage2 is down by about **130MB**.

I believe there is more to gain by fiddling with the enums' layouts.
flip1995 pushed a commit to flip1995/rust that referenced this pull request May 5, 2023
Don't suggest `suboptimal_flops` unavailable in nostd

Fixes rust-lang#10634

changelog: Enhancement: [`suboptimal_flops`]: Do not suggest `{f32,f64}::abs()` or `{f32,f64}::mul_add()` in a `no_std`-environment.
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.

7 participants