Skip to content

Eliminated SmallIntSet - #7934

Closed
sfackler wants to merge 1 commit into
rust-lang:masterfrom
sfackler:smallintset
Closed

Eliminated SmallIntSet#7934
sfackler wants to merge 1 commit into
rust-lang:masterfrom
sfackler:smallintset

Conversation

@sfackler

Copy link
Copy Markdown
Member

SmallIntSet is equivalent to BitvSet but with 64 times the memory
overhead. There's no reason for it to exist.

SmallIntSet's overhead should really only be 8 times, but for some
reason, sys::size_of::<Option<()>>() == 8, not 1.

@huonw

Copy link
Copy Markdown
Contributor

The reason that Option<()> is 8 bytes is because the discriminant is always word size, rather than shrinking to be the minimum possible (#1647).

@thestinger

Copy link
Copy Markdown
Contributor

I don't think these are really equivalent. If you add 1, 2 and 3 to SmallIntSet, it will contain them as 3 distinct elements. If you only insert 1 and 2, it will not contain 3. I might be misunderstanding what BitvSet is, but I thought it was a set of bits.

@huonw

Copy link
Copy Markdown
Contributor

@thestingernth bit set == n in the BitvSet. (At least, that's the current semantics.)

@thestinger

Copy link
Copy Markdown
Contributor

Ah.

@huonw

Copy link
Copy Markdown
Contributor

@sfackler needs a rebase.

@sfackler

Copy link
Copy Markdown
MemberAuthor

@huonw Done.

SmallIntSet is equivalent to BitvSet but with 8 times the memory
overhead. There's no reason for it to exist.
bors added a commit that referenced this pull request Jul 23, 2013
SmallIntSet is equivalent to BitvSet but with 64 times the memory
overhead. There's no reason for it to exist.
SmallIntSet's overhead should really only be 8 times, but for some
reason, `sys::size_of::<Option<()>>() == 8`, not 1.
@borsbors closed this Jul 23, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 23, 2021
Fix ICE in undocumented_unsafe_blocks
changelog: Fix ICE in [`undocumented_unsafe_blocks`]
closes: rust-lang#7934
Sign up for freeto 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

@sfackler@huonw@thestinger@alexcrichton@bors