Rollback of issue 887801
*** Reason for Rollback ***
Received a report that this changes breaks hosted mode for some users.
*** Original change description ***
Optimize ResourceOracle refresh by doing multiple oracles at the same time
Review at http://gwt-code-reviews.appspot.com/887801
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@8801 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java b/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java
index da9dcb8..06fd472 100644
--- a/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java
+++ b/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java
@@ -340,7 +340,7 @@
NON_JAVA_RESOURCES, pathPrefix.shouldReroot()));
}
lazyResourcesOracle.setPathPrefixes(newPathPrefixes);
- ResourceOracleImpl.refresh(TreeLogger.NULL, lazyResourcesOracle);
+ lazyResourcesOracle.refresh(TreeLogger.NULL);
}
return lazyResourcesOracle;
}
@@ -402,11 +402,11 @@
+ "'");
// Refresh resource oracles.
- if (lazyResourcesOracle == null) {
- ResourceOracleImpl.refresh(logger, lazyPublicOracle, lazySourceOracle);
- } else {
- ResourceOracleImpl.refresh(
- logger, lazyPublicOracle, lazySourceOracle, lazyResourcesOracle);
+ lazyPublicOracle.refresh(logger);
+ lazySourceOracle.refresh(logger);
+
+ if (lazyResourcesOracle != null) {
+ lazyResourcesOracle.refresh(logger);
}
moduleDefEvent.end();
}
@@ -482,13 +482,13 @@
lazyPublicOracle = new ResourceOracleImpl(branch);
lazyPublicOracle.setPathPrefixes(publicPrefixSet);
}
+ lazyPublicOracle.refresh(branch);
// Create the source path.
branch = Messages.SOURCE_PATH_LOCATIONS.branch(logger, null);
lazySourceOracle = new ResourceOracleImpl(branch);
lazySourceOracle.setPathPrefixes(sourcePrefixSet);
-
- ResourceOracleImpl.refresh(logger, lazyPublicOracle, lazySourceOracle);
+ lazySourceOracle.refresh(branch);
if (lazySourceOracle.getResources().isEmpty()) {
branch.log(TreeLogger.WARN,
"No source path entries; expect subsequent failures", null);
diff --git a/dev/core/src/com/google/gwt/dev/resource/impl/ClassPathEntry.java b/dev/core/src/com/google/gwt/dev/resource/impl/ClassPathEntry.java
index 850c03f..5ada435 100644
--- a/dev/core/src/com/google/gwt/dev/resource/impl/ClassPathEntry.java
+++ b/dev/core/src/com/google/gwt/dev/resource/impl/ClassPathEntry.java
@@ -17,8 +17,6 @@
import com.google.gwt.core.ext.TreeLogger;
-import java.util.ArrayList;
-import java.util.List;
import java.util.Map;
/**
@@ -39,22 +37,6 @@
*/
public abstract Map<AbstractResource, PathPrefix> findApplicableResources(
TreeLogger logger, PathPrefixSet pathPrefixSet);
-
- /**
- * Finds applicable resources for a list of pathPrefixSets, returning a
- * distinct answer for each set.
- *
- * @see #findApplicableResources(TreeLogger, PathPrefixSet)
- */
- public List<Map<AbstractResource, PathPrefix>> findApplicableResources(
- TreeLogger logger, List<PathPrefixSet> pathPrefixSets) {
- List<Map<AbstractResource, PathPrefix>> results = new ArrayList<
- Map<AbstractResource, PathPrefix>>(pathPrefixSets.size());
- for (PathPrefixSet pathPrefixSet : pathPrefixSets) {
- results.add(findApplicableResources(logger, pathPrefixSet));
- }
- return results;
- }
/**
* Gets a URL string that describes this class path entry.
diff --git a/dev/core/src/com/google/gwt/dev/resource/impl/DirectoryClassPathEntry.java b/dev/core/src/com/google/gwt/dev/resource/impl/DirectoryClassPathEntry.java
index 315fcd9..0359a1b 100644
--- a/dev/core/src/com/google/gwt/dev/resource/impl/DirectoryClassPathEntry.java
+++ b/dev/core/src/com/google/gwt/dev/resource/impl/DirectoryClassPathEntry.java
@@ -16,13 +16,10 @@
package com.google.gwt.dev.resource.impl;
import com.google.gwt.core.ext.TreeLogger;
-import com.google.gwt.dev.util.collect.Lists;
import com.google.gwt.dev.util.msg.Message1String;
import java.io.File;
-import java.util.ArrayList;
import java.util.IdentityHashMap;
-import java.util.List;
import java.util.Map;
/**
@@ -31,9 +28,15 @@
public class DirectoryClassPathEntry extends ClassPathEntry {
private static class Messages {
+ static final Message1String NOT_DESCENDING_INTO_DIR = new Message1String(
+ TreeLogger.SPAM, "Prefix set does not include dir: $0");
+
static final Message1String DESCENDING_INTO_DIR = new Message1String(
TreeLogger.SPAM, "Descending into dir: $0");
+ static final Message1String EXCLUDING_FILE = new Message1String(
+ TreeLogger.DEBUG, "Filter excludes file: $0");
+
static final Message1String INCLUDING_FILE = new Message1String(
TreeLogger.DEBUG, "Including file: $0");
}
@@ -55,23 +58,10 @@
}
@Override
- public List<Map<AbstractResource, PathPrefix>> findApplicableResources(
- TreeLogger logger, List<PathPrefixSet> pathPrefixSets) {
- List<Map<AbstractResource, PathPrefix>> results = new ArrayList<Map<AbstractResource, PathPrefix>>(
- pathPrefixSets.size());
- for (int i = 0, c = pathPrefixSets.size(); i < c; ++i) {
- results.add(new IdentityHashMap<AbstractResource, PathPrefix>());
- }
- descendToFindResources(logger, pathPrefixSets, results, dir, "");
- return results;
- }
-
- @Override
public Map<AbstractResource, PathPrefix> findApplicableResources(
TreeLogger logger, PathPrefixSet pathPrefixSet) {
Map<AbstractResource, PathPrefix> results = new IdentityHashMap<AbstractResource, PathPrefix>();
- descendToFindResources(logger, Lists.create(pathPrefixSet),
- Lists.create(results), dir, "");
+ descendToFindResources(logger, pathPrefixSet, results, dir, "");
return results;
}
@@ -82,19 +72,16 @@
/**
* @param logger logs progress
- * @param pathPrefixSets the sets of path prefixes to determine what resources
- * are included
- * @param results the accumulating sets of resources (each with the
+ * @param resources the accumulating set of resources (each with the
* corresponding pathPrefix) found
* @param dir the file or directory to consider
* @param dirPath the abstract path name associated with 'parent', which
* explicitly does not include the classpath entry in its path
*/
private void descendToFindResources(TreeLogger logger,
- List<PathPrefixSet> pathPrefixSets,
- List<Map<AbstractResource, PathPrefix>> results, File dir, String dirPath) {
+ PathPrefixSet pathPrefixSet, Map<AbstractResource, PathPrefix> resources,
+ File dir, String dirPath) {
assert (dir.isDirectory()) : dir + " is not a directory";
- int len = pathPrefixSets.size();
// Assert: this directory is included in the path prefix set.
@@ -103,23 +90,26 @@
String childPath = dirPath + child.getName();
if (child.isDirectory()) {
String childDirPath = childPath + "/";
- for (int i = 0; i < len; ++i) {
- if (pathPrefixSets.get(i).includesDirectory(childDirPath)) {
- Messages.DESCENDING_INTO_DIR.log(logger, child.getPath(), null);
- descendToFindResources(logger, pathPrefixSets, results, child,
- childDirPath);
- break;
- }
+ if (pathPrefixSet.includesDirectory(childDirPath)) {
+ Messages.DESCENDING_INTO_DIR.log(logger, child.getAbsolutePath(),
+ null);
+ descendToFindResources(logger, pathPrefixSet, resources, child,
+ childDirPath);
+ } else {
+ Messages.NOT_DESCENDING_INTO_DIR.log(logger, child.getAbsolutePath(),
+ null);
}
} else if (child.isFile()) {
- for (int i = 0; i < len; ++i) {
- PathPrefix prefix = null;
- if ((prefix = pathPrefixSets.get(i).includesResource(childPath)) != null) {
- Messages.INCLUDING_FILE.log(logger, childPath, null);
- FileResource r = new FileResource(this, childPath, child);
- results.get(i).put(r, prefix);
- }
+ PathPrefix prefix = null;
+ if ((prefix = pathPrefixSet.includesResource(childPath)) != null) {
+ Messages.INCLUDING_FILE.log(logger, childPath, null);
+ FileResource r = new FileResource(this, childPath, child);
+ resources.put(r, prefix);
+ } else {
+ Messages.EXCLUDING_FILE.log(logger, childPath, null);
}
+ } else {
+ Messages.EXCLUDING_FILE.log(logger, childPath, null);
}
}
}
diff --git a/dev/core/src/com/google/gwt/dev/resource/impl/ResourceOracleImpl.java b/dev/core/src/com/google/gwt/dev/resource/impl/ResourceOracleImpl.java
index 7894a32..2b7e547 100644
--- a/dev/core/src/com/google/gwt/dev/resource/impl/ResourceOracleImpl.java
+++ b/dev/core/src/com/google/gwt/dev/resource/impl/ResourceOracleImpl.java
@@ -40,8 +40,8 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
-import java.util.Map.Entry;
import java.util.Set;
+import java.util.Map.Entry;
/**
* The normal implementation of {@link ResourceOracle}.
@@ -51,11 +51,24 @@
private static class Messages {
static final Message1String EXAMINING_PATH_ROOT = new Message1String(
TreeLogger.DEBUG, "Searching for resources within $0");
+
static final Message1String IGNORING_SHADOWED_RESOURCE = new Message1String(
TreeLogger.DEBUG,
"Resource '$0' is being shadowed by another resource higher in the classpath having the same name; this one will not be used");
+
+ static final Message1String NEW_RESOURCE_FOUND = new Message1String(
+ TreeLogger.DEBUG, "Found new resource: $0");
+
static final Message0 REFRESHING_RESOURCES = new Message0(TreeLogger.TRACE,
"Refreshing resources");
+
+ static final Message1String RESOURCE_BECAME_INVALID_BECAUSE_IT_IS_STALE = new Message1String(
+ TreeLogger.SPAM,
+ "Resource '$0' has been modified since it was last loaded and needs to be reloaded");
+
+ static final Message1String RESOURCE_BECAME_INVALID_BECAUSE_IT_MOVED = new Message1String(
+ TreeLogger.DEBUG,
+ "Resource '$0' was found on a different classpath entry and needs to be reloaded");
}
/**
@@ -171,98 +184,6 @@
}
}
- /**
- * Rescans the associated paths to recompute the available resources.
- *
- * @param logger status and error details are written here
- */
- public static void refresh(TreeLogger logger, ResourceOracleImpl... oracles) {
- int len = oracles.length;
- if (len == 0) {
- return;
- }
-
- Event resourceOracle =
- SpeedTracerLogger.start(CompilerEventType.RESOURCE_ORACLE, "phase", "refresh");
- TreeLogger refreshBranch = Messages.REFRESHING_RESOURCES.branch(logger,
- null);
-
- /*
- * Allocate fresh data structures in anticipation of needing to honor the
- * "new identity for the collections if anything changes" guarantee. Use a
- * LinkedHashMap because we do not want the order to change.
- */
- List<Map<String, ResourceData>> resourceDataMaps = new ArrayList<
- Map<String, ResourceData>>();
-
- List<PathPrefixSet> pathPrefixSets = new ArrayList<PathPrefixSet>();
- for (ResourceOracleImpl oracle : oracles) {
- if (oracle.classPath != oracles[0].classPath) {
- throw new IllegalArgumentException();
- }
- resourceDataMaps.add(new LinkedHashMap<String, ResourceData>());
- pathPrefixSets.add(oracle.pathPrefixSet);
- }
-
- /*
- * Walk across path roots (i.e. classpath entries) in priority order. This
- * is a "reverse painter's algorithm", relying on being careful never to add
- * a resource that has already been added to the new map under construction
- * to create the effect that resources founder earlier on the classpath take
- * precedence.
- *
- * Exceptions: super has priority over non-super; and if there are two super
- * resources with the same path, the one with the higher-priority path
- * prefix wins.
- */
- for (ClassPathEntry pathRoot : oracles[0].classPath) {
- TreeLogger branchForClassPathEntry = Messages.EXAMINING_PATH_ROOT.branch(
- refreshBranch, pathRoot.getLocation(), null);
-
- List<Map<AbstractResource, PathPrefix>> resourceToPrefixMaps = pathRoot.findApplicableResources(
- branchForClassPathEntry, pathPrefixSets);
- for (int i = 0; i < len; ++i) {
- Map<String, ResourceData> resourceDataMap = resourceDataMaps.get(i);
- Map<AbstractResource, PathPrefix> resourceToPrefixMap =
- resourceToPrefixMaps.get(i);
- for (Entry<AbstractResource, PathPrefix> entry :
- resourceToPrefixMap.entrySet()) {
- ResourceData newCpeData = new ResourceData(entry.getKey(),
- entry.getValue());
- String resourcePath = newCpeData.resource.getPath();
- ResourceData oldCpeData = resourceDataMap.get(resourcePath);
- // Old wins unless the new resource has higher priority.
- if (oldCpeData == null || oldCpeData.compareTo(newCpeData) < 0) {
- resourceDataMap.put(resourcePath, newCpeData);
- } else {
- Messages.IGNORING_SHADOWED_RESOURCE.log(branchForClassPathEntry,
- resourcePath, null);
- }
- }
- }
- }
-
- for (int i = 0; i < len; ++i) {
- Map<String, ResourceData> resourceDataMap = resourceDataMaps.get(i);
- Map<String, Resource> externalMap = new HashMap<String, Resource>();
- Set<Resource> externalSet = new HashSet<Resource>();
- for (Entry<String, ResourceData> entry : resourceDataMap.entrySet()) {
- String path = entry.getKey();
- ResourceData data = entry.getValue();
- externalMap.put(path, data.resource);
- externalSet.add(data.resource);
- }
-
- // Update exposed collections with new (unmodifiable) data structures.
- oracles[i].exposedResources = Collections.unmodifiableSet(externalSet);
- oracles[i].exposedResourceMap = Collections.unmodifiableMap(externalMap);
- oracles[i].exposedPathNames = Collections.unmodifiableSet(
- externalMap.keySet());
- }
-
- resourceOracle.end();
- }
-
private static void addAllClassPathEntries(TreeLogger logger,
ClassLoader classLoader, List<ClassPathEntry> classPath) {
// URL is expensive in collections, so we use URI instead
@@ -319,7 +240,7 @@
return classPath;
}
- private final List<ClassPathEntry> classPath;
+ private final List<ClassPathEntry> classPath = new ArrayList<ClassPathEntry>();
private Set<String> exposedPathNames = Collections.emptySet();
@@ -331,11 +252,13 @@
/**
* Constructs a {@link ResourceOracleImpl} from a set of
- * {@link ClassPathEntry ClassPathEntries}. The list is held by reference and
- * must not be modified.
+ * {@link ClassPathEntry ClassPathEntries}. The passed-in list is copied, but
+ * the underlying entries in the list are not. Those entries must be
+ * effectively immutable except for reflecting actual changes to the
+ * underlying resources.
*/
public ResourceOracleImpl(List<ClassPathEntry> classPath) {
- this.classPath = classPath;
+ this.classPath.addAll(classPath);
}
/**
@@ -378,6 +301,72 @@
return exposedResources;
}
+ /**
+ * Rescans the associated paths to recompute the available resources.
+ *
+ * @param logger status and error details are written here
+ */
+ public void refresh(TreeLogger logger) {
+ Event resourceOracle =
+ SpeedTracerLogger.start(CompilerEventType.RESOURCE_ORACLE, "phase", "refresh");
+ TreeLogger refreshBranch = Messages.REFRESHING_RESOURCES.branch(logger,
+ null);
+
+ /*
+ * Allocate fresh data structures in anticipation of needing to honor the
+ * "new identity for the collections if anything changes" guarantee. Use a
+ * LinkedHashMap because we do not want the order to change.
+ */
+ Map<String, ResourceData> newInternalMap = new LinkedHashMap<String, ResourceData>();
+
+ /*
+ * Walk across path roots (i.e. classpath entries) in priority order. This
+ * is a "reverse painter's algorithm", relying on being careful never to add
+ * a resource that has already been added to the new map under construction
+ * to create the effect that resources founder earlier on the classpath take
+ * precedence.
+ *
+ * Exceptions: super has priority over non-super; and if there are two super
+ * resources with the same path, the one with the higher-priority path
+ * prefix wins.
+ */
+ for (ClassPathEntry pathRoot : classPath) {
+ TreeLogger branchForClassPathEntry = Messages.EXAMINING_PATH_ROOT.branch(
+ refreshBranch, pathRoot.getLocation(), null);
+
+ Map<AbstractResource, PathPrefix> resourceToPrefixMap = pathRoot.findApplicableResources(
+ branchForClassPathEntry, pathPrefixSet);
+ for (Entry<AbstractResource, PathPrefix> entry : resourceToPrefixMap.entrySet()) {
+ ResourceData newCpeData = new ResourceData(entry.getKey(),
+ entry.getValue());
+ String resourcePath = newCpeData.resource.getPath();
+ ResourceData oldCpeData = newInternalMap.get(resourcePath);
+ // Old wins unless the new resource has higher priority.
+ if (oldCpeData == null || oldCpeData.compareTo(newCpeData) < 0) {
+ newInternalMap.put(resourcePath, newCpeData);
+ } else {
+ Messages.IGNORING_SHADOWED_RESOURCE.log(branchForClassPathEntry,
+ resourcePath, null);
+ }
+ }
+ }
+
+ Map<String, Resource> externalMap = new HashMap<String, Resource>();
+ Set<Resource> externalSet = new HashSet<Resource>();
+ for (Entry<String, ResourceData> entry : newInternalMap.entrySet()) {
+ String path = entry.getKey();
+ ResourceData data = entry.getValue();
+ externalMap.put(path, data.resource);
+ externalSet.add(data.resource);
+ }
+
+ // Update exposed collections with new (unmodifiable) data structures.
+ exposedResources = Collections.unmodifiableSet(externalSet);
+ exposedResourceMap = Collections.unmodifiableMap(externalMap);
+ exposedPathNames = Collections.unmodifiableSet(externalMap.keySet());
+ resourceOracle.end();
+ }
+
public void setPathPrefixes(PathPrefixSet pathPrefixSet) {
this.pathPrefixSet = pathPrefixSet;
}
diff --git a/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplRealClasspathTest.java b/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplRealClasspathTest.java
index 68bc014..d623069 100644
--- a/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplRealClasspathTest.java
+++ b/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplRealClasspathTest.java
@@ -66,7 +66,7 @@
pathPrefixSet.add(makeJunitPrefix());
pathPrefixSet.add(makeThisClassPrefix());
resourceOracle.setPathPrefixes(pathPrefixSet);
- ResourceOracleImpl.refresh(logger, resourceOracle);
+ resourceOracle.refresh(logger);
Map<String, Resource> resourceMap = resourceOracle.getResourceMap();
assertEquals(2, resourceMap.size());
}
@@ -76,17 +76,17 @@
pathPrefixSet.add(makeJunitPrefix());
pathPrefixSet.add(makeThisClassPrefix());
resourceOracle.setPathPrefixes(pathPrefixSet);
- ResourceOracleImpl.refresh(logger, resourceOracle);
+ resourceOracle.refresh(logger);
Map<String, Resource> resourceMap = resourceOracle.getResourceMap();
assertEquals(2, resourceMap.size());
// Plain refresh should have no effect.
- ResourceOracleImpl.refresh(logger, resourceOracle);
+ resourceOracle.refresh(logger);
assertResourcesEqual(resourceMap, resourceOracle.getResourceMap());
// Setting same path entries should have no effect.
resourceOracle.setPathPrefixes(pathPrefixSet);
- ResourceOracleImpl.refresh(logger, resourceOracle);
+ resourceOracle.refresh(logger);
assertResourcesEqual(resourceMap, resourceOracle.getResourceMap());
// Setting identical path entries should have no effect.
@@ -94,17 +94,17 @@
pathPrefixSet.add(makeJunitPrefix());
pathPrefixSet.add(makeThisClassPrefix());
resourceOracle.setPathPrefixes(pathPrefixSet);
- ResourceOracleImpl.refresh(logger, resourceOracle);
+ resourceOracle.refresh(logger);
assertResourcesEqual(resourceMap, resourceOracle.getResourceMap());
// Setting identical result should have no effect.
pathPrefixSet.add(makeJunitPrefix());
- ResourceOracleImpl.refresh(logger, resourceOracle);
+ resourceOracle.refresh(logger);
assertResourcesEqual(resourceMap, resourceOracle.getResourceMap());
// Actually change the working set.
pathPrefixSet.add(makeThisClassPrefixPlus());
- ResourceOracleImpl.refresh(logger, resourceOracle);
+ resourceOracle.refresh(logger);
assertEquals(3, resourceOracle.getResourceMap().size());
}
}
diff --git a/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplTest.java b/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplTest.java
index c2a449e..12b257f 100644
--- a/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplTest.java
+++ b/dev/core/test/com/google/gwt/dev/resource/impl/ResourceOracleImplTest.java
@@ -511,6 +511,10 @@
makeJavaLangPrefix());
}
+ /**
+ * @param classPathEntry
+ * @param string
+ */
private void assertJarEntry(ClassPathEntry classPathEntry, String expected) {
assertTrue("Should be instance of ZipFileClassPathEntry",
classPathEntry instanceof ZipFileClassPathEntry);
@@ -541,9 +545,9 @@
private ResourceOracleSnapshot refreshAndSnapshot(TreeLogger logger,
ResourceOracleImpl oracle) {
- ResourceOracleImpl.refresh(logger, oracle);
+ oracle.refresh(logger);
ResourceOracleSnapshot snapshot1 = new ResourceOracleSnapshot(oracle);
- ResourceOracleImpl.refresh(logger, oracle);
+ oracle.refresh(logger);
ResourceOracleSnapshot snapshot2 = new ResourceOracleSnapshot(oracle);
snapshot1.assertSameCollections(snapshot2);
return snapshot1;
