Author: etnu
Date: Sat Nov 29 15:00:39 2008
New Revision: 721736
URL: http://svn.apache.org/viewvc?rev=721736&view=rev
Log:
Fixed a problem with cache size growth when different collection types are used.
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeatureRegistry.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingContentRewriter.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetFeatureRegistryTest.java
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeatureRegistry.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeatureRegistry.java?rev=721736&r1=721735&r2=721736&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeatureRegistry.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeatureRegistry.java
Sat Nov 29 15:00:39 2008
@@ -27,9 +27,11 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.logging.Logger;
/**
@@ -46,8 +48,7 @@
private final Map<String, GadgetFeature> core;
// Caches the transitive dependencies to enable faster lookups.
- private final Map<Collection<String>, Collection<GadgetFeature>> cache
- = Maps.newConcurrentHashMap();
+ final Map<Set<String>, Collection<GadgetFeature>> cache =
Maps.newConcurrentHashMap();
private boolean graphComplete = false;
@@ -125,27 +126,33 @@
public Collection<GadgetFeature> getFeatures(Collection<String> needed,
Collection<String> unsupported)
{
graphComplete = true;
+
+ Set<String> neededSet;
if (needed.isEmpty()) {
- needed = core.keySet();
+ neededSet = core.keySet();
+ } else {
+ neededSet = new HashSet<String>(needed);
}
+
// We use the cache only for situations where all needed are available.
// if any are missing, the result won't be cached.
- Collection<GadgetFeature> libCache = cache.get(needed);
+ Collection<GadgetFeature> libCache = cache.get(neededSet);
if (libCache != null) {
return libCache;
}
List<GadgetFeature> ret = new LinkedList<GadgetFeature>();
- populateDependencies(needed, ret);
+ populateDependencies(neededSet, ret);
// Fill in anything that was optional but missing. These won't be cached.
if (unsupported != null) {
- for (String feature : needed) {
+ for (String feature : neededSet) {
if (!features.containsKey(feature)) {
unsupported.add(feature);
}
}
}
if (unsupported == null || unsupported.isEmpty()) {
- cache.put(needed, Collections.unmodifiableList(ret));
+ cache.put(neededSet, Collections.unmodifiableList(ret));
+ logger.info("Added to cache. Size is now: " + cache.size());
}
return ret;
}
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingContentRewriter.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingContentRewriter.java?rev=721736&r1=721735&r2=721736&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingContentRewriter.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingContentRewriter.java
Sat Nov 29 15:00:39 2008
@@ -218,7 +218,6 @@
forced = Sets.newHashSet(forcedLibs.split(":"));
}
-
// Forced libs are always done first.
if (!forced.isEmpty()) {
String jsUrl = urlGenerator.getBundledJsUrl(forced, context);
@@ -227,6 +226,8 @@
headTag.appendChild(libsTag);
// Forced transitive deps need to be added as well so that they don't
get pulled in twice.
+ // Without this, a shared dependency between forced and non-forced libs
would get pulled into
+ // both the external forced script and the inlined script.
// TODO: Figure out a clean way to avoid having to call getFeatures
twice.
for (GadgetFeature dep : featureRegistry.getFeatures(forced)) {
forced.add(dep.getName());
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetFeatureRegistryTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetFeatureRegistryTest.java?rev=721736&r1=721735&r2=721736&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetFeatureRegistryTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetFeatureRegistryTest.java
Sat Nov 29 15:00:39 2008
@@ -20,8 +20,14 @@
package org.apache.shindig.gadgets;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+
+import edu.emory.mathcs.backport.java.util.Collections;
+
import org.junit.Before;
import org.junit.Test;
@@ -90,6 +96,33 @@
}
@Test
+ @SuppressWarnings("unchecked")
+ public void getFeaturesUsesCache() throws GadgetException {
+ registry.register(makeFeature(DEP_NAME, DEP_CONTENT, null));
+ registry.register(makeFeature("feat0", CONTENT, DEP_NAME));
+ registry.register(makeFeature("feat1", CONTENT, DEP_NAME));
+
+ Set<String> setKeys = Sets.immutableSortedSet("feat0", "feat1");
+ List<String> listKeys = Lists.newLinkedList("feat0", "feat1");
+ Collection<String> collectKeys
+ = Collections.unmodifiableCollection(Lists.newArrayList("feat0",
"feat1"));
+
+ // Fill the cache.
+ assertEquals(0, registry.cache.size());
+ registry.getFeatures(collectKeys);
+ assertEquals(1, registry.cache.size());
+
+ Collection<GadgetFeature> setFeatures = registry.getFeatures(setKeys);
+ assertEquals(1, registry.cache.size());
+ Collection<GadgetFeature> listFeatures = registry.getFeatures(listKeys);
+ assertEquals(1, registry.cache.size());
+ Collection<GadgetFeature> collectFeatures =
registry.getFeatures(collectKeys);
+ assertEquals(1, registry.cache.size());
+ assertSame(listFeatures, collectFeatures);
+ assertSame(setFeatures, listFeatures);
+ }
+
+ @Test
public void getAllFeatures() throws Exception {
for (String feature : FEATURE_LIST) {
registry.register(makeFeature(feature, CONTENT, DEP_NAME));
