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

Tighten the CountedBitSet class #27632

Merged
merged 3 commits into from
Dec 4, 2017
Merged

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Dec 3, 2017

Sorry @jasontedor for missing your comments in #27547. This PR addresses those comments.

This commit addresses the missed comments from elastic#27547.
@dnhatn dnhatn changed the title Tighten the CountedBitSet class - Follow-up of 27547 Tighten the CountedBitSet class - Followup of #27547 Dec 3, 2017
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

I left one suggestion but either way LGTM.

@@ -79,6 +82,7 @@ public void testReleaseInternalBitSet() {
assertThat(countedBitSet.isInternalBitsetReleased(), equalTo(true));
assertThat(countedBitSet.length(), equalTo(numBits));
assertThat(countedBitSet.cardinality(), equalTo(numBits));
assertThat(countedBitSet.ramBytesUsed(), lessThan(ramBytesUsedWithBitSet));
Copy link
Member

Choose a reason for hiding this comment

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

Make the constant package private and assert that the size here is BASE_RAM_BYTES_USED?

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed 8dbf71d

@jasontedor
Copy link
Member

Thanks @dnhatn!

@dnhatn
Copy link
Member Author

dnhatn commented Dec 4, 2017

Thanks @jasontedor.

@dnhatn dnhatn merged commit e213fa0 into elastic:master Dec 4, 2017
@dnhatn dnhatn deleted the bitset-followup branch December 4, 2017 14:51
@clintongormley clintongormley changed the title Tighten the CountedBitSet class - Followup of #27547 Tighten the CountedBitSet class Dec 11, 2017
@jpountz jpountz added the :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. label Jan 29, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. >enhancement v6.2.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants