geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kl...@apache.org
Subject [47/57] [abbrv] incubator-geode git commit: GEODE-784: Offheap MemoryInspector refactor
Date Mon, 25 Jan 2016 17:45:22 GMT
GEODE-784: Offheap MemoryInspector refactor

Extracted MemoryInspector from SimpleMemoryAllocator and added tests


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/fc8726fa
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/fc8726fa
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/fc8726fa

Branch: refs/heads/feature/GEODE-819
Commit: fc8726fa28babff562d171fa5688f075a85a5a8d
Parents: 792cc48
Author: Swapnil Bawaskar <sbawaskar@pivotal.io>
Authored: Mon Jan 18 10:50:56 2016 -0800
Committer: Swapnil Bawaskar <sbawaskar@pivotal.io>
Committed: Fri Jan 22 10:06:17 2016 -0800

----------------------------------------------------------------------
 .../internal/offheap/MemoryBlockNode.java       |   2 +-
 .../internal/offheap/MemoryInspector.java       |   9 +-
 .../internal/offheap/MemoryInspectorImpl.java   |  99 +++++++++++++
 .../offheap/SimpleMemoryAllocatorImpl.java      |  80 ++---------
 .../offheap/MemoryInspectorImplJUnitTest.java   | 142 +++++++++++++++++++
 .../offheap/OffHeapValidationJUnitTest.java     |  10 +-
 .../offheap/SimpleMemoryAllocatorJUnitTest.java |  16 +--
 7 files changed, 269 insertions(+), 89 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/fc8726fa/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryBlockNode.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryBlockNode.java
b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryBlockNode.java
index 546feee..5c6182a 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryBlockNode.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryBlockNode.java
@@ -46,7 +46,7 @@ public class MemoryBlockNode implements MemoryBlock {
   }
   @Override
   public MemoryBlock getNextBlock() {
-    return this.ma.getBlockAfter(this);
+    return this.ma.getMemoryInspector().getBlockAfter(this);
   }
   public int getSlabId() {
     return this.ma.findSlab(getMemoryAddress());

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/fc8726fa/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryInspector.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryInspector.java
b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryInspector.java
index cde24bc..c904cd3 100755
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryInspector.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryInspector.java
@@ -26,9 +26,11 @@ import java.util.List;
  */
 public interface MemoryInspector {
 
-  public void clearInspectionSnapshot();
+  public void clearSnapshot();
   
-  public void createInspectionSnapshot();
+  public void createSnapshot();
+
+  public List<MemoryBlock> getSnapshot();
 
   public MemoryBlock getFirstBlock();
   
@@ -37,6 +39,5 @@ public interface MemoryInspector {
   public List<MemoryBlock> getAllocatedBlocks();
   
   public MemoryBlock getBlockAfter(MemoryBlock block);
-  
-  public List<MemoryBlock> getOrphans();
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/fc8726fa/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryInspectorImpl.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryInspectorImpl.java
b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryInspectorImpl.java
new file mode 100644
index 0000000..eda15cb
--- /dev/null
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/MemoryInspectorImpl.java
@@ -0,0 +1,99 @@
+/*
+ * 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 com.gemstone.gemfire.internal.offheap;
+
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Implementation of MemoryInspector that provides for inspection of meta-data
+ * for off-heap memory blocks
+ */
+public class MemoryInspectorImpl implements MemoryInspector {
+  /** The inspection snapshot for MemoryInspector */
+  private List<MemoryBlock> memoryBlocks;
+
+  private FreeListManager freeList;
+
+  public MemoryInspectorImpl(FreeListManager freeList) {
+    this.freeList = freeList;
+  }
+
+  @Override
+  public synchronized void clearSnapshot() {
+    this.memoryBlocks = null;
+  }
+
+  @Override
+  public synchronized void createSnapshot() {
+    List<MemoryBlock> value = this.memoryBlocks;
+    if (value == null) {
+      value = getOrderedBlocks();
+      this.memoryBlocks = value;
+    }
+  }
+
+  @Override
+  public synchronized List<MemoryBlock> getSnapshot() {
+    List<MemoryBlock> value = this.memoryBlocks;
+    if (value == null) {
+      return Collections.<MemoryBlock>emptyList();
+    } else {
+      return value;
+    }
+  }
+
+
+  @Override
+  public MemoryBlock getFirstBlock() {
+    final List<MemoryBlock> value = getSnapshot();
+    if (value.isEmpty()) {
+      return null;
+    } else {
+      return value.get(0);
+    }
+  }
+
+  @Override
+  public List<MemoryBlock> getAllBlocks() {
+    return getOrderedBlocks();
+  }
+
+  @Override
+  public List<MemoryBlock> getAllocatedBlocks() {
+    return this.freeList.getAllocatedBlocks();
+  }
+
+  @Override
+  public MemoryBlock getBlockAfter(MemoryBlock block) {
+    if (block == null) {
+      return null;
+    }
+    List<MemoryBlock> blocks = getSnapshot();
+    int nextBlock = blocks.indexOf(block) + 1;
+    if (nextBlock > 0 && blocks.size() > nextBlock) {
+      return blocks.get(nextBlock);
+    } else {
+      return null;
+    }
+  }
+
+  private List<MemoryBlock> getOrderedBlocks() {
+    return this.freeList.getOrderedBlocks();
+  }
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/fc8726fa/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/SimpleMemoryAllocatorImpl.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/SimpleMemoryAllocatorImpl.java
b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/SimpleMemoryAllocatorImpl.java
index d053797..12d297b 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/SimpleMemoryAllocatorImpl.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/offheap/SimpleMemoryAllocatorImpl.java
@@ -55,7 +55,7 @@ import com.gemstone.gemfire.internal.offheap.annotations.Unretained;
  * @author Kirk Lund
  * @since 9.0
  */
-public final class SimpleMemoryAllocatorImpl implements MemoryAllocator, MemoryInspector
{
+public final class SimpleMemoryAllocatorImpl implements MemoryAllocator {
 
   static final Logger logger = LogService.getLogger();
   
@@ -97,7 +97,9 @@ public final class SimpleMemoryAllocatorImpl implements MemoryAllocator,
MemoryI
   private final int largestSlab;
   
   public final FreeListManager freeList;
-  
+
+  private MemoryInspector memoryInspector;
+
   private volatile MemoryUsageListener[] memoryUsageListeners = new MemoryUsageListener[0];
   
   private static SimpleMemoryAllocatorImpl singleton = null;
@@ -292,6 +294,7 @@ public final class SimpleMemoryAllocatorImpl implements MemoryAllocator,
MemoryI
     this.stats.incFreeMemory(this.totalSlabSize);
     
     this.freeList = new FreeListManager(this);
+    this.memoryInspector = new MemoryInspectorImpl(this.freeList);
   }
   
   public List<Chunk> getLostChunks() {
@@ -588,39 +591,7 @@ public final class SimpleMemoryAllocatorImpl implements MemoryAllocator,
MemoryI
       }
     }
   }
-  
-  /** The inspection snapshot for MemoryInspector */
-  private List<MemoryBlock> memoryBlocks;
-  
-  @Override
-  public MemoryInspector getMemoryInspector() {
-    return this;
-  }
-  
-  @Override
-  public synchronized void clearInspectionSnapshot() {
-    this.memoryBlocks = null;
-  }
-  
-  @Override
-  public synchronized void createInspectionSnapshot() {
-    List<MemoryBlock> value = this.memoryBlocks;
-    if (value == null) {
-      value = getOrderedBlocks();
-      this.memoryBlocks = value;
-    }
-  }
 
-  synchronized List<MemoryBlock> getInspectionSnapshot() {
-    List<MemoryBlock> value = this.memoryBlocks;
-    if (value == null) {
-      return Collections.<MemoryBlock>emptyList();
-    } else {
-      return value;
-    }
-  }
-  
-  @Override
   public synchronized List<MemoryBlock> getOrphans() {
     List<Chunk> liveChunks = this.freeList.getLiveChunks();
     List<Chunk> regionChunks = getRegionLiveChunks();
@@ -629,55 +600,22 @@ public final class SimpleMemoryAllocatorImpl implements MemoryAllocator,
MemoryI
     for (Chunk chunk: liveChunks) {
       orphans.add(new MemoryBlockNode(this, chunk));
     }
-    Collections.sort(orphans, 
+    Collections.sort(orphans,
         new Comparator<MemoryBlock>() {
           @Override
           public int compare(MemoryBlock o1, MemoryBlock o2) {
             return Long.valueOf(o1.getMemoryAddress()).compareTo(o2.getMemoryAddress());
           }
-    });
+        });
     //this.memoryBlocks = new WeakReference<List<MemoryBlock>>(orphans);
     return orphans;
   }
-  
-  @Override
-  public MemoryBlock getFirstBlock() {
-    final List<MemoryBlock> value = getInspectionSnapshot();
-    if (value.isEmpty()) {
-      return null;
-    } else {
-      return value.get(0);
-    }
-  }
-  
-  @Override
-  public List<MemoryBlock> getAllBlocks() {
-    return getOrderedBlocks();
-  }
-  
-  @Override
-  public List<MemoryBlock> getAllocatedBlocks() {
-    return this.freeList.getAllocatedBlocks();
-  }
 
   @Override
-  public MemoryBlock getBlockAfter(MemoryBlock block) {
-    if (block == null) {
-      return null;
-    }
-    List<MemoryBlock> blocks = getInspectionSnapshot();
-    int nextBlock = blocks.indexOf(block) + 1;
-    if (nextBlock > 0 && blocks.size() > nextBlock) {
-      return blocks.get(nextBlock);
-    } else {
-      return null;
-    }
+  public MemoryInspector getMemoryInspector() {
+    return this.memoryInspector;
   }
 
-  private List<MemoryBlock> getOrderedBlocks() {
-    return this.freeList.getOrderedBlocks();
-  }
-  
   /*
    * Set this to "true" to perform data integrity checks on allocated and reused Chunks.
 This may clobber 
    * performance so turn on only when necessary.

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/fc8726fa/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/MemoryInspectorImplJUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/MemoryInspectorImplJUnitTest.java
b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/MemoryInspectorImplJUnitTest.java
new file mode 100644
index 0000000..b3eb01c
--- /dev/null
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/MemoryInspectorImplJUnitTest.java
@@ -0,0 +1,142 @@
+/*
+ * 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 com.gemstone.gemfire.internal.offheap;
+
+import com.gemstone.gemfire.test.junit.categories.UnitTest;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
+
+
+/**
+ * Created by sbawaskar on 1/15/16.
+ */
+@Category(UnitTest.class)
+public class MemoryInspectorImplJUnitTest {
+  private FreeListManager freeList;
+  private MemoryInspector inspector;
+  @Before
+  public void setUp() {
+    this.freeList = mock(FreeListManager.class);
+    this.inspector =  new MemoryInspectorImpl(freeList);
+  }
+
+  @Test
+  public void getSnapshotBeforeCreateSnapshotReturnsEmptyList() {
+    assertTrue(inspector.getSnapshot().isEmpty());
+  }
+
+  @Test
+  public void getAllBlocksBeforeCreateSnapshotReturnsEmptyList() {
+    assertTrue(inspector.getAllBlocks().isEmpty());
+  }
+
+  @Test
+  public void getAllocatedBlocksBeforeCreateSnapshotReturnsEmptyList() {
+    assertTrue(inspector.getAllocatedBlocks().isEmpty());
+  }
+  @Test
+  public void getFirstBlockBeforeCreateSnapshotReturnsNull() {
+    assertNull(inspector.getFirstBlock());
+  }
+
+  @Test
+  public void getBlockAfterWithNullBeforeCreateSnapshotReturnsNull() {
+    assertNull(inspector.getBlockAfter(null));
+  }
+
+  @Test
+  public void getBlockAfterBeforeCreateSnapshotReturnsNull() {
+    MemoryBlock block = mock(MemoryBlock.class);
+    assertNull(inspector.getBlockAfter(block));
+  }
+
+  @Test
+  public void canClearUncreatedSnapshot() {
+    inspector.clearSnapshot();
+    assertTrue(inspector.getSnapshot().isEmpty());
+  }
+
+  private void createSnapshot(List<MemoryBlock> memoryBlocks) {
+    when(this.freeList.getOrderedBlocks()).thenReturn(memoryBlocks);
+    inspector.createSnapshot();
+  }
+
+  @Test
+  public void createSnapshotCallsGetOrderedBlocks() {
+    List<MemoryBlock> emptyList = new ArrayList<MemoryBlock>();
+    createSnapshot(emptyList);
+    verify(this.freeList , times(1)).getOrderedBlocks();
+    assertSame(emptyList, inspector.getSnapshot());
+  }
+
+  @Test
+  public void createSnapshotIsIdempotent() {
+    List<MemoryBlock> emptyList = new ArrayList<MemoryBlock>();
+    createSnapshot(emptyList);
+    when(this.freeList.getOrderedBlocks()).thenReturn(null);
+    inspector.createSnapshot();
+    verify(this.freeList, times(1)).getOrderedBlocks();
+    assertSame(emptyList, inspector.getSnapshot());
+  }
+
+  @Test
+  public void clearSnapshotAfterCreatingOneReturnsEmptyList() {
+    List<MemoryBlock> emptyList = new ArrayList<MemoryBlock>();
+    createSnapshot(emptyList);
+
+    inspector.clearSnapshot();
+    assertTrue(inspector.getSnapshot().isEmpty());
+  }
+
+  @Test
+  public void getFirstBlockReturnsFirstBlockFromSnapshot() {
+    List<MemoryBlock> fakeSnapshot = setupFakeSnapshot();
+
+    assertSame(fakeSnapshot.get(0), inspector.getFirstBlock());
+  }
+
+  @Test
+  public void getFirstBlockAfterReturnsCorrectBlock() {
+    List<MemoryBlock> fakeSnapshot = setupFakeSnapshot();
+
+    assertSame(fakeSnapshot.get(1), inspector.getBlockAfter(fakeSnapshot.get(0)));
+  }
+
+  @Test
+  public void getFirstBlockAfterReturnsNullForLastBlock() {
+    List<MemoryBlock> fakeSnapshot = setupFakeSnapshot();
+
+    assertNull(inspector.getBlockAfter(fakeSnapshot.get(1)));
+  }
+
+  private List<MemoryBlock> setupFakeSnapshot() {
+    MemoryBlock mock1 = mock(MemoryBlock.class);
+    MemoryBlock mock2 = mock(MemoryBlock.class);
+    List<MemoryBlock> memoryBlocks = new ArrayList<>();
+    memoryBlocks.add(mock1);
+    memoryBlocks.add(mock2);
+    createSnapshot(memoryBlocks);
+    return memoryBlocks;
+  }
+}

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/fc8726fa/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/OffHeapValidationJUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/OffHeapValidationJUnitTest.java
b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/OffHeapValidationJUnitTest.java
index b9e8456..2d86296 100755
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/OffHeapValidationJUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/OffHeapValidationJUnitTest.java
@@ -115,7 +115,7 @@ public class OffHeapValidationJUnitTest {
     assertNotNull(allocator);
     MemoryInspector inspector = allocator.getMemoryInspector();
     assertNotNull(inspector);
-    inspector.createInspectionSnapshot();
+    inspector.createSnapshot();
     try {
       MemoryBlock firstBlock = inspector.getFirstBlock();
       assertNotNull(firstBlock);
@@ -130,7 +130,7 @@ public class OffHeapValidationJUnitTest {
       assertFalse(firstBlock.isCompressed());
       assertFalse(firstBlock.isSerialized());
     } finally {
-      inspector.clearInspectionSnapshot();
+      inspector.clearSnapshot();
     }
     
     // create off-heap region
@@ -179,12 +179,12 @@ public class OffHeapValidationJUnitTest {
     // TODO: PDX_INLINE_ENUM
     
     // validate inspection
-    inspector.createInspectionSnapshot();
+    inspector.createSnapshot();
     try {
     MemoryBlock firstBlock = inspector.getFirstBlock();
     assertEquals(MemoryBlock.State.UNUSED, firstBlock.getState());
     
-    //System.out.println(((SimpleMemoryAllocatorImpl)inspector).getInspectionSnapshot());
+    //System.out.println(((SimpleMemoryAllocatorImpl)inspector).getSnapshot());
     
     // sort the ExpectedValues into the same order as the MemberBlocks from inspector
     Collections.sort(expected, 
@@ -235,7 +235,7 @@ public class OffHeapValidationJUnitTest {
     }
     assertEquals("All blocks: "+inspector.getAllBlocks(), expected.size(), i);
     } finally {
-      inspector.clearInspectionSnapshot();
+      inspector.clearSnapshot();
     }
     
     // perform more ops

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/fc8726fa/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/SimpleMemoryAllocatorJUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/SimpleMemoryAllocatorJUnitTest.java
b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/SimpleMemoryAllocatorJUnitTest.java
index e9095ed..d9979cc 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/SimpleMemoryAllocatorJUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/SimpleMemoryAllocatorJUnitTest.java
@@ -413,14 +413,14 @@ public class SimpleMemoryAllocatorJUnitTest {
       MemoryInspector inspector = ma.getMemoryInspector();
       assertNotNull(inspector);
       assertEquals(null, inspector.getFirstBlock());
-      assertEquals(Collections.emptyList(), ma.getInspectionSnapshot());
-      assertEquals(Collections.emptyList(), ma.getAllocatedBlocks());
-      assertEquals(null, ma.getBlockAfter(null));
-      inspector.createInspectionSnapshot();
+      assertEquals(Collections.emptyList(), inspector.getSnapshot());
+      assertEquals(Collections.emptyList(), inspector.getAllocatedBlocks());
+      assertEquals(null, inspector.getBlockAfter(null));
+      inspector.createSnapshot();
       // call this twice for code coverage
-      inspector.createInspectionSnapshot();
+      inspector.createSnapshot();
       try {
-        assertEquals(ma.getAllBlocks(), ma.getInspectionSnapshot());
+        assertEquals(inspector.getAllBlocks(), inspector.getSnapshot());
         MemoryBlock firstBlock = inspector.getFirstBlock();
         assertNotNull(firstBlock);
         assertEquals(1024*1024, firstBlock.getBlockSize());
@@ -433,9 +433,9 @@ public class SimpleMemoryAllocatorJUnitTest {
         assertEquals(MemoryBlock.State.UNUSED, firstBlock.getState());
         assertFalse(firstBlock.isCompressed());
         assertFalse(firstBlock.isSerialized());
-        assertEquals(null, ma.getBlockAfter(firstBlock));
+        assertEquals(null, inspector.getBlockAfter(firstBlock));
       } finally {
-        inspector.clearInspectionSnapshot();
+        inspector.clearSnapshot();
       }
     } finally {
       SimpleMemoryAllocatorImpl.freeOffHeapMemory();


Mime
View raw message