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

Size of HashMap increased between 2/16 and 2/17 nightly #40042

Closed
jdm opened this issue Feb 22, 2017 · 5 comments
Closed

Size of HashMap increased between 2/16 and 2/17 nightly #40042

jdm opened this issue Feb 22, 2017 · 5 comments

Comments

@jdm
Copy link
Contributor

jdm commented Feb 22, 2017

use std::collections::HashMap;
use std::mem::size_of;

struct S {
    _handlers: HashMap<usize, usize>,
}

fn main() {
    println!("{}", size_of::<S>());
}
godot2:tmp jdm$ rustup override set nightly-2017-02-16
info: using existing install for 'nightly-2017-02-16-x86_64-apple-darwin'
info: override toolchain for '/private/tmp' set to 'nightly-2017-02-16-x86_64-apple-darwin'

  nightly-2017-02-16-x86_64-apple-darwin unchanged - rustc 1.17.0-nightly (62eb6056d 2017-02-15)

godot2:tmp jdm$ rustc node.rs
godot2:tmp jdm$ ./node
40

godot2:tmp jdm$ rustup override set nightly-2017-02-17
info: using existing install for 'nightly-2017-02-17-x86_64-apple-darwin'
info: override toolchain for '/private/tmp' set to 'nightly-2017-02-17-x86_64-apple-darwin'

  nightly-2017-02-17-x86_64-apple-darwin unchanged - rustc 1.17.0-nightly (668864d9e 2017-02-16)

godot2:tmp jdm$ rustc node.rs
godot2:tmp jdm$ ./node
48
@jdm
Copy link
Contributor Author

jdm commented Feb 22, 2017

I see that this is caused by 57940d0#diff-1ab410241f658d595f1c5eec7a979b01R409. Eight bytes for a bool is sad-making, since this increases the size of every single DOM node in Servo.

@SimonSapin
Copy link
Contributor

In #38368 I proposed packing this bit in RawTable::size or RawTable::capacity.

@arielb1
Copy link
Contributor

arielb1 commented Feb 23, 2017

This is fairly crappy - an "insecure" HashMap is only 3 words long.

@SimonSapin
Copy link
Contributor

(Aligned up to) 4 words now, with the extra bool field added in #38368.

This is unrelated to the size cost of RandomState (16 bytes).

@arthurprs
Copy link
Contributor

arthurprs commented Feb 24, 2017

Wound something like this https://is.gd/pMbxfY work instead of the Unique<> we use in RawTable? Taking a bit from capacity or length could require some annoying changes to overflow handling, reserving, etc.

Edit: that would surely not please the borrow checker though. As Entry<,> require a mut ref to the table.
Edit2: nevermind that, we can access the &mut table using the readily available FullBucket and EmptyBucket.

arthurprs added a commit to arthurprs/rust that referenced this issue Mar 3, 2017
Exposes a boolean flag in RawTable and use it
instead of a bool field in HashMap.

Fixes: rust-lang#40042
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 8, 2017
Reduce size overhead of adaptative hashmap

Exposes a boolean flag in RawTable and use it instead of a bool field in HashMap.

Taking a bit from capacity or length would make overflow handling tricky.

Fixes: rust-lang#40042
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

4 participants