From 1f511986c2065240fd072fc51d781519fe7b17bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Thu, 12 Sep 2024 08:42:10 +0200 Subject: [PATCH] [performance] BatchImageBuilder: write .class files in batches ProcessTaskManager * use java.util.concurrent for queue * signal Cancel/Exception/Stop via the queue * implements AutoCloseable for try-with-resource * drain as much Elements as possible from the queue Improves the performance of "Clean all projects" For example building platform workspace on Windows AbstractImageBuilder.compile(): 120 sec -> 91 sec With this change the Compiler is actually waiting for parsing most time and not for the write to FileSystem anymore. --- .../jdt/internal/compiler/Compiler.java | 57 ++-- .../internal/compiler/ICompilerRequestor.java | 15 ++ .../internal/compiler/ProcessTaskManager.java | 255 ++++++++---------- org.eclipse.jdt.core/META-INF/MANIFEST.MF | 2 +- .../core/builder/AbstractImageBuilder.java | 12 +- .../core/builder/BatchImageBuilder.java | 76 +++++- 6 files changed, 237 insertions(+), 180 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/Compiler.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/Compiler.java index 9b0c94eeaf8..38fb6fa4c0c 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/Compiler.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/Compiler.java @@ -553,7 +553,6 @@ protected void restoreAptProblems() { protected void processCompiledUnits(int startingIndex, boolean lastRound) throws java.lang.Error { CompilationUnitDeclaration unit = null; - ProcessTaskManager processingTask = null; try { if (this.useSingleThread) { // process all units (some more could be injected in the loop by the lookup environment) @@ -596,30 +595,38 @@ protected void processCompiledUnits(int startingIndex, boolean lastRound) throws })); } } else { - processingTask = new ProcessTaskManager(this, startingIndex); - int acceptedCount = 0; - // process all units (some more could be injected in the loop by the lookup environment) - // the processTask can continue to process units until its fixed sized cache is full then it must wait - // for this this thread to accept the units as they appear (it only waits if no units are available) - while (true) { + try (ProcessTaskManager processingTask = new ProcessTaskManager(this, startingIndex)){ + int acceptedCount = 0; + // process all units (some more could be injected in the loop by the lookup environment) + // the processTask can continue to process units until its fixed sized cache is full then it must wait + // for this this thread to accept the units as they appear (it only waits if no units are available) + this.requestor.startBatch(); try { - unit = processingTask.removeNextUnit(); // waits if no units are in the processed queue - } catch (Error | RuntimeException e) { - unit = processingTask.unitToProcess; - throw e; + do { + Collection units; + try { + units = processingTask.removeNextUnits(); + } catch (Error | RuntimeException e) { + unit = processingTask.getUnitToProcess(); + throw e; + } + if (units == null) { + break; + } + for (CompilationUnitDeclaration u : units) { + unit = u; + reportWorked(1, acceptedCount++); + this.stats.lineCount += unit.compilationResult.lineSeparatorPositions.length; + this.requestor.acceptResult(unit.compilationResult.tagAsAccepted()); + if (this.options.verbose) + this.out.println(Messages.bind(Messages.compilation_done, + new String[] { String.valueOf(acceptedCount), + String.valueOf(this.totalUnits), new String(unit.getFileName()) })); + } + } while (true); /* while (units != null) */ + } finally { + this.requestor.endBatch(); } - if (unit == null) break; - reportWorked(1, acceptedCount++); - this.stats.lineCount += unit.compilationResult.lineSeparatorPositions.length; - this.requestor.acceptResult(unit.compilationResult.tagAsAccepted()); - if (this.options.verbose) - this.out.println( - Messages.bind(Messages.compilation_done, - new String[] { - String.valueOf(acceptedCount), - String.valueOf(this.totalUnits), - new String(unit.getFileName()) - })); } } if (!lastRound) { @@ -640,10 +647,6 @@ protected void processCompiledUnits(int startingIndex, boolean lastRound) throws this.handleInternalException(e, unit, null); throw e; // rethrow } finally { - if (processingTask != null) { - processingTask.shutdown(); - processingTask = null; - } reset(); this.annotationProcessorStartIndex = 0; this.stats.endTime = System.currentTimeMillis(); diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ICompilerRequestor.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ICompilerRequestor.java index 8c1452a9b6d..69eb0101b33 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ICompilerRequestor.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ICompilerRequestor.java @@ -22,4 +22,19 @@ public interface ICompilerRequestor { * Accept a compilation result. */ public void acceptResult(CompilationResult result); + + /** + * Optionally called to start multiple {@link #acceptResult(CompilationResult)} + */ + public default void startBatch() { + //nothing + } + + /** + * if {@link #startBatch} was called then endBatch is called to finalize possibly multiple + * {@link #acceptResult(CompilationResult)} + */ + public default void endBatch() { + // nothing + } } diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ProcessTaskManager.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ProcessTaskManager.java index f14b356a275..f27f3197d73 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ProcessTaskManager.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ProcessTaskManager.java @@ -14,6 +14,11 @@ package org.eclipse.jdt.internal.compiler; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -25,19 +30,19 @@ import org.eclipse.jdt.internal.compiler.problem.AbortCompilation; import org.eclipse.jdt.internal.compiler.util.Messages; -public class ProcessTaskManager { +public class ProcessTaskManager implements AutoCloseable { private final Compiler compiler; private final int startingIndex; - private volatile Future processingTask; // synchronized write, volatile read - CompilationUnitDeclaration unitToProcess; - private Throwable caughtException; + private final Future processingTask; // synchronized write, volatile read + /** synchronized access **/ + private CompilationUnitDeclaration lastUnitToProcess; - // queue - volatile int currentIndex, availableIndex, size, sleepCount; - private final CompilationUnitDeclaration[] units; + /** contains CompilationUnitDeclaration or something else as stop signal **/ + private final BlockingQueue units; - public static final int PROCESSED_QUEUE_SIZE = 100; + private static final int PROCESSED_QUEUE_SIZE = 100; + private static final Object STOP_SIGNAL = new Object(); /** Normally a single thread is created an reused on subsequent builds **/ private static final ExecutorService executor = Executors.newCachedThreadPool(r -> { @@ -46,159 +51,131 @@ public class ProcessTaskManager { return t; }); -public ProcessTaskManager(Compiler compiler, int startingIndex) { - this.compiler = compiler; - this.startingIndex = startingIndex; + public ProcessTaskManager(Compiler compiler, int startingIndex) { + this.compiler = compiler; + this.startingIndex = startingIndex; - this.currentIndex = 0; - this.availableIndex = 0; - this.size = PROCESSED_QUEUE_SIZE; - this.sleepCount = 0; // 0 is no one, +1 is the processing thread & -1 is the writing/main thread - this.units = new CompilationUnitDeclaration[this.size]; + this.units = new ArrayBlockingQueue<>(PROCESSED_QUEUE_SIZE); - synchronized (this) { - this.processingTask = executor.submit(this::compile); + this.processingTask = executor.submit(this::processing); } -} // add unit to the queue - wait if no space is available -private synchronized void addNextUnit(CompilationUnitDeclaration newElement) { - while (this.units[this.availableIndex] != null) { - //System.out.print('a'); - //if (this.sleepCount < 0) throw new IllegalStateException(Integer.valueOf(this.sleepCount).toString()); - this.sleepCount = 1; + private void addNextUnit(Object newElement) { try { - wait(250); - } catch (InterruptedException ignore) { - // ignore + this.units.put(newElement); + } catch (InterruptedException interrupt) { + throw new RuntimeException(interrupt); } - this.sleepCount = 0; } - this.units[this.availableIndex++] = newElement; - if (this.availableIndex >= this.size) - this.availableIndex = 0; - if (this.sleepCount <= -1) - notify(); // wake up writing thread to accept next unit - could be the last one - must avoid deadlock -} + private Object lastSignal = null; -public CompilationUnitDeclaration removeNextUnit() throws Error { - CompilationUnitDeclaration next = null; - boolean yield = false; - synchronized (this) { - next = this.units[this.currentIndex]; - if (next == null || this.caughtException != null) { - do { - if (this.processingTask == null) { - if (this.caughtException != null) { - // rethrow the caught exception from the processingThread in the main compiler thread - if (this.caughtException instanceof Error) - throw (Error) this.caughtException; - throw (RuntimeException) this.caughtException; - } - return null; + /** returns null when no more elements can be expected **/ + public Collection removeNextUnits() throws Error { + List elements = new ArrayList<>(); + do { + Object next = this.lastSignal; + this.lastSignal = null; + try { + // wait until at least 1 element is available: + if (next ==null) { + next = this.units.take(); } - //System.out.print('r'); - //if (this.sleepCount > 0) throw new IllegalStateException(Integer.valueOf(this.sleepCount).toString()); - this.sleepCount = -1; - try { - wait(100); - } catch (InterruptedException ignore) { - // ignore + while (next instanceof CompilationUnitDeclaration cu) { + elements.add(cu); + // optionally read more elements if already available: + next = this.units.poll(); } - this.sleepCount = 0; - next = this.units[this.currentIndex]; - } while (next == null); - } - - this.units[this.currentIndex++] = null; - if (this.currentIndex >= this.size) - this.currentIndex = 0; - if (this.sleepCount >= 1 && ++this.sleepCount > 4) { - notify(); // wake up processing thread to add next unit but only after removing some elements first - yield = this.sleepCount > 8; - } + if (!elements.isEmpty()) { + if (next !=null) { + // defer any stop signal until all CompilationUnitDeclaration read + this.lastSignal= next; + } + return elements; + } + } catch (InterruptedException interrupt) { + throw new RuntimeException(interrupt); + } + if (next instanceof Error error) { + throw error; + } + if (next instanceof RuntimeException runtimeException) { + throw runtimeException; + } + if (next == STOP_SIGNAL) { + return null; + } + throw new IllegalStateException(String.valueOf(next)); + } while (true); } - if (yield) - Thread.yield(); - return next; -} -private void compile() { - int unitIndex = this.startingIndex; - synchronized (this) { // wait until processingTask is assigned - @SuppressWarnings("unused") - Future p = this.processingTask; - } - boolean noAnnotations = this.compiler.annotationProcessorManager == null; - while (this.processingTask != null) { - this.unitToProcess = null; - int index = -1; - boolean cleanup = noAnnotations || this.compiler.shouldCleanup(unitIndex); + private void processing() { try { - synchronized (this) { - if (this.processingTask == null) return; + int unitIndex = this.startingIndex; + boolean noAnnotations = this.compiler.annotationProcessorManager == null; + while (true) { + CompilationUnitDeclaration unitToProcess; + int index = -1; + boolean cleanup = noAnnotations || this.compiler.shouldCleanup(unitIndex); + try { + synchronized (this) { + unitToProcess = this.compiler.getUnitToProcess(unitIndex); + this.lastUnitToProcess = unitToProcess; + if (unitToProcess == null) { + break; + } + index = unitIndex++; + if (unitToProcess.compilationResult.hasBeenAccepted) { + continue; + } + } - this.unitToProcess = this.compiler.getUnitToProcess(unitIndex); - if (this.unitToProcess == null) { - this.processingTask = null; - return; - } - index = unitIndex++; - if (this.unitToProcess.compilationResult.hasBeenAccepted) - continue; - } + try { + this.compiler.reportProgress(Messages.bind(Messages.compilation_processing, + new String(unitToProcess.getFileName()))); + if (this.compiler.options.verbose) + this.compiler.out.println(Messages.bind(Messages.compilation_process, + new String[] { String.valueOf(index + 1), String.valueOf(this.compiler.totalUnits), + new String(unitToProcess.getFileName()) })); + try { + this.compiler.process(unitToProcess, index); + } catch (AbortCompilation abortCompilation) { + unitToProcess.cleanUp(); + throw abortCompilation; + } catch (Error | RuntimeException uncheckedThrowable) { + throw new RuntimeException( + "Internal Error compiling " + new String(unitToProcess.getFileName()), //$NON-NLS-1$ + uncheckedThrowable); + } + } finally { + // cleanup compilation unit result, but only if not annotation processed. + if (cleanup) { + unitToProcess.cleanUp(); + } + } - try { - this.compiler.reportProgress(Messages.bind(Messages.compilation_processing, new String(this.unitToProcess.getFileName()))); - if (this.compiler.options.verbose) - this.compiler.out.println( - Messages.bind(Messages.compilation_process, - new String[] { - String.valueOf(index + 1), - String.valueOf(this.compiler.totalUnits), - new String(this.unitToProcess.getFileName()) - })); - try { - this.compiler.process(this.unitToProcess, index); - } catch (AbortCompilation keptCancelation) { - throw keptCancelation; - } catch (Error | RuntimeException e) { - throw new RuntimeException("Internal Error compiling " + new String(this.unitToProcess.getFileName()), e); //$NON-NLS-1$ + addNextUnit(unitToProcess); + } catch (Error | RuntimeException uncheckedThrowable) { + this.units.clear(); // make sure there is room for a premature stop signal + addNextUnit(uncheckedThrowable); + return; } - } finally { - // cleanup compilation unit result, but only if not annotation processed. - if (this.unitToProcess != null && cleanup) - this.unitToProcess.cleanUp(); } - - addNextUnit(this.unitToProcess); - } catch (Error | RuntimeException e) { - synchronized (this) { - this.processingTask = null; - this.caughtException = e; - } - return; + } finally { + addNextUnit(STOP_SIGNAL); } } -} + synchronized CompilationUnitDeclaration getUnitToProcess(){ + return this.lastUnitToProcess; + } -public void shutdown() { - try { - Future t = null; - synchronized (this) { - t = this.processingTask; - if (t != null) { - // stop processing on error: - this.processingTask = null; - notifyAll(); - } - } - if (t != null) { - t.get(250, TimeUnit.MILLISECONDS); // do not wait forever + @Override + public void close() { + try { + this.processingTask.get(250, TimeUnit.MILLISECONDS); // do not wait forever + } catch (InterruptedException | ExecutionException | TimeoutException ignored) { + // ignore } - } catch (InterruptedException | ExecutionException | TimeoutException ignored) { - // ignore } } -} diff --git a/org.eclipse.jdt.core/META-INF/MANIFEST.MF b/org.eclipse.jdt.core/META-INF/MANIFEST.MF index b8016fbff37..efc948c8518 100644 --- a/org.eclipse.jdt.core/META-INF/MANIFEST.MF +++ b/org.eclipse.jdt.core/META-INF/MANIFEST.MF @@ -42,7 +42,7 @@ Export-Package: org.eclipse.jdt.core, org.eclipse.jdt.internal.formatter;x-friends:="org.eclipse.jdt.core.tests.model, org.eclipse.jdt.core.tests.compiler, org.eclipse.jdt.core.tests.builder, org.eclipse.jdt.core.tests.performance, org.eclipse.jdt.ui.tests, org.eclipse.jdt.ui.tests", org.eclipse.jdt.internal.formatter.linewrap;x-friends:="org.eclipse.jdt.core.tests.model, org.eclipse.jdt.core.tests.compiler, org.eclipse.jdt.core.tests.builder, org.eclipse.jdt.core.tests.performance, org.eclipse.jdt.ui.tests", org.eclipse.jdt.internal.formatter.old;x-friends:="org.eclipse.jdt.core.tests.model, org.eclipse.jdt.core.tests.compiler, org.eclipse.jdt.core.tests.builder, org.eclipse.jdt.core.tests.performance, org.eclipse.jdt.ui.tests" -Require-Bundle: org.eclipse.core.resources;bundle-version="[3.21.0,4.0.0)", +Require-Bundle: org.eclipse.core.resources;bundle-version="[3.22.0,4.0.0)", org.eclipse.core.runtime;bundle-version="[3.29.0,4.0.0)", org.eclipse.core.filesystem;bundle-version="[1.11.0,2.0.0)", org.eclipse.text;bundle-version="[3.6.0,4.0.0)", diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/AbstractImageBuilder.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/AbstractImageBuilder.java index 4ca89596caf..b6fcb4fadad 100644 --- a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/AbstractImageBuilder.java +++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/AbstractImageBuilder.java @@ -159,9 +159,7 @@ public void acceptResult(CompilationResult result) { ArrayList duplicateTypeNames = null; ArrayList definedTypeNames = new ArrayList(length); ArrayList postProcessingResults = new ArrayList<>(); - for (int i = 0; i < length; i++) { - ClassFile classFile = classFiles[i]; - + for (ClassFile classFile : classFiles) { char[][] compoundName = classFile.getCompoundName(); char[] typeName = compoundName[compoundName.length - 1]; boolean isNestedType = classFile.isNestedType; @@ -1004,10 +1002,6 @@ protected char[] writeClassFile(ClassFile classFile, SourceFile compilationUnit, return filePath.lastSegment().toCharArray(); } -protected void writeClassFileContents(ClassFile classFile, IFile file, String qualifiedFileName, boolean isTopLevelType, SourceFile compilationUnit) throws CoreException { - if (JavaBuilder.DEBUG) { - trace("Writing changed class file " + file.getName());//$NON-NLS-1$ - } - file.write(classFile.getBytes(), true, true, false, null); -} +abstract protected void writeClassFileContents(ClassFile classFile, IFile file, String qualifiedFileName, boolean isTopLevelType, SourceFile compilationUnit) throws CoreException; + } diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/BatchImageBuilder.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/BatchImageBuilder.java index c9fc66c8f5e..e12fa996e23 100644 --- a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/BatchImageBuilder.java +++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/BatchImageBuilder.java @@ -24,15 +24,31 @@ import org.eclipse.jdt.internal.core.util.Messages; import org.eclipse.jdt.internal.core.util.Util; +import static org.eclipse.jdt.internal.core.JavaModelManager.trace; + import java.util.*; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; @SuppressWarnings({ "rawtypes", "unchecked" }) public class BatchImageBuilder extends AbstractImageBuilder { - IncrementalImageBuilder incrementalBuilder; // if annotations or secondary types have to be processed after the compile loop - ArrayList secondaryTypes; // qualified names for all secondary types found during batch compile - Set typeLocatorsWithUndefinedTypes; // type locators for all source files with errors that may be caused by 'not found' secondary types - final CompilationGroup compilationGroup; + private IncrementalImageBuilder incrementalBuilder; // if annotations or secondary types have to be processed after the compile loop + private ArrayList secondaryTypes; // qualified names for all secondary types found during batch compile + private Set typeLocatorsWithUndefinedTypes; // type locators for all source files with errors that may be caused by 'not found' secondary types + private final CompilationGroup compilationGroup; + + /* leave 2 threads for compiler + reader.*/ + private static final ExecutorService WRITER_EXECUTOR = Executors.newFixedThreadPool(Math.max(1, Runtime.getRuntime().availableProcessors() - 2), r -> { + Thread t = new Thread(r, "Compiler Class File Writer"); //$NON-NLS-1$ + t.setDaemon(true); + return t; + }); + private static final long MAX_CLASS_CONTENTS_BYTES_QUEUED = 100_000_000; // 100MB + private final Map classContents = new HashMap<>(); + private long classContentsBytesQueued; + private boolean batchMode; + protected BatchImageBuilder(JavaBuilder javaBuilder, boolean buildStarting, CompilationGroup compilationGroup) { super(javaBuilder, buildStarting, null, compilationGroup); @@ -348,4 +364,56 @@ protected void storeProblemsFor(SourceFile sourceFile, CategorizedProblem[] prob public String toString() { return "batch image builder for:\n\tnew state: " + this.newState; //$NON-NLS-1$ } + +@Override +public void startBatch() { + this.batchMode = true; +} + +@Override +protected void writeClassFileContents(ClassFile classFile, IFile file, String qualifiedFileName, boolean isTopLevelType, + SourceFile compilationUnit) throws CoreException { + byte[] content = classFile.getBytes(); + if (this.batchMode) { + if (JavaBuilder.DEBUG) { + trace("Batching changed class file " + file.getName());//$NON-NLS-1$ + } + // flush before limit to avoid OOME: + if (!this.classContents.isEmpty() && this.classContentsBytesQueued + content.length >= MAX_CLASS_CONTENTS_BYTES_QUEUED) { + flushBatch(); + } + this.classContents.put(file, content); + this.classContentsBytesQueued += content.length; + } else { + if (JavaBuilder.DEBUG) { + trace("Writing changed class file " + file.getName());//$NON-NLS-1$ + } + file.write(content, true, true, false, null); + } +} + +@Override +public void endBatch() { + try { + flushBatch(); + } finally { + this.batchMode = false; + } +} + +private void flushBatch() { + try { + if (JavaBuilder.DEBUG) { + this.classContents.keySet().forEach(file -> trace("Writing changed class file " + file.getName()));//$NON-NLS-1$ + } + ResourcesPlugin.getWorkspace().write(this.classContents, true, true, false, null, WRITER_EXECUTOR); + } catch (CoreException e) { + // Already existing class files should not happen: + // Duplicate classes get marked earlier with a "The type {} is already defined" + Util.log(e, "JavaBuilder handling CoreException"); //$NON-NLS-1$ + } finally { + this.classContents.clear(); + this.classContentsBytesQueued = 0; + } } +} \ No newline at end of file