Allon Mureinik has uploaded a new change for review.
Change subject: core, restapi: Remove double ReapedMaps
......................................................................
core, restapi: Remove double ReapedMaps
There are two almost identical (except for whitespaces and the
occasional formatting difference) classes called ReadpedMap in oVirt
Engine:
1. org.ovirt.engine.api.common.util.ReapedMap
2. org.ovirt.engine.core.utils.ReapedMap
This patch removes the api class, and leaves only the core one.
It includes:
1. Removal of the redundant class and deferring all usages to the
correct class.
2. Moving the test from the rest package to the core package.
3. Translating the test from EasyMock to Mockito, as used in the rest of
the core package tests.
Change-Id: I924355a495945a063b8e15f233188fc236df2432
Signed-off-by: Allon Mureinik <amure...@redhat.com>
---
M backend/manager/modules/restapi/interface/common/jaxrs/pom.xml
M
backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/resource/AbstractActionableResource.java
D
backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/ReapedMap.java
R
backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ReapedMapTest.java
4 files changed, 43 insertions(+), 301 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/22/12722/1
diff --git a/backend/manager/modules/restapi/interface/common/jaxrs/pom.xml
b/backend/manager/modules/restapi/interface/common/jaxrs/pom.xml
index dbc4e95..91f8e05 100644
--- a/backend/manager/modules/restapi/interface/common/jaxrs/pom.xml
+++ b/backend/manager/modules/restapi/interface/common/jaxrs/pom.xml
@@ -15,6 +15,11 @@
<dependencies>
<dependency>
+ <groupId>org.ovirt.engine.core</groupId>
+ <artifactId>utils</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
<groupId>org.ovirt.engine.api</groupId>
<artifactId>restapi-definition</artifactId>
<version>${project.version}</version>
diff --git
a/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/resource/AbstractActionableResource.java
b/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/resource/AbstractActionableResource.java
index f90e01b..424139c 100644
---
a/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/resource/AbstractActionableResource.java
+++
b/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/resource/AbstractActionableResource.java
@@ -23,17 +23,17 @@
import java.util.concurrent.Executor;
import javax.ws.rs.core.Response;
-import javax.ws.rs.core.UriInfo;
import javax.ws.rs.core.Response.Status;
+import javax.ws.rs.core.UriInfo;
import org.ovirt.engine.api.common.util.LinkHelper;
-import org.ovirt.engine.api.common.util.ReapedMap;
import org.ovirt.engine.api.common.util.StatusUtils;
import org.ovirt.engine.api.model.Action;
import org.ovirt.engine.api.model.BaseResource;
import org.ovirt.engine.api.model.CreationStatus;
import org.ovirt.engine.api.model.Fault;
import org.ovirt.engine.api.resource.ActionResource;
+import org.ovirt.engine.core.utils.ReapedMap;
public abstract class AbstractActionableResource<R extends BaseResource>
extends AbstractUpdatableResource<R> {
@@ -91,6 +91,7 @@
action.setStatus(StatusUtils.create(CreationStatus.PENDING));
actions.put(action.getId(), actionResource);
executor.execute(new Runnable() {
+ @Override
public void run() {
perform(task);
actions.reapable(actionResource.getAction().getId());
@@ -169,6 +170,7 @@
this.reason = reason;
}
+ @Override
public void run() {
try {
execute();
@@ -206,6 +208,7 @@
public DoNothingTask(Action action) {
super(action);
}
+ @Override
public void execute(){
}
}
@@ -214,6 +217,7 @@
* Fallback executor, starts a new thread for each task.
*/
protected static class SimpleExecutor implements Executor {
+ @Override
public void execute(Runnable task) {
new Thread(task).start();
}
diff --git
a/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/ReapedMap.java
b/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/ReapedMap.java
deleted file mode 100644
index 968acb0..0000000
---
a/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/ReapedMap.java
+++ /dev/null
@@ -1,259 +0,0 @@
-/*
-* Copyright (c) 2010 Red Hat, Inc.
-*
-* Licensed 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.ovirt.engine.api.common.util;
-
-import java.lang.ref.ReferenceQueue;
-import java.lang.ref.SoftReference;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.LinkedHashMap;
-import java.util.Map;
-
-
-/**
- * Models a strongly-referenced primary hash map, coupled with a reapable
- * secondary map based on soft-referenced values (rather than keys, as is
- * the case with the java.util.WeakHashMap).
- *
- * Use this like a normal hash map, but when values need no longer be
- * retained they should be marked as reapable so that they are made eligible
- * for garbage collection. Entries are finally evicted if not already GC'd
- * before the expiry of the reapAfter timeout (calculated either from the
- * point at which it was marked reapable or the last time of access).
- * Freeing of the entry prior to this timeout expiring is at the discretion
- * of the garbage collector and depends on whether the JVM is experiencing
- * memory pressure, the type of JVM selected (the client JVM uses much more
- * aggressive GC policies that the server variant) and also the JVM options
- * controlling the heap size.
- *
- * REVISIT: does the world really need yet another hand-rolled cache type?
- * REVISIT: inherited entrySet() etc. don't take account of secondary
- *
- * @param <K> key type
- * @param <V> value type
- */
-public class ReapedMap<K, V> extends HashMap<K, V> {
-
- // FIXME: I just made up this UID
- static final long serialVersionUID = 12345678987654321L;
-
- private static Long DEFAULT_REAP_AFTER = 10 * 60 * 1000L; // 10 minutes
-
- private long reapAfter;
- private boolean accessBasedAging;
- private ReferenceQueue<V> queue;
-
- // Secondary Map, note:
- // - keys are strongly referenced, as GC of corresponding values
- // will trigger their release
- // - reap requires a predictable iteration order (based on insertion order)
- // hence the use of LinkedHasMap
- //
- LinkedHashMap<K, IdAwareReference<K, V>> reapableMap;
-
- public ReapedMap() {
- this(DEFAULT_REAP_AFTER);
- }
-
- /**
- * @param reapAfter entries become eligible for reaping after this
duration (ms)
- */
- public ReapedMap(long reapAfter) {
- this(reapAfter, false);
- }
-
- /**
- * @param reapAfter entries become eligible for reaping after this
duration (ms)
- * @param accessBasedAging reset reapAfter timeout on each access
- */
- public ReapedMap(long reapAfter, boolean accessBasedAging) {
- this(reapAfter, accessBasedAging, new ReferenceQueue<V>());
- }
-
- /**
- * Package-protected constructor intended for test use.
- *
- * @param reapAfter entries become eligible for reaping after this
duration (ms)
- * @param accessBasedAging reset reapAfter timeout on each access
- * @param queue reference queue to avoid leaked mappings in case where
- * aggressive GC eats referent before it is reaped
- */
- ReapedMap(long reapAfter, boolean accessBasedAging, ReferenceQueue<V>
queue) {
- this.reapAfter = reapAfter;
- this.accessBasedAging = accessBasedAging;
- this.queue = queue;
- reapableMap = new LinkedHashMap<K, IdAwareReference<K, V>>();
- }
-
- @Override
- public synchronized V get(Object key) {
- V ret = super.get(key);
- if (ret == null) {
- IdAwareReference<K, V> ref = accessBasedAging ?
reapableMap.remove(key) : reapableMap.get(key);
- if (ref != null) {
- if (ref.isEnqueued()) {
- ref.clear();
- reapableMap.remove(key);
- } else {
- ret = ref.get();
- if (ret == null) {
- reapableMap.remove(key);
- } else if (accessBasedAging) {
- // re-insert on timestamp reset so
- // as to maintain insertion order
- reapableMap.put(ref.key, ref.reset());
- }
- }
- }
- }
- reap();
- return ret;
- }
-
- @Override
- public synchronized V put(K k, V v) {
- reap();
- return super.put(k, v);
- }
-
- @Override
- public synchronized V remove(Object key) {
- V ret = super.remove(key);
- if (ret == null) {
- IdAwareReference<K, V> ref = reapableMap.remove(key);
- if (ref != null) {
- if (ref.isEnqueued()) {
- ref.clear();
- } else {
- ret = ref.get();
- }
- }
- }
- reap();
- return ret;
- }
-
- @Override
- public synchronized void clear() {
- super.clear();
- reapableMap.clear();
- while (queue.poll() != null)
- ;
- }
-
- /**
- * Mark a key as being reapable, caching corresponding soft reference to
- * corresponding value in the secondary map.
- *
- * @param k
- */
- public synchronized void reapable(K k) {
- V v = super.remove(k);
- if (v != null) {
- reapableMap.put(k, new IdAwareReference<K, V>(k, v, queue));
- }
- reap();
- }
-
- /**
- * @return the size of the secondary map
- */
- public synchronized long reapableSize() {
- return reapableMap.size();
- }
-
- /**
- * Reap <i>before</i> additive operations, <i>after</i> for neutral and
- * destructive ones.
- */
- private synchronized void reap() {
-
- // reap entries older than age permitted
- //
- long now = System.currentTimeMillis();
- Iterator<Map.Entry<K, IdAwareReference<K, V>>> entries =
reapableMap.entrySet().iterator();
- while (entries.hasNext()) {
- Map.Entry<K, IdAwareReference<K, V>> entry = entries.next();
- IdAwareReference<K, V> v = entry.getValue();
- if (now - v.timestamp > reapAfter) {
- entries.remove();
- entry.getValue().clear();
- entry.setValue(null);
- } else {
- // guaranteed iteration on insertion order => no older entries
- //
- break;
- }
- }
-
- // poll reference queue for GC-pending references to trigger
- // reaping of referent
- //
- Object ref = null;
- while ((ref = queue.poll()) != null) {
- @SuppressWarnings("unchecked")
- IdAwareReference<K, V> value = (IdAwareReference<K, V>)ref;
- if (value != null) {
- reapableMap.remove(value.getKey());
- }
- }
- }
-
- /**
- * Encapsulate key and timestamp (the latter is used for
- * eager reaping). The reference queue provides access to
- * finalizable instances of the reference type, not the
- * class wrapping it. Hence we must extend SoftReference
- * as opposed to encapsulating it.
- */
- static class IdAwareReference<T, S> extends SoftReference<S> {
- long timestamp;
- long last;
- T key;
-
- IdAwareReference(T key, S value, ReferenceQueue<S> queue) {
- super(value, queue);
- this.key = key;
- last = timestamp = System.currentTimeMillis();
- }
-
- public T getKey() {
- return key;
- }
-
- public boolean equals(Object other) {
- boolean ret = false;
- S one = null;
- ret = other == this
- || (other instanceof SoftReference<?>
- && (one = get()) != null
- && one.equals(((SoftReference<?>)other).get()));
- return ret;
- }
-
- public int hashCode() {
- S one = get();
- return one != null ? one.hashCode() : 0;
- }
-
- private IdAwareReference<T, S> reset() {
- last = timestamp;
- timestamp = System.currentTimeMillis();
- return this;
- }
- }
-}
diff --git
a/backend/manager/modules/restapi/interface/common/jaxrs/src/test/java/org/ovirt/engine/api/common/util/ReapableMapTest.java
b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ReapedMapTest.java
similarity index 72%
rename from
backend/manager/modules/restapi/interface/common/jaxrs/src/test/java/org/ovirt/engine/api/common/util/ReapableMapTest.java
rename to
backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ReapedMapTest.java
index 9e644c5..dc2dad5 100644
---
a/backend/manager/modules/restapi/interface/common/jaxrs/src/test/java/org/ovirt/engine/api/common/util/ReapableMapTest.java
+++
b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ReapedMapTest.java
@@ -14,33 +14,28 @@
* limitations under the License.
*/
-package org.ovirt.engine.api.common.util;
+package org.ovirt.engine.core.utils;
-import static org.easymock.EasyMock.expect;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import java.lang.ref.Reference;
import java.lang.ref.ReferenceQueue;
-import org.easymock.IExpectationSetters;
-import org.easymock.classextension.EasyMock;
-import org.easymock.classextension.IMocksControl;
import org.junit.Assert;
-import org.junit.Before;
import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.ovirt.engine.core.utils.ReapedMap.IdAwareReference;
-public class ReapableMapTest extends Assert {
+public class ReapedMapTest extends Assert {
private static final String[] NUMBERS = { "one", "two", "three", "four",
"five" };
private static final NumberMapper MAPPER = new NumberMapper();
- private IMocksControl control;
private ReapedMap<String, Integer> map;
-
- @Before
- public void setUp() {
- control = EasyMock.createNiceControl();
- }
@Test
public void testReapingWithoutGC() throws Exception {
@@ -78,8 +73,6 @@
assertExpected(3);
assertSizes(1, 1);
assertNull(map.get("three"));
-
- control.verify();
}
@Test
@@ -106,8 +99,6 @@
assertExpected(2);
assertSizes(1, 0);
assertNull(map.get("three"));
-
- control.verify();
}
@Test
@@ -121,8 +112,6 @@
populate(4);
assertSizes(2, 1);
assertNull(map.get("three"));
-
- control.verify();
}
@Test
@@ -136,36 +125,39 @@
map.remove("two");
assertSizes(0, 1);
assertNull(map.get("three"));
-
- control.verify();
}
@SuppressWarnings({ "unchecked", "rawtypes" })
- private void setUpGCExpectations(int gcAfter) {
- ReferenceQueue<Integer> queue =
(ReferenceQueue<Integer>)control.createMock(ReferenceQueue.class);
+ private void setUpGCExpectations(final int gcAfter) {
+ ReferenceQueue<Integer> queue = mock(ReferenceQueue.class);
map = new ReapedMap<String, Integer>(10000, false, queue);
- for (int i = 0 ; i < gcAfter ; i++) {
- expect(queue.poll()).andReturn(null);
- }
- // the gcAfter^th queue poll simulates a GC event and triggers deletion
- // on the reapable map
- ReapedMap.IdAwareReference<String, Integer> ref =
control.createMock(ReapedMap.IdAwareReference.class);
- // awkward syntax required to work around compilation error
- // on Reference<capture#nnn ? extends Integer> mismatch
- ((IExpectationSetters) expect(queue.poll())).andReturn(ref);
- IExpectationSetters<? extends String> refSetter = expect(ref.getKey());
- ((IExpectationSetters<String>)refSetter).andReturn("three");
- control.replay();
+ final IdAwareReference ref = mock(IdAwareReference.class);
+ when(ref.getKey()).thenReturn("three", new Object[] { null });
+
+ // the gcAfter queue poll simulates a GC event and triggers deletion
+ // on the reapable map
+ when(queue.poll()).thenAnswer(new Answer<Reference<Integer>>() {
+ private int times = 0;
+
+ @Override
+ public Reference<Integer> answer(InvocationOnMock invocation)
throws Throwable {
+ if (times == gcAfter) {
+ ++times;
+ return ref;
+ }
+
+ ++times;
+ return null;
+ }
+ });
}
@SuppressWarnings("unchecked")
private void setUpAccessBaseAgingExpectations() {
- ReferenceQueue<Integer> queue =
(ReferenceQueue<Integer>)control.createMock(ReferenceQueue.class);
+ ReferenceQueue<Integer> queue = mock(ReferenceQueue.class);
map = new ReapedMap<String, Integer>(1000, true, queue);
- expect(queue.poll()).andReturn(null).anyTimes();
-
- control.replay();
+ when(queue.poll()).thenReturn(null);
}
private void populate(Integer ... values) {
@@ -189,5 +181,5 @@
public String getKey(Integer i) {
return i <= NUMBERS.length ? NUMBERS[i-1] : null;
}
- };
+ }
}
--
To view, visit http://gerrit.ovirt.org/12722
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I924355a495945a063b8e15f233188fc236df2432
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <amure...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
