nonbinaryprogrammer commented on a change in pull request #6296:
URL: https://github.com/apache/geode/pull/6296#discussion_r627600463
##########
File path:
geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisStringTest.java
##########
@@ -351,4 +354,95 @@ public void
setExpirationTimestamp_stores_delta_that_is_stable() throws IOExcept
o2.fromDelta(in);
assertThat(o2).isEqualTo(o1);
}
+
+ /************* Size in Bytes Tests *************/
+ /******* constructors *******/
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROSSize_ofLargeStrings() {
+ String javaString = makeStringOfSpecifiedSize(10_000);
+ RedisString string = new RedisString(new
ByteArrayWrapper(javaString.getBytes()));
+
+ int actual = string.getSizeInBytes();
+ int expected = reflectionObjectSizer.sizeof(string);
+
+ assertThat(actual).isEqualTo(expected);
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void should_calculateSize_closeToROSSize_ofStringOfVariousSizes() {
+ String javaString;
+ for (int i = 0; i < 512; i += 8) {
+ javaString = makeStringOfSpecifiedSize(i);
+ RedisString string = new RedisString(new
ByteArrayWrapper(javaString.getBytes()));
+
+ int expected = reflectionObjectSizer.sizeof(string);
+ int actual = string.getSizeInBytes();
+
+ assertThat(actual).isEqualTo(expected);
+ }
+ }
+
+ /******* changing values *******/
+ @Test
+ public void changingStringValue_toShorterString_shouldDecreaseSizeInBytes() {
+ String baseString = "baseString";
+ RedisString string = new RedisString(new ByteArrayWrapper((baseString +
"asdf").getBytes()));
+
+ int initialSize = string.getSizeInBytes();
+
+ string.set(new ByteArrayWrapper(baseString.getBytes()));
+
+ int finalSize = string.getSizeInBytes();
+
+ assertThat(finalSize).isEqualTo(initialSize - 4);
+ }
+
+ @Test
+ public void changingStringValue_toLongerString_shouldIncreaseSizeInBytes() {
+ String baseString = "baseString";
+ RedisString string = new RedisString(new
ByteArrayWrapper(baseString.getBytes()));
+
+ int initialSize = string.getSizeInBytes();
+
+ string.set(new ByteArrayWrapper((baseString + "asdf").getBytes()));
+
+ int finalSize = string.getSizeInBytes();
+
+ assertThat(finalSize).isEqualTo(initialSize + 4);
+ }
+
+ @Test
+ public void
changingStringValue_toEmptyString_shouldDecreaseSizeInBytes_toBaseSize() {
+ String baseString = "baseString";
+ RedisString string = new RedisString(new ByteArrayWrapper((baseString +
"asdf").getBytes()));
+
+ string.set(new ByteArrayWrapper("".getBytes()));
+
+ int finalSize = string.getSizeInBytes();
+
+ assertThat(finalSize).isEqualTo(BASE_REDIS_STRING_OVERHEAD);
+ }
+
+ /******* constants *******/
+ @Test
+ public void overheadConstants_shouldMatchCalculatedValue() {
+ assertThat(RedisString.PER_OBJECT_OVERHEAD).isEqualTo(8); // see todo in
RedisString
Review comment:
the TODO that I'm referring to is: `// TODO: for a 64bit jvm with small
ops this is 12; for other 64bit jvms it is 16`. If you command-click on the
`PER_OBJECT_OVERHEAD` constant, it's the TODO right above where that constant
is defined in the Sizeable interface. We do need to keep the constant, at least
for now, but if you feel that the test does not add value, I can remove that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
