Introduce MetricMap as a way of exporting compiler metrics
Any method that has a TreeLogger can export a metric by calling
setAmount(). These metrics are exported by Super Dev Mode inside
each JobEvent.
Started with one metric for DeclaredTypesInModule as proof of
concept.
Also, fixed error-handling bugs in Super Dev Mode:
- JobEvent VALID_TAG regular expression was incorrect.
- If a Job failed with a RuntimeException that happened outside the
compiler, the stack trace was not being logged and the job would
never complete.
Clean up:
- JobEvent was exposing the rebased ImmutableList class
- JobRunner had an unused dependency on OutboxTable
Change-Id: Icb6f7e457c6d9b3b7de341a21b18b2ea0d505177
Review-Link: https://gwt-review.googlesource.com/#/c/9862/
diff --git a/dev/codeserver/java/com/google/gwt/dev/codeserver/CodeServer.java b/dev/codeserver/java/com/google/gwt/dev/codeserver/CodeServer.java
index aeb7ebb..945296f 100644
--- a/dev/codeserver/java/com/google/gwt/dev/codeserver/CodeServer.java
+++ b/dev/codeserver/java/com/google/gwt/dev/codeserver/CodeServer.java
@@ -118,7 +118,7 @@
OutboxTable outboxes = makeOutboxes(options, startupLogger);
JobEventTable eventTable = new JobEventTable();
- JobRunner runner = new JobRunner(eventTable, outboxes);
+ JobRunner runner = new JobRunner(eventTable);
JsonExporter exporter = new JsonExporter(options, outboxes);
diff --git a/dev/codeserver/java/com/google/gwt/dev/codeserver/Job.java b/dev/codeserver/java/com/google/gwt/dev/codeserver/Job.java
index 2b883d3..8272d35 100644
--- a/dev/codeserver/java/com/google/gwt/dev/codeserver/Job.java
+++ b/dev/codeserver/java/com/google/gwt/dev/codeserver/Job.java
@@ -20,8 +20,10 @@
import com.google.gwt.dev.cfg.ModuleDef;
import com.google.gwt.dev.codeserver.JobEvent.CompileStrategy;
import com.google.gwt.dev.codeserver.JobEvent.Status;
+import com.google.gwt.dev.util.log.AbstractTreeLogger;
import com.google.gwt.thirdparty.guava.common.base.Preconditions;
import com.google.gwt.thirdparty.guava.common.collect.ImmutableList;
+import com.google.gwt.thirdparty.guava.common.collect.ImmutableMap;
import com.google.gwt.thirdparty.guava.common.collect.ImmutableSortedMap;
import com.google.gwt.thirdparty.guava.common.util.concurrent.Futures;
import com.google.gwt.thirdparty.guava.common.util.concurrent.ListenableFuture;
@@ -291,9 +293,18 @@
out.setCompileStrategy(compileStrategy);
out.setArguments(args);
out.setTags(tags);
+ out.setMetricMap(getMetricMapSnapshot());
return out.build();
}
+ private Map<String, Long> getMetricMapSnapshot() {
+ TreeLogger logger = getLogger();
+ if (logger instanceof AbstractTreeLogger) {
+ return ((AbstractTreeLogger)logger).getMetricMap().getSnapshot();
+ }
+ return ImmutableMap.of(); // not found
+ }
+
/**
* Makes an event visible externally.
*/
@@ -331,6 +342,9 @@
synchronized TreeLogger get() {
if (child == null) {
child = parent.branch(Type.INFO, "Job " + jobId);
+ if (child instanceof AbstractTreeLogger) {
+ ((AbstractTreeLogger)child).resetMetricMap();
+ }
}
return child;
}
diff --git a/dev/codeserver/java/com/google/gwt/dev/codeserver/JobEvent.java b/dev/codeserver/java/com/google/gwt/dev/codeserver/JobEvent.java
index bae068a..56f6e01 100644
--- a/dev/codeserver/java/com/google/gwt/dev/codeserver/JobEvent.java
+++ b/dev/codeserver/java/com/google/gwt/dev/codeserver/JobEvent.java
@@ -17,6 +17,7 @@
import com.google.gwt.dev.cfg.ModuleDef;
import com.google.gwt.dev.cfg.ModuleDefSchema;
+import com.google.gwt.dev.util.log.MetricName;
import com.google.gwt.thirdparty.guava.common.base.Preconditions;
import com.google.gwt.thirdparty.guava.common.collect.ImmutableList;
import com.google.gwt.thirdparty.guava.common.collect.ImmutableMap;
@@ -33,7 +34,7 @@
* <p>JobEvent objects are deeply immutable, though they describe a Job that changes.
*/
public final class JobEvent {
- private static final Pattern VALID_TAG = Pattern.compile("\\S{1,100}");
+ private static final Pattern VALID_TAG = Pattern.compile("^\\S{1,100}$");
private final String jobId;
@@ -47,6 +48,7 @@
private final CompileStrategy compileStrategy;
private final ImmutableList<String> arguments;
private final ImmutableList<String> tags;
+ private final ImmutableSortedMap<String, Long> metricMap;
private JobEvent(Builder builder) {
this.jobId = Preconditions.checkNotNull(builder.jobId);
@@ -55,12 +57,14 @@
this.status = Preconditions.checkNotNull(builder.status);
this.message = builder.message == null ? status.defaultMessage : builder.message;
+ this.arguments = ImmutableList.copyOf(builder.args);
+ this.tags = ImmutableList.copyOf(builder.tags);
+ this.metricMap = ImmutableSortedMap.copyOf(builder.metricMap);
+
// The following fields may be null.
this.outputModuleName = builder.outputModuleName;
this.compileDir = builder.compileDir;
this.compileStrategy = builder.compileStrategy;
- this.arguments = ImmutableList.copyOf(builder.args);
- this.tags = ImmutableList.copyOf(builder.tags);
// Any new fields added should allow nulls for backward compatibility.
}
@@ -131,14 +135,23 @@
/**
* The arguments passed to Super Dev Mode at startup, or null if not available.
*/
- public ImmutableList<String> getArguments() {
+ public List<String> getArguments() {
return arguments;
}
/**
* User-defined tags associated with this job. (Not null but may be empty.)
*/
- public ImmutableList<String> getTags() { return tags; }
+ public List<String> getTags() { return tags; }
+
+ /**
+ * Returns the amounts of performance-related metrics. (Not null but may be empty.)
+ * The keys are subject to change.
+ */
+ public SortedMap<String, Long> getMetricMap() {
+ // can't return ImmutableSortedMap because it's repackaged
+ return metricMap;
+ }
/**
* If all the given tags are valid, returns a list containing the tags.
@@ -216,11 +229,14 @@
private Map<String, String> bindings = ImmutableMap.of();
private Status status;
private String message;
- private CompileDir compileDir;
- private CompileStrategy compileStrategy;
+
private List<String> args = ImmutableList.of();
private List<String> tags = ImmutableList.of();
+ private Map<String, Long> metricMap = ImmutableMap.of();
+
private String outputModuleName;
+ private CompileDir compileDir;
+ private CompileStrategy compileStrategy;
/**
* A unique id for this job. Required.
@@ -311,6 +327,21 @@
this.tags = checkTags(tags);
}
+ /**
+ * Sets a map containing metrics used for understanding compiler performance.
+ * Optional but may not be null. If not set, defaults to the empty map.
+ * Each key must be a valid identifier beginning with a capital letter.
+ * (This constraint may be relaxed later.)
+ */
+ public void setMetricMap(Map<String, Long> nameToMetric) {
+ for (String key : nameToMetric.keySet()) {
+ // TODO: allow '.' in the key after making an API for that.
+ Preconditions.checkArgument(MetricName.isValidKey(key),
+ "invalid counter key: %s", key);
+ }
+ this.metricMap = nameToMetric;
+ }
+
public JobEvent build() {
return new JobEvent(this);
}
diff --git a/dev/codeserver/java/com/google/gwt/dev/codeserver/JobRunner.java b/dev/codeserver/java/com/google/gwt/dev/codeserver/JobRunner.java
index 404d16c..d07f360 100644
--- a/dev/codeserver/java/com/google/gwt/dev/codeserver/JobRunner.java
+++ b/dev/codeserver/java/com/google/gwt/dev/codeserver/JobRunner.java
@@ -33,12 +33,10 @@
*/
public class JobRunner {
private final JobEventTable table;
- private final OutboxTable outboxes;
private final ExecutorService executor = Executors.newSingleThreadExecutor();
- JobRunner(JobEventTable table, OutboxTable outboxes) {
+ JobRunner(JobEventTable table) {
this.table = table;
- this.outboxes = outboxes;
}
/**
@@ -70,13 +68,29 @@
executor.submit(new Runnable() {
@Override
public void run() {
- recompile(job, outboxes);
+ try {
+ recompile(job);
+ } catch (Throwable t) {
+ // Try to release the job so the HTTP request will return an error.
+ // (But this might not work if the same exception is thrown while
+ // sending the finished event.)
+ if (!job.isDone()) {
+ try {
+ job.onFinished(new Job.Result(null, null, t));
+ return;
+ } catch (Throwable t2) {
+ // fall through and log original exception
+ }
+ }
+ // Assume everything is broken. Last-ditch attempt to report the error.
+ t.printStackTrace();
+ }
}
});
job.getLogger().log(Type.TRACE, "added job to queue");
}
- private static void recompile(Job job, OutboxTable outboxes) {
+ private static void recompile(Job job) {
job.getLogger().log(Type.INFO, "starting job: " + job.getId());
job.getOutbox().recompile(job);
}
diff --git a/dev/codeserver/javatests/com/google/gwt/dev/codeserver/RecompilerTest.java b/dev/codeserver/javatests/com/google/gwt/dev/codeserver/RecompilerTest.java
index 9cc7656..023551b 100644
--- a/dev/codeserver/javatests/com/google/gwt/dev/codeserver/RecompilerTest.java
+++ b/dev/codeserver/javatests/com/google/gwt/dev/codeserver/RecompilerTest.java
@@ -119,7 +119,7 @@
Outbox outbox = new Outbox("Transactional Cache", recompiler, options, logger);
OutboxTable outboxes = new OutboxTable();
outboxes.addOutbox(outbox);
- JobRunner runner = new JobRunner(new JobEventTable(), outboxes);
+ JobRunner runner = new JobRunner(new JobEventTable());
// Perform a first compile. This should pass since all resources are valid.
Result result =
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java b/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
index f68ca3e..6295000 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
@@ -83,6 +83,7 @@
import com.google.gwt.dev.util.StringInterner;
import com.google.gwt.dev.util.collect.IdentityHashSet;
import com.google.gwt.dev.util.collect.Lists;
+import com.google.gwt.dev.util.log.MetricName;
import com.google.gwt.dev.util.log.speedtracer.CompilerEventType;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event;
@@ -862,9 +863,13 @@
mainLoop();
if (incrementalCompile) {
+ int declaredTypesInModule = program.getModuleDeclaredTypes().size();
+
+ MetricName.DECLARED_TYPES_IN_MODULE.setAmount(logger, declaredTypesInModule);
+
logger.log(TreeLogger.INFO, "Unification traversed " + liveFieldsAndMethods.size()
+ " fields and methods and " + program.getDeclaredTypes().size() + " types. "
- + program.getModuleDeclaredTypes().size()
+ + declaredTypesInModule
+ " are considered part of the current module and " + fullFlowTypes.size()
+ " had all of their fields and methods traversed.");
diff --git a/dev/core/src/com/google/gwt/dev/util/log/AbstractTreeLogger.java b/dev/core/src/com/google/gwt/dev/util/log/AbstractTreeLogger.java
index 5ea39c0..edc86b8c 100644
--- a/dev/core/src/com/google/gwt/dev/util/log/AbstractTreeLogger.java
+++ b/dev/core/src/com/google/gwt/dev/util/log/AbstractTreeLogger.java
@@ -23,7 +23,7 @@
/**
* Abstract base class for TreeLoggers.
*/
-public abstract class AbstractTreeLogger extends TreeLogger {
+public abstract class AbstractTreeLogger extends TreeLogger implements CanUpdateMetrics {
private static class UncommittedBranchData {
@@ -101,6 +101,12 @@
private UncommittedBranchData uncommitted;
/**
+ * Descendant tree loggers share the same MetricMap by default,
+ * but this can be overridden with {@link #resetMetricMap()}.
+ */
+ private volatile MetricMap metricMap = new MetricMap();
+
+ /**
* The constructor used when creating a top-level logger.
*/
protected AbstractTreeLogger() {
@@ -146,6 +152,9 @@
childLogger.uncommitted = new UncommittedBranchData(type, msg, caught,
helpInfo);
+ // Inherit the parent's compiler counters by default.
+ childLogger.metricMap = metricMap;
+
// This logic is intertwined with log(). If a log message is associated
// with a special error condition, then we turn it into a branch,
// so this method can be called directly from log(). It is of course
@@ -170,10 +179,26 @@
return childLogger;
}
+ /**
+ * Clears this logger's MetricMap. If the map was shared with the
+ * parent logger, the parent's map will be unchanged and the new map will
+ * be independent of its parent.
+ */
+ public void resetMetricMap() {
+ metricMap = new MetricMap();
+ }
+
public final int getBranchedIndex() {
return indexWithinMyParent;
}
+ /**
+ * Returns the MetricMap currently associated with this TreeLogger.
+ */
+ public MetricMap getMetricMap() {
+ return metricMap;
+ }
+
public final synchronized TreeLogger.Type getMaxDetail() {
return logLevel;
}
@@ -314,6 +339,10 @@
protected abstract void doLog(int indexOfLogEntryWithinParentLogger,
TreeLogger.Type type, String msg, Throwable caught, HelpInfo helpInfo);
+ public void setAmount(MetricName name, long amount) {
+ metricMap.setAmount(name, amount);
+ }
+
/**
* Scans <code>t</code> and its causes for {@link OutOfMemoryError} or
* {@link StackOverflowError}.
diff --git a/dev/core/src/com/google/gwt/dev/util/log/CanUpdateMetrics.java b/dev/core/src/com/google/gwt/dev/util/log/CanUpdateMetrics.java
new file mode 100644
index 0000000..e1205b2
--- /dev/null
+++ b/dev/core/src/com/google/gwt/dev/util/log/CanUpdateMetrics.java
@@ -0,0 +1,27 @@
+/*
+ * Copyright 2014 Google 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 com.google.gwt.dev.util.log;
+
+/**
+ * Something that accepts updates to metrics.
+ */
+interface CanUpdateMetrics {
+
+ /**
+ * A metric's value should normally be set using {@link MetricName#setAmount}.
+ */
+ void setAmount(MetricName name, long amount);
+}
diff --git a/dev/core/src/com/google/gwt/dev/util/log/CompositeTreeLogger.java b/dev/core/src/com/google/gwt/dev/util/log/CompositeTreeLogger.java
index 8f06b23..e12b343 100644
--- a/dev/core/src/com/google/gwt/dev/util/log/CompositeTreeLogger.java
+++ b/dev/core/src/com/google/gwt/dev/util/log/CompositeTreeLogger.java
@@ -21,7 +21,7 @@
* Forks logging over two child loggers. This provides the graphics + file
* logging of DevModeBase's -logfile option.
*/
-public class CompositeTreeLogger extends TreeLogger {
+public class CompositeTreeLogger extends TreeLogger implements CanUpdateMetrics {
private TreeLogger[] loggers;
@@ -55,4 +55,11 @@
logger.log(type, msg, caught, helpInfo);
}
}
+
+ @Override
+ public void setAmount(MetricName name, long amount) {
+ for (TreeLogger logger : loggers) {
+ name.setAmount(logger, amount);
+ }
+ }
}
diff --git a/dev/core/src/com/google/gwt/dev/util/log/MetricMap.java b/dev/core/src/com/google/gwt/dev/util/log/MetricMap.java
new file mode 100644
index 0000000..a5c4118
--- /dev/null
+++ b/dev/core/src/com/google/gwt/dev/util/log/MetricMap.java
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2014 Google 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 com.google.gwt.dev.util.log;
+
+import com.google.gwt.dev.util.collect.HashMap;
+import com.google.gwt.thirdparty.guava.common.collect.ImmutableSortedMap;
+
+import java.util.Map;
+
+/**
+ * Holds mappings from metrics to their values. Thread-safe.
+ */
+public class MetricMap implements CanUpdateMetrics {
+ private final Map<String, Long> map = new HashMap<String, Long>();
+
+ /**
+ * Each MetricMap lives in one or more TreeLoggers.
+ * See {@link AbstractTreeLogger#resetMetricMap()}.
+ */
+ MetricMap() {
+ }
+
+ public synchronized void setAmount(MetricName name, long amount) {
+ map.put(name.key, amount);
+ }
+
+ public synchronized ImmutableSortedMap<String, Long> getSnapshot() {
+ return ImmutableSortedMap.copyOf(map);
+ }
+}
diff --git a/dev/core/src/com/google/gwt/dev/util/log/MetricName.java b/dev/core/src/com/google/gwt/dev/util/log/MetricName.java
new file mode 100644
index 0000000..0808afe
--- /dev/null
+++ b/dev/core/src/com/google/gwt/dev/util/log/MetricName.java
@@ -0,0 +1,76 @@
+/*
+ * Copyright 2014 Google 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 com.google.gwt.dev.util.log;
+
+import com.google.gwt.core.ext.TreeLogger;
+import com.google.gwt.core.ext.TreeLogger.Type;
+import com.google.gwt.thirdparty.guava.common.base.Preconditions;
+
+import java.util.regex.Pattern;
+
+/**
+ * Defines a name for each performance-related metric.
+ */
+public enum MetricName {
+
+ /**
+ * The number of types that UnifyAst considers to be part of the module being compiled.
+ */
+ DECLARED_TYPES_IN_MODULE("DeclaredTypesInModule");
+
+ // Note: this constraint is used in JobEvent which is a public API (for Super Dev Mode).
+ // Allowing more characters in keys may break data collection.
+ private static final Pattern VALID_KEY = Pattern.compile("^[A-Z][A-Za-z0-9]*$");
+
+ static {
+ // Cannot check this in the enum constructor.
+ for (MetricName counter : values()) {
+ Preconditions.checkState(isValidKey(counter.key), "invalid key: %s", counter.key);
+ }
+ }
+
+ final String key;
+
+ /**
+ * @param key the string key to appear in output. Must be an identifier beginning
+ * with a capital letter and not containing underscores.
+ */
+ MetricName(String key) {
+ this.key = key;
+ }
+
+ /**
+ * Adds the given amount to the counter.
+ * @param logger the destination where the count will be logged.
+ */
+ public void setAmount(TreeLogger logger, long amount) {
+ Preconditions.checkNotNull(logger);
+ Preconditions.checkArgument(amount >= 0, "attempted to set a negative amount");
+ if (logger instanceof CanUpdateMetrics) {
+ ((CanUpdateMetrics) logger).setAmount(this, amount);
+ } else {
+ // Just log it.
+ logger.log(Type.DEBUG, "Metric: " + key + " = " + amount);
+ }
+ }
+
+ /**
+ * Returns true if the given name can be used as a key for a compiler counter.
+ */
+ public static boolean isValidKey(String name) {
+ return VALID_KEY.matcher(name).matches();
+ }
+}
