You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by up...@apache.org on 2021/05/05 16:31:58 UTC
[geode] branch develop updated: GEODE-9217: Adding memory overhead
tests for redis (#6405)
This is an automated email from the ASF dual-hosted git repository.
upthewaterspout pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 73e2f35 GEODE-9217: Adding memory overhead tests for redis (#6405)
73e2f35 is described below
commit 73e2f357e534b281cf9bd84828a364d83830a5a3
Author: Dan Smith <da...@vmware.com>
AuthorDate: Wed May 5 09:30:45 2021 -0700
GEODE-9217: Adding memory overhead tests for redis (#6405)
Adding integration tests that measure the memory overhead of both native redis
and radish. These tests measure the memory used before and after adding
entries. They assert that the measured per entry overhead is within 7 bytes of
the expected per entry overhead. They are measuring the all of the overhead -
whether it's from geode region entries or our radish data structures.
To get these tests to work on JDK 11, we added a few more --add-opens
to test.gradle. The ObjectGraphSizer was also fixed to exclude PhantomReferences,
which JDK 11 now uses in DirectByteBuffers.
---
.../MemoryOverheadNativeRedisAcceptanceTest.java | 60 +++++
.../AbstractMemoryOverheadIntegrationTest.java | 253 +++++++++++++++++++++
.../hash/MemoryOverheadIntegrationTest.java | 104 +++++++++
.../geode/internal/size/ObjectGraphSizer.java | 4 +-
gradle/test.gradle | 7 +
5 files changed, 427 insertions(+), 1 deletion(-)
diff --git a/geode-apis-compatible-with-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/hash/MemoryOverheadNativeRedisAcceptanceTest.java b/geode-apis-compatible-with-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/hash/MemoryOverheadNativeRedisAcceptanceTest.java
new file mode 100755
index 0000000..8b797bf
--- /dev/null
+++ b/geode-apis-compatible-with-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/hash/MemoryOverheadNativeRedisAcceptanceTest.java
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.hash;
+
+import static org.junit.Assert.assertTrue;
+
+import java.util.EnumMap;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.junit.ClassRule;
+
+import org.apache.geode.NativeRedisTestRule;
+
+public class MemoryOverheadNativeRedisAcceptanceTest extends AbstractMemoryOverheadIntegrationTest {
+
+ private static final Pattern FIND_USED_MEMORY = Pattern.compile("used_memory:(\\d+)");
+
+ @ClassRule
+ public static NativeRedisTestRule redis = new NativeRedisTestRule();
+
+ @Override
+ EnumMap<Measurement, Integer> expectedPerEntryOverhead() {
+ EnumMap<Measurement, Integer> result = new EnumMap<>(Measurement.class);
+ result.put(Measurement.STRING, 61);
+ result.put(Measurement.SET, 223);
+ result.put(Measurement.SET_ENTRY, 75);
+ result.put(Measurement.HASH, 228);
+ result.put(Measurement.HASH_ENTRY, 70);
+
+ return result;
+ }
+
+
+ @Override
+ public int getPort() {
+ return redis.getPort();
+ }
+
+ @Override
+ long getUsedMemory() {
+ String memoryInfo = jedis.info("memory");
+ Matcher matcher = FIND_USED_MEMORY.matcher(memoryInfo);
+ assertTrue(matcher.find());
+ String usedMemory = matcher.group(1);
+ return Long.parseLong(usedMemory);
+ }
+}
diff --git a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractMemoryOverheadIntegrationTest.java b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractMemoryOverheadIntegrationTest.java
new file mode 100755
index 0000000..5d31ab1
--- /dev/null
+++ b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractMemoryOverheadIntegrationTest.java
@@ -0,0 +1,253 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.hash;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.EnumMap;
+
+import org.assertj.core.data.Offset;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+
+import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.dunit.rules.RedisPortSupplier;
+
+/**
+ * Contains tests that measure the used memory of redis or radish and assert that the memory
+ * overhead is equal to an expected memory overhead.
+ *
+ * @see MemoryOverheadIntegrationTest
+ * @see #measureAndCheckPerEntryOverhead(AddEntryFunction, Measurement) (IntToLongFunction,
+ * Measurement)
+ */
+public abstract class AbstractMemoryOverheadIntegrationTest implements RedisPortSupplier {
+
+ private static final int WARM_UP_ENTRY_COUNT = 1000;
+ private static final int TOTAL_ENTRY_COUNT = 5000;
+ private static final int SAMPLE_INTERVAL = 100;
+ private static final int REDIS_CLIENT_TIMEOUT =
+ Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+
+ /**
+ * A value that force native redis to not use an optimized data structure.
+ * Native redis uses an optimized data structure (a zip list) for very small hashes and sets. This
+ * will allow us to force redis not to use a ziplist.
+ */
+ public static final String LARGE_STRING =
+ "value_that_will_force_redis_to_not_use_a_ziplist______________________________________________________________";
+
+ /**
+ * Allowed variance in our measurements before the test fails. This allows us to be up to
+ * 7 bytes off from the original measurement.
+ */
+ public static final long ALLOWED_BYTE_DIFFERENCE = 7L;
+ protected Jedis jedis;
+
+ protected enum Measurement {
+ STRING,
+ HASH,
+ HASH_ENTRY,
+ SET,
+ SET_ENTRY
+ }
+
+ @Before
+ public void setUp() {
+ jedis = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+ }
+
+ @After
+ public void tearDown() {
+ jedis.flushAll();
+ jedis.close();
+ }
+
+ /**
+ * Subclasses should use this to return the expected per entry overhead for each measurement.
+ *
+ * @return A map with the expected overhead for each measurement type.
+ */
+ abstract EnumMap<Measurement, Integer> expectedPerEntryOverhead();
+
+ /**
+ * Return the total used memory on the server.
+ */
+ abstract long getUsedMemory();
+
+ /**
+ * Measure the overhead for each redis string that is added to the server.
+ */
+ @Test
+ public void measureOverheadPerString() {
+ // Function that adds a new redis string to the server
+ final AddEntryFunction addStringFunction = uniqueString -> {
+ String response = jedis.set(uniqueString, uniqueString);
+ assertThat(response).isEqualTo("OK");
+
+ // Note - jedis convert strings to bytes with the UTF-8 charset
+ // Since the strings above are all ASCII, the length == the number of bytes
+ return uniqueString.length() + uniqueString.length();
+ };
+
+ measureAndCheckPerEntryOverhead(addStringFunction, Measurement.STRING);
+ }
+
+ /**
+ * Measure the overhead for each redis hash that is added to the server.
+ */
+ @Test
+ public void measureOverheadPerHash() {
+ // Function that adds a new redis hash to the server
+ final AddEntryFunction addHashFunction = uniqueString -> {
+ String mapKey = "key";
+ Long response = jedis.hset(uniqueString, mapKey, LARGE_STRING);
+ assertThat(response).isEqualTo(1);
+ return uniqueString.length() + mapKey.length() + LARGE_STRING.length();
+ };
+
+ measureAndCheckPerEntryOverhead(addHashFunction, Measurement.HASH);
+ }
+
+ /**
+ * Measure the overhead for each entry that is added to a redis hash. This
+ * uses a single hash and adds additional fields to the hash and measures the overhead
+ * of the additional fields.
+ */
+ @Test
+ public void measureOverheadPerHashEntry() {
+ // Function that adds an additional hash entry to a single redis hash
+ final AddEntryFunction addHashEntryFunction = uniqueString -> {
+
+ String valueString = String.format("%s value-%s", LARGE_STRING, uniqueString);
+ Long response = jedis.hset("TestSet", uniqueString, valueString);
+ assertThat(response).isEqualTo(1);
+
+ return uniqueString.length() + valueString.length();
+ };
+
+ measureAndCheckPerEntryOverhead(addHashEntryFunction, Measurement.HASH_ENTRY);
+ }
+
+ /**
+ * Measure the overhead for each redis set that is added to the server.
+ */
+ @Test
+ public void measureOverheadPerSet() {
+ // Function that adds a new redis set to the server
+ final AddEntryFunction addSetFunction = uniqueString -> {
+ Long response = jedis.sadd(uniqueString, LARGE_STRING);
+ assertThat(response).isEqualTo(1);
+ return uniqueString.length() + LARGE_STRING.length();
+ };
+
+ measureAndCheckPerEntryOverhead(addSetFunction, Measurement.SET);
+ }
+
+ /**
+ * Measure the overhead for each entry that is added to a redis set. This
+ * uses a single sets and adds additional fields to the hash and measures the overhead
+ * of the additional fields.
+ */
+ @Test
+ public void measureOverheadPerSetEntry() {
+ // Function that adds a new entry to a single redis set
+ final AddEntryFunction addSetEntryFunction = uniqueString -> {
+
+ String valueString = String.format("%s value-%s", LARGE_STRING, uniqueString);
+ Long response = jedis.sadd("TestSet", valueString);
+ assertThat(response).isEqualTo(1);
+
+ return valueString.length();
+ };
+
+ measureAndCheckPerEntryOverhead(addSetEntryFunction, Measurement.SET_ENTRY);
+ }
+
+ /**
+ * Measures the per entry overhead of a particular operation. The passed in function
+ * should add a single value to the server using the given uniqueString. This method
+ * will call that function repeatedly and measure the memory use.
+ *
+ * This method measures the total memory use of the server (radish or redis)
+ * before and after adding a certain number of entries, and computes the per entry overhead.
+ * It asserts that the overhead matches the result of {@link #expectedPerEntryOverhead()} for
+ * the given measurement
+ *
+ * @param addEntry A function that adds an entry to the server through some redis operation. The
+ * function should return the amount of actual user data added. This user data
+ * size will be subtracted out to compute the pure overhead of redis or radish
+ * data structures.
+ * @param measurement Indicates what data structure we are measuring. Used to look up the expected
+ * memory usage.
+ */
+ private void measureAndCheckPerEntryOverhead(AddEntryFunction addEntry, Measurement measurement) {
+
+ long expectedOverhead = expectedPerEntryOverhead().get(measurement);
+
+ // Put some entries to make sure we initialize any constant size data structures. We are
+ // just trying to measure the cost of each add entry operation.
+ for (int i = 0; i < WARM_UP_ENTRY_COUNT; i++) {
+ String uniqueString = String.format("warmup-%10d", i);
+ addEntry.addEntryAndReturnDataSize(uniqueString);
+ }
+
+ // Perform measurements
+ long baseline = getUsedMemory();
+ long totalDataSize = 0;
+ System.out.println("Measuring the per entry overhead for each " + measurement);
+ System.out.printf("%20s, %20s, %20s", "Used Memory", "Total Mem Per Entry",
+ "Overhead Per Entry\n");
+ long perEntryOverhead = 0;
+ for (int i = 0; i < TOTAL_ENTRY_COUNT; i++) {
+ String uniqueString = String.format("%10d", i);
+ totalDataSize += addEntry.addEntryAndReturnDataSize(uniqueString);
+ if (i % SAMPLE_INTERVAL == (SAMPLE_INTERVAL - 1)) {
+ long currentMemory = getUsedMemory() - baseline;
+ long perEntryMemory = currentMemory / i;
+ perEntryOverhead = (currentMemory - totalDataSize) / i;
+ System.out.printf("%20d, %20d, %20d\n", currentMemory, perEntryMemory, perEntryOverhead);
+ }
+ }
+
+ // These assertions compare the computed per entry overhead against result of the
+ // expectedPerEntryOverhead function. Please look at that function for the expected values.
+ // We are allow these values to be off by 1 byte due to rounding issues
+ assertThat(perEntryOverhead).withFailMessage(
+ "The overhead per %s has increased from %s to %s. Please see if you can avoid introducing additional memory overhead.",
+ measurement, expectedOverhead, perEntryOverhead)
+ .isLessThanOrEqualTo(expectedOverhead + ALLOWED_BYTE_DIFFERENCE);
+
+ assertThat(perEntryOverhead).withFailMessage(
+ "The overhead per %s has decreased from %s to %s. Great job! Please update the expected value in this test.",
+ measurement, expectedOverhead, perEntryOverhead)
+ .isCloseTo(expectedOverhead, Offset.offset(ALLOWED_BYTE_DIFFERENCE));
+ }
+
+ @FunctionalInterface
+ public interface AddEntryFunction {
+ /**
+ * Add an entry to the server.
+ *
+ * @param uniqueString A unique string than can be used to generate a unique key
+ * for the entry.
+ * @return the size of the user data added by the operation
+ */
+ long addEntryAndReturnDataSize(String uniqueString);
+
+ }
+}
diff --git a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/MemoryOverheadIntegrationTest.java b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/MemoryOverheadIntegrationTest.java
new file mode 100755
index 0000000..506365f
--- /dev/null
+++ b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/MemoryOverheadIntegrationTest.java
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.hash;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.EnumMap;
+
+import javax.management.MBeanServer;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.springframework.shell.converters.AvailableCommandsConverter;
+
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.distributed.ConfigurationProperties;
+import org.apache.geode.internal.JvmSizeUtils;
+import org.apache.geode.internal.cache.BucketRegion;
+import org.apache.geode.internal.cache.PartitionedRegion;
+import org.apache.geode.internal.cache.RegionEntry;
+import org.apache.geode.internal.size.ObjectGraphSizer;
+import org.apache.geode.redis.GeodeRedisServerRule;
+import org.apache.geode.redis.internal.RegionProvider;
+
+public class MemoryOverheadIntegrationTest extends AbstractMemoryOverheadIntegrationTest {
+ protected static ObjectGraphSizer.ObjectFilter filter = new SkipBrokenClassesFilter();
+
+ @Rule
+ public GeodeRedisServerRule server = new GeodeRedisServerRule()
+ .withProperty(ConfigurationProperties.LOG_LEVEL, "error");
+
+ @Before
+ public void checkJVMPlatform() {
+ // Fail
+ assertThat(JvmSizeUtils.is64Bit())
+ .withFailMessage("The expected values for these tests was computed for a 64 bit JVM")
+ .isTrue();
+ }
+
+ @Override
+ EnumMap<Measurement, Integer> expectedPerEntryOverhead() {
+ EnumMap<Measurement, Integer> result = new EnumMap<>(Measurement.class);
+ result.put(Measurement.STRING, 201);
+ result.put(Measurement.SET, 386);
+ result.put(Measurement.SET_ENTRY, 72);
+ result.put(Measurement.HASH, 554);
+ result.put(Measurement.HASH_ENTRY, 106);
+
+ return result;
+ }
+
+ /**
+ * Print out a histogram of one of our redis keys. This may help track down where memory
+ * usage is coming from.
+ */
+ @After
+ public void printHistogramOfOneRedisKey() throws IllegalAccessException {
+ final PartitionedRegion dataRegion =
+ (PartitionedRegion) CacheFactory.getAnyInstance()
+ .getRegion(RegionProvider.REDIS_DATA_REGION);
+ final Object redisKey = dataRegion.keys().iterator().next();
+ BucketRegion bucket = dataRegion.getBucketRegion(redisKey);
+ RegionEntry entry = bucket.entries.getEntry(redisKey);
+ System.out.println("----------------------------------------------------");
+ System.out.println("Histogram of memory usage of first region entry");
+ System.out.println("----------------------------------------------------");
+ System.out.println(ObjectGraphSizer.histogram(entry, false));
+ }
+
+ @Override
+ public int getPort() {
+ return server.getPort();
+ }
+
+ @Override
+ long getUsedMemory() {
+ try {
+ return ObjectGraphSizer.size(server.getServer(), filter, false);
+ } catch (IllegalAccessException e) {
+ throw new RuntimeException("Couldn't compute size of cache", e);
+ }
+ }
+
+ private static class SkipBrokenClassesFilter implements ObjectGraphSizer.ObjectFilter {
+ @Override
+ public boolean accept(Object parent, Object object) {
+ return !(object instanceof AvailableCommandsConverter)
+ && !(object instanceof MBeanServer);
+ }
+ }
+}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/size/ObjectGraphSizer.java b/geode-core/src/main/java/org/apache/geode/internal/size/ObjectGraphSizer.java
index 82b2e9f..35b049e 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/size/ObjectGraphSizer.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/size/ObjectGraphSizer.java
@@ -14,6 +14,7 @@
*/
package org.apache.geode.internal.size;
+import java.lang.ref.PhantomReference;
import java.lang.ref.SoftReference;
import java.lang.ref.WeakReference;
import java.util.HashMap;
@@ -229,7 +230,8 @@ public class ObjectGraphSizer {
// We do want to include the size of the reference itself, but
// we don't visit the children because they will be GC'd if there is no
// other reference
- return !(object instanceof WeakReference) && !(object instanceof SoftReference);
+ return !(object instanceof WeakReference) && !(object instanceof SoftReference)
+ && !(object instanceof PhantomReference);
}
public long getTotalSize() {
diff --git a/gradle/test.gradle b/gradle/test.gradle
index 48dcd32..d0ea9f4 100644
--- a/gradle/test.gradle
+++ b/gradle/test.gradle
@@ -173,6 +173,13 @@ gradle.taskGraph.whenReady({ graph ->
jvmArgs += ["--add-opens", "java.xml/jdk.xml.internal=ALL-UNNAMED"]
jvmArgs += ["--add-opens", "java.base/jdk.internal.module=ALL-UNNAMED"]
jvmArgs += ["--add-opens", "java.base/java.lang.module=ALL-UNNAMED"]
+ jvmArgs += ["--add-opens", "java.base/jdk.internal.ref=ALL-UNNAMED"]
+ jvmArgs += ["--add-opens", "java.base/jdk.internal.reflect=ALL-UNNAMED"]
+ jvmArgs += ["--add-opens", "java.base/jdk.internal.loader=ALL-UNNAMED"]
+ jvmArgs += ["--add-opens", "java.base/jdk.internal.misc=ALL-UNNAMED"]
+ jvmArgs += ["--add-opens", "java.base/jdk.internal.util.jar=ALL-UNNAMED"]
+ jvmArgs += ["--add-opens", "jdk.management/com.sun.management.internal=ALL-UNNAMED"]
+ jvmArgs += ["--add-opens", "java.base/jdk.internal.platform.cgroupv1=ALL-UNNAMED"]
}
if (project.hasProperty('testJVM') && !testJVM.trim().isEmpty()) {
executable = "${testJVM}/bin/java"