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

Another attempt to reduce size_of<HashMap> #159

Merged
merged 11 commits into from
May 23, 2020
Merged

Another attempt to reduce size_of<HashMap> #159

merged 11 commits into from
May 23, 2020

Conversation

iwa0
Copy link
Contributor

@iwa0 iwa0 commented May 17, 2020

#69

Changes made
  • data field is removed, instead the allocation layout is changed such that first element of control bytes start exactly at one past last element of bucket table. So we just use negative index to access bucket table without touching size field. Allocation layout looks like this:

    hashbrown/src/raw/mod.rs

    Lines 359 to 361 in 7027781

    // [Padding], T1, T2, ..., Tlast, C1, C2, ...
    // ^ points here
    ctrl: NonNull<u8>,
  • Changed calculate_layout implementation:
    • Layout is now [Paddings] | [Buckets] | [Ctrls], previously it was [Ctrls] | [Paddings] | [Buckets]
    • Returned offset is now start of control bytes in the allocation and also one past last element of buckets. (Previously it was data offset)
  • Renamed Bucket::add to Bucket::next_n
Drawbacks
  • Development cost? accessing with negative index seems awkward
  • Code is slower on cases where start of data pointer is needed
Issues
  • calling Bucket::as_ptr() on one past last element yields UB (crosses allocation boundary).
  • data_start() doesn't equal data_end() when table is empty because empty table returns buckets() == 1

cargo benchcmp (from local)

 name                         before ns/iter  after ns/iter  diff ns/iter   diff %  speedup 
 clone_from_large             1,454           1,858                   404   27.79%   x 0.78 
 clone_from_small             17              22                        5   29.41%   x 0.77 
 clone_large                  1,985           1,731                  -254  -12.80%   x 1.15 
 clone_small                  47              39                       -8  -17.02%   x 1.21 
 insert_ahash_highbits        9,314           8,935                  -379   -4.07%   x 1.04 
 insert_ahash_random          9,556           9,509                   -47   -0.49%   x 1.00 
 insert_ahash_serial          9,739           9,475                  -264   -2.71%   x 1.03 
 insert_erase_ahash_highbits  20,957          20,920                  -37   -0.18%   x 1.00 
 insert_erase_ahash_random    21,220          22,805                1,585    7.47%   x 0.93 
 insert_erase_ahash_serial    25,188          24,349                 -839   -3.33%   x 1.03 
 insert_erase_std_highbits    50,005          50,587                  582    1.16%   x 0.99 
 insert_erase_std_random      49,894          50,796                  902    1.81%   x 0.98 
 insert_erase_std_serial      42,161          44,495                2,334    5.54%   x 0.95 
 insert_std_highbits          22,460          22,433                  -27   -0.12%   x 1.00 
 insert_std_random            22,554          22,943                  389    1.72%   x 0.98 
 insert_std_serial            19,504          19,748                  244    1.25%   x 0.99 
 iter_ahash_highbits          1,659           1,747                    88    5.30%   x 0.95 
 iter_ahash_random            1,626           1,743                   117    7.20%   x 0.93 
 iter_ahash_serial            1,492           1,520                    28    1.88%   x 0.98 
 iter_std_highbits            1,639           1,792                   153    9.33%   x 0.91 
 iter_std_random              1,600           1,834                   234   14.62%   x 0.87 
 iter_std_serial              1,466           1,500                    34    2.32%   x 0.98 
 lookup_ahash_highbits        4,445           4,912                   467   10.51%   x 0.90 
 lookup_ahash_random          4,467           4,964                   497   11.13%   x 0.90 
 lookup_ahash_serial          6,240           4,738                -1,502  -24.07%   x 1.32 
 lookup_fail_ahash_highbits   4,241           4,444                   203    4.79%   x 0.95 
 lookup_fail_ahash_random     4,514           4,603                    89    1.97%   x 0.98 
 lookup_fail_ahash_serial     4,471           4,622                   151    3.38%   x 0.97 
 lookup_fail_std_highbits     18,394          18,476                   82    0.45%   x 1.00 
 lookup_fail_std_random       18,129          18,378                  249    1.37%   x 0.99 
 lookup_fail_std_serial       15,348          15,716                  368    2.40%   x 0.98 
 lookup_std_highbits          18,042          18,351                  309    1.71%   x 0.98 
 lookup_std_random            18,247          18,567                  320    1.75%   x 0.98 
 lookup_std_serial            15,054          15,767                  713    4.74%   x 0.95 

Some of results vary a lot in my tests, they seem roughly same except clone_from_large is slower.

iwa0 added 4 commits May 17, 2020 21:09
- `data` field is removed, instead the allocation layout is changed such that first element of control bytes start exactly at one past last element of data table. So we just use negative index to access data table without touching size field.
- Rewritten `calculate_layout` to remove padding between two tables, its placed either at start or end of allocation if necessary.
doing `ptr as usize as *mut _` does escape from miri analysis
```
ptr: NonNull::new_unchecked(ptr),
     |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a potentially NULL pointer, but expected something that cannot possibly fail to be greater or equal to 1
```
I am not sure if this is correct way to fix it.
src/raw/mod.rs Show resolved Hide resolved
src/raw/mod.rs Outdated Show resolved Hide resolved
src/raw/mod.rs Outdated Show resolved Hide resolved
src/raw/mod.rs Outdated Show resolved Hide resolved
src/raw/mod.rs Outdated Show resolved Hide resolved
src/raw/mod.rs Outdated Show resolved Hide resolved
src/raw/mod.rs Outdated Show resolved Hide resolved
src/raw/mod.rs Outdated Show resolved Hide resolved
iwa0 added 4 commits May 20, 2020 16:11
- Removed unnecessary padding at the end of `calculate_layout`
- Put back old comments
- Renamed some functions
- Use `#[cfg(feature = "...")]` instead `#[allow(dead_code)]`
- nightly version of calculate_layout was putting padding between two tables in some cases, fixed. Also added debug_assert! to check this.
- use wrapping_sub() instead sub() in data_start() (empty RawTable returns buckets() == 1, crosses allocation bounds)
src/raw/mod.rs Outdated Show resolved Hide resolved
src/raw/mod.rs Show resolved Hide resolved
iwa0 added 3 commits May 22, 2020 20:29
- Removed unnecessary code from nightly `calculate_layout`
- Added comment for `Bucket::ptr` data member
removed wrong assertation code
@Amanieu
Copy link
Member

Amanieu commented May 23, 2020

@bors r+

@bors
Copy link
Collaborator

bors commented May 23, 2020

📌 Commit b500a0a has been approved by Amanieu

@bors
Copy link
Collaborator

bors commented May 23, 2020

⌛ Testing commit b500a0a with merge 9498006...

@bors
Copy link
Collaborator

bors commented May 23, 2020

☀️ Test successful - checks-travis
Approved by: Amanieu
Pushing 9498006 to master...

@bors bors merged commit 9498006 into rust-lang:master May 23, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 7, 2020
Update hashbrown to 0.8.1

This update includes:
- rust-lang/hashbrown#146, which improves the performance of `Clone` and implements `clone_from`.
- rust-lang/hashbrown#159, which reduces the size of `HashMap` by 8 bytes.
- rust-lang/hashbrown#162, which avoids creating small 1-element tables.

Fixes rust-lang#28481
bors-servo added a commit to servo/servo that referenced this pull request Aug 29, 2020
Upgrade to rustc 1.48.0-nightly (d006f5734 2020-08-28)

rust-lang/hashbrown#159 reduced `size_of::<HashMap>()`
bors-servo added a commit to servo/servo that referenced this pull request Nov 11, 2020
Upgrade to rustc 1.48.0-nightly (d006f5734 2020-08-28)

rust-lang/hashbrown#159 reduced `size_of::<HashMap>()`
bors-servo added a commit to servo/servo that referenced this pull request Nov 12, 2020
Upgrade to rustc 1.48.0-nightly (d006f5734 2020-08-28)

rust-lang/hashbrown#159 reduced `size_of::<HashMap>()`
bors-servo added a commit to servo/servo that referenced this pull request Jan 25, 2021
Upgrade to rustc 1.51.0-nightly (1d0d76f8d 2021-01-24)

rust-lang/hashbrown#159 reduced `size_of::<HashMap>()`
bors-servo added a commit to servo/servo that referenced this pull request Jan 25, 2021
Upgrade to rustc 1.51.0-nightly (1d0d76f8d 2021-01-24)

rust-lang/hashbrown#159 reduced `size_of::<HashMap>()`
bors-servo added a commit to servo/servo that referenced this pull request Jan 25, 2021
Upgrade to rustc 1.51.0-nightly (1d0d76f8d 2021-01-24)

rust-lang/hashbrown#159 reduced `size_of::<HashMap>()`
bors-servo added a commit to servo/servo that referenced this pull request Jan 26, 2021
Upgrade to rustc 1.49.0-nightly (5404efc28 2020-11-11)

rust-lang/hashbrown#159 reduced `size_of::<HashMap>()`
bors-servo added a commit to servo/servo that referenced this pull request Jan 26, 2021
Upgrade to rustc 1.48.0-nightly (623fb90b5 2020-09-26)

rust-lang/hashbrown#159 reduced `size_of::<HashMap>()`
bors-servo added a commit to servo/servo that referenced this pull request Jan 26, 2021
Upgrade to rustc 1.48.0-nightly (623fb90b5 2020-09-26)

rust-lang/hashbrown#159 reduced `size_of::<HashMap>()`
bors-servo added a commit to servo/servo that referenced this pull request Feb 25, 2021
Upgrade to rustc 1.48.0-nightly (623fb90b5 2020-09-26)

rust-lang/hashbrown#159 reduced `size_of::<HashMap>()`
bors-servo added a commit to servo/servo that referenced this pull request Feb 25, 2021
Upgrade to rustc 1.48.0-nightly (623fb90b5 2020-09-26)

rust-lang/hashbrown#159 reduced `size_of::<HashMap>()`
bors-servo added a commit to servo/servo that referenced this pull request Feb 25, 2021
Upgrade to rustc 1.48.0-nightly (623fb90b5 2020-09-26)

rust-lang/hashbrown#159 reduced `size_of::<HashMap>()`
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.

3 participants