-
Notifications
You must be signed in to change notification settings - Fork 3.8k
CASSANDRA-17402: Fix ObjectSizes implementation and usages #1470
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
CASSANDRA-17402: Fix ObjectSizes implementation and usages #1470
Conversation
7dae6fe to
86eb8e8
Compare
| public static long sizeOfEmptyHeapByteBuffer() | ||
| { | ||
| return BUFFER_EMPTY_SIZE; | ||
| int arrayLen = buffer.array().length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
| * @return Total in-memory size of the String | ||
| */ | ||
| //@TODO hard coding this to 2 isn't necessarily correct in Java 11 | ||
| // TODO hard coding this to 2 isn't necessarily correct in Java 11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this TODO comment still valid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I didn't change that logic at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the comment is mentioning the hardcoded "2" value, I understand that meaning the 2 bytes that were passed to MemoryLayoutSpecification.sizeOfArray. However, this has changed to send Character.SIZE, which has a value of 16 bits. So we are calling sizeOfArray with the size of a char in bits, when it expects bytes. I guess we should change it to Character.BYTES.
| import org.apache.cassandra.db.rows.NativeCell; | ||
| import org.apache.cassandra.schema.ColumnMetadata; | ||
| import org.apache.cassandra.schema.TableMetadata; | ||
| import org.apache.cassandra.utils.ByteBufferUtil; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: unused, can be fixed on commit
### What is the issue These metrics need to be customizable. ### What does this PR fix and why was it fixed Allow custom implementations via -Dcassandra.custom_internode_outbound_metrics_provider_class
…he#1470) These metrics need to be customizable. Allow custom implementations via -Dcassandra.custom_internode_outbound_metrics_provider_class
No description provided.