Super Dev Mode: move RecompileListener event handling to Job No externally visible changes. Rationale: simplify event handling by putting Job in charge of reporting events to listeners. Other refactoring: - simplify initWithoutRecompile() so it doesn't need a Job. (It's not really a compile job anyway.) - simplify progress reporting in Recompiler by making Job keep track of progress state. Change-Id: Iec6639d31948a649c79941bacc2429efc6a7d548
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 6da73ba..c23fecd 100644 --- a/dev/codeserver/java/com/google/gwt/dev/codeserver/CodeServer.java +++ b/dev/codeserver/java/com/google/gwt/dev/codeserver/CodeServer.java
@@ -75,7 +75,7 @@ try { // TODO: actually test recompiling here. // (This is just running precompiles repeatedly.) - outboxes.defaultCompileAll(options.getNoPrecompile(), logger); + outboxes.defaultCompileAll(logger); } catch (Throwable t) { t.printStackTrace(); System.out.println("FAIL"); @@ -152,7 +152,7 @@ String outboxId = moduleName + "_" + nextOutboxId; nextOutboxId++; - outboxes.addOutbox(new Outbox(outboxId, recompiler, options.getNoPrecompile(), logger)); + outboxes.addOutbox(new Outbox(outboxId, recompiler, options, logger)); } return 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 cc913b9..586d3ca 100644 --- a/dev/codeserver/java/com/google/gwt/dev/codeserver/Job.java +++ b/dev/codeserver/java/com/google/gwt/dev/codeserver/Job.java
@@ -40,6 +40,8 @@ private static final ConcurrentMap<String, AtomicInteger> prefixToNextId = new ConcurrentHashMap<String, AtomicInteger>(); + // Primary key + private final String id; // Input @@ -55,24 +57,45 @@ // Listeners private final Outbox outbox; - + private final RecompileListener recompileListener; private final LogSupplier logSupplier; private ProgressTable table; // non-null when submitted + // Progress + + /** + * The number of calls to {@link #onCompilerProgress}. + */ + private int finishedSteps = 0; + + /** + * The estimated total number of calls to {@link #onCompilerProgress}. + */ + private int totalSteps = -1; // non-negative after the compile has started + + /** + * The id to report to the recompile listener. + */ + private int compileId = -1; // non-negative after the compile has started + + private Exception recompileListenerFailure; + /** * Creates a job to update an outbox. * @param bindingProperties Properties that uniquely identify a permutation. * (Otherwise, more than one permutation will be compiled.) * @param parentLogger The parent of the logger that will be used for this job. */ - Job(Outbox box, Map<String, String> bindingProperties, TreeLogger parentLogger) { + Job(Outbox box, Map<String, String> bindingProperties, + TreeLogger parentLogger, RecompileListener recompileListener) { this.id = chooseNextId(box); this.outbox = box; this.inputModuleName = box.getInputModuleName(); // TODO: we will use the binding properties to find or create the outbox, // then take binding properties from the outbox here. this.bindingProperties = ImmutableSortedMap.copyOf(bindingProperties); + this.recompileListener = recompileListener; this.logSupplier = new LogSupplier(parentLogger, id); } @@ -134,6 +157,10 @@ return result; } + Exception getRecompileListenerFailure() { + return recompileListenerFailure; + } + // === state transitions === /** @@ -162,14 +189,40 @@ } /** - * Reports that this job has made progress. - * @throws IllegalStateException if the job is not running. + * Reports that we started to compile the job. */ - synchronized void onCompilerProgress(Progress.Compiling newProgress) { + synchronized void onStarted(int totalSteps, int compileId, CompileDir compileDir) { + if (totalSteps < 0) { + throw new IllegalArgumentException("totalSteps should not be negative: " + totalSteps); + } if (table == null || !table.isActive(this)) { throw new IllegalStateException("compile job is not active: " + id); } - table.publish(newProgress, getLogger()); + if (this.totalSteps >= 0) { + throw new IllegalStateException("onStarted already called for " + id); + } + this.totalSteps = totalSteps; + this.compileId = compileId; + + try { + recompileListener.startedCompile(inputModuleName, compileId, compileDir); + } catch (Exception e) { + getLogger().log(TreeLogger.Type.WARN, "recompile listener threw exception", e); + recompileListenerFailure = e; + } + } + + /** + * Reports that this job has made progress. + * @throws IllegalStateException if the job is not running. + */ + synchronized void onCompilerProgress(String stepMessage) { + if (table == null || !table.isActive(this)) { + throw new IllegalStateException("compile job is not active: " + id); + } + finishedSteps++; + table.publish(new Progress.Compiling(this, finishedSteps, totalSteps, stepMessage), + getLogger()); } /** @@ -180,6 +233,17 @@ if (table == null || !table.isActive(this)) { throw new IllegalStateException("compile job is not active: " + id); } + + // Report that we finished unless the listener messed up already. + if (recompileListenerFailure == null) { + try { + recompileListener.finishedCompile(inputModuleName, compileId, newResult.isOk()); + } catch (Exception e) { + getLogger().log(TreeLogger.Type.WARN, "recompile listener threw exception", e); + recompileListenerFailure = e; + } + } + result.set(newResult); if (newResult.isOk()) { table.publish(new Progress(this, Status.SERVING), getLogger()); @@ -224,8 +288,6 @@ */ static class Result { - final Job job; - /** * non-null if successful */ @@ -236,10 +298,8 @@ */ final Throwable error; - Result(Job job, CompileDir outputDir, Throwable error) { - assert job != null; + Result(CompileDir outputDir, Throwable error) { assert (outputDir == null) != (error == null); - this.job = job; this.outputDir = outputDir; this.error = error; }
diff --git a/dev/codeserver/java/com/google/gwt/dev/codeserver/Outbox.java b/dev/codeserver/java/com/google/gwt/dev/codeserver/Outbox.java index 305f1a5..f2275f2 100644 --- a/dev/codeserver/java/com/google/gwt/dev/codeserver/Outbox.java +++ b/dev/codeserver/java/com/google/gwt/dev/codeserver/Outbox.java
@@ -44,14 +44,17 @@ private final String id; private final Recompiler recompiler; + private final Options options; - private final AtomicReference<Job.Result> published = new AtomicReference<Job.Result>(); + private final AtomicReference<Result> published = new AtomicReference<Result>(); + private Job publishedJob; // may be null if the Result wasn't created by a Job. - Outbox(String id, Recompiler recompiler, boolean noPrecompile, TreeLogger logger) + Outbox(String id, Recompiler recompiler, Options options, TreeLogger logger) throws UnableToCompleteException { this.id = id; this.recompiler = recompiler; - maybePrecompile(noPrecompile, logger); + this.options = options; + maybePrecompile(logger); } /** @@ -65,8 +68,14 @@ * Loads the module and maybe compiles it. Sets up the output directory. * Throws an exception if unable. (In this case, Super Dev Mode fails to start.) */ - void maybePrecompile(boolean noPrecompile, TreeLogger logger) throws UnableToCompleteException { - // TODO: each box will have its own binding properties. + void maybePrecompile(TreeLogger logger) throws UnableToCompleteException { + + if (options.getNoPrecompile()) { + publish(recompiler.initWithoutPrecompile(logger), null); + return; + } + + // TODO: each box will have its own binding properties Map<String, String> defaultProps = new HashMap<String, String>(); defaultProps.put("user.agent", "safari"); defaultProps.put("locale", "en"); @@ -74,17 +83,29 @@ // Create a dummy job for the first compile. // Its progress is not visible externally but will still be logged. ProgressTable dummy = new ProgressTable(); - Job job = new Job(this, defaultProps, logger); + Job job = makeJob(defaultProps, logger); job.onSubmitted(dummy); + publish(recompiler.precompile(job), job); - if (noPrecompile) { - publish(recompiler.initWithoutPrecompile(job)); - } else { - publish(recompiler.precompile(job)); + if (options.isCompileTest()) { + // Listener errors are fatal in compile tests + Throwable error = job.getRecompileListenerFailure(); + if (error != null) { + UnableToCompleteException e = new UnableToCompleteException(); + e.initCause(error); + throw e; + } } } /** + * Creates a Job whose output will be saved in this outbox. + */ + Job makeJob(Map<String, String> bindingProperties, TreeLogger parentLogger) { + return new Job(this, bindingProperties, parentLogger, options.getRecompileListener()); + } + + /** * Compiles the module again, possibly changing the output directory. * After returning, the result of the compile can be found via {@link Job#waitForResult} */ @@ -97,7 +118,7 @@ Result result = recompiler.recompile(job); if (result.isOk()) { - publish(result); + publish(result, job); } else { job.getLogger().log(TreeLogger.Type.WARN, "continuing to serve previous version"); } @@ -105,12 +126,14 @@ /** * Makes the result of a compile downloadable via HTTP. + * @param job the job that created this result, or null if none. */ - private void publish(Result result) { - Result previous = published.getAndSet(result); - if (previous != null) { - previous.job.onGone(); + private synchronized void publish(Result result, Job job) { + if (publishedJob != null) { + publishedJob.onGone(); } + publishedJob = job; + published.set(result); } private CompileDir getOutputDir() {
diff --git a/dev/codeserver/java/com/google/gwt/dev/codeserver/OutboxTable.java b/dev/codeserver/java/com/google/gwt/dev/codeserver/OutboxTable.java index 400a3eb..54ce117 100644 --- a/dev/codeserver/java/com/google/gwt/dev/codeserver/OutboxTable.java +++ b/dev/codeserver/java/com/google/gwt/dev/codeserver/OutboxTable.java
@@ -69,9 +69,9 @@ return result; } - void defaultCompileAll(boolean noPrecompile, TreeLogger logger) throws UnableToCompleteException { + void defaultCompileAll(TreeLogger logger) throws UnableToCompleteException { for (Outbox box: outboxes.values()) { - box.maybePrecompile(noPrecompile, logger); + box.maybePrecompile(logger); } } }
diff --git a/dev/codeserver/java/com/google/gwt/dev/codeserver/Recompiler.java b/dev/codeserver/java/com/google/gwt/dev/codeserver/Recompiler.java index 247b4ee..9a27ed6 100644 --- a/dev/codeserver/java/com/google/gwt/dev/codeserver/Recompiler.java +++ b/dev/codeserver/java/com/google/gwt/dev/codeserver/Recompiler.java
@@ -116,10 +116,10 @@ } catch (UnableToCompleteException e) { // No point in logging a stack trace for this exception job.getLogger().log(TreeLogger.Type.WARN, "recompile failed"); - result = new Result(job, null, e); + result = new Result(null, e); } catch (Throwable error) { job.getLogger().log(TreeLogger.Type.WARN, "recompile failed", error); - result = new Result(job, null, error); + result = new Result(null, error); } job.onFinished(result); @@ -134,8 +134,7 @@ * @return a non-error Job.Result if successful. * @throws UnableToCompleteException for compile failures. */ - private Job.Result compile(Job job) - throws UnableToCompleteException { + private Job.Result compile(Job job) throws UnableToCompleteException { assert job.wasSubmitted(); @@ -154,31 +153,15 @@ CompileDir compileDir = makeCompileDir(compileId, job.getLogger()); TreeLogger compileLogger = makeCompileLogger(compileDir, job.getLogger()); - boolean listenerFailed = false; - try { - options.getRecompileListener().startedCompile(inputModuleName, compileId, compileDir); - } catch (Exception e) { - compileLogger.log(TreeLogger.Type.WARN, "listener threw exception", e); - listenerFailed = true; - } + int totalSteps = options.shouldCompileIncremental() ? 1 : 2; + job.onStarted(totalSteps, compileId, compileDir); - boolean success = false; - try { - if (options.shouldCompileIncremental()) { - // Just have one message for now. - job.onCompilerProgress(new Progress.Compiling(job, 0, 1, "Compiling")); - - success = compileIncremental(compileLogger, compileDir); - } else { - success = compileMonolithic(compileLogger, compileDir, job); - } - } finally { - try { - options.getRecompileListener().finishedCompile(inputModuleName, compilesDone, success); - } catch (Exception e) { - compileLogger.log(TreeLogger.Type.WARN, "listener threw exception", e); - listenerFailed = true; - } + boolean success; + if (options.shouldCompileIncremental()) { + job.onCompilerProgress("Compiling (incrementally)"); + success = compileIncremental(compileLogger, compileDir); + } else { + success = compileMonolithic(compileLogger, compileDir, job); } if (!success) { @@ -190,22 +173,19 @@ compileLogger.log(TreeLogger.Type.INFO, String.format("%.3fs total -- Compile completed", elapsedTime / 1000d)); - if (options.isCompileTest() && listenerFailed) { - throw new UnableToCompleteException(); - } - - return new Result(job, publishedCompileDir, null); + return new Result(publishedCompileDir, null); } /** * Creates a dummy output directory without compiling the module. * Either this method or {@link #precompile} should be called first. */ - synchronized Job.Result initWithoutPrecompile(Job job) throws UnableToCompleteException { + synchronized Job.Result initWithoutPrecompile(TreeLogger logger) + throws UnableToCompleteException { long startTime = System.currentTimeMillis(); - CompileDir compileDir = makeCompileDir(++compilesDone, job.getLogger()); - TreeLogger compileLogger = makeCompileLogger(compileDir, job.getLogger()); + CompileDir compileDir = makeCompileDir(++compilesDone, logger); + TreeLogger compileLogger = makeCompileLogger(compileDir, logger); ModuleDef module = loadModule(compileLogger); String newModuleName = module.getName(); // includes any rename. @@ -235,9 +215,7 @@ long elapsedTime = System.currentTimeMillis() - startTime; compileLogger.log(TreeLogger.Type.INFO, "Module setup completed in " + elapsedTime + " ms"); - Result result = new Result(job, compileDir, null); - job.onFinished(result); - return result; + return new Result(compileDir, null); } private boolean compileIncremental(TreeLogger compileLogger, CompileDir compileDir) { @@ -276,8 +254,7 @@ private boolean compileMonolithic(TreeLogger compileLogger, CompileDir compileDir, Job job) throws UnableToCompleteException { - job.onCompilerProgress( - new Progress.Compiling(job, 0, 2, "Loading modules")); + job.onCompilerProgress("Loading modules"); CompilerOptions loadOptions = new CompilerOptionsImpl(compileDir, inputModuleName, options); compilerContext = compilerContextBuilder.options(loadOptions).build(); @@ -297,7 +274,7 @@ return true; } - job.onCompilerProgress(new Progress.Compiling(job, 1, 2, "Compiling")); + job.onCompilerProgress("Compiling"); // TODO: use speed tracer to get more compiler events? CompilerOptions runOptions = new CompilerOptionsImpl(compileDir, newModuleName, options);
diff --git a/dev/codeserver/java/com/google/gwt/dev/codeserver/WebServer.java b/dev/codeserver/java/com/google/gwt/dev/codeserver/WebServer.java index 95dcdaf..bcede76 100644 --- a/dev/codeserver/java/com/google/gwt/dev/codeserver/WebServer.java +++ b/dev/codeserver/java/com/google/gwt/dev/codeserver/WebServer.java
@@ -213,7 +213,7 @@ // cause a spurious recompile, resulting in an unexpected permutation being loaded later. // // It would be unsafe to allow a configuration property to be changed. - Job job = new Job(outbox, getBindingProperties(request), logger); + Job job = outbox.makeJob(getBindingProperties(request), logger); runner.submit(job); Job.Result result = job.waitForResult(); JsonObject json = jsonExporter.exportRecompileResponse(result);