From 8ca2bb51471f63a9d47daa836f99747e63a7e033 Mon Sep 17 00:00:00 2001 From: Sebastian Zarnekow Date: Sat, 21 Aug 2021 14:59:03 +0200 Subject: [PATCH] [#1710] Don't touch the project again if it was already built The strategy for the recovery build was changed such that we keep an additional flag on the builder to indicate that the next regular build is supposed to be a full build because the resource deltas cannot be trusted. Signed-off-by: Sebastian Zarnekow --- .../builder/impl/BuildManagerAccess.java | 15 ++ .../builder/impl/BuilderStateDiscarder.java | 58 +++----- .../xtext/builder/impl/IBuildFlag.java | 2 +- .../xtext/builder/impl/XtextBuilder.java | 131 +++++++++++++++++- 4 files changed, 164 insertions(+), 42 deletions(-) diff --git a/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/BuildManagerAccess.java b/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/BuildManagerAccess.java index fb99533a1e..9330fff221 100644 --- a/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/BuildManagerAccess.java +++ b/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/BuildManagerAccess.java @@ -99,5 +99,20 @@ public static void requestBuild() { throw new RuntimeException("Unexpected workspace implementation"); } } + + /** + * Schedule an auto build to be run. + * + * @since 2.26 + */ + public static void scheduleAutoBuild() { + IWorkspace workspace = ResourcesPlugin.getWorkspace(); + if (workspace instanceof Workspace) { + BuildManager buildManager = ((Workspace) workspace).getBuildManager(); + buildManager.endTopLevel(true); + } else { + throw new RuntimeException("Unexpected workspace implementation"); + } + } } \ No newline at end of file diff --git a/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/BuilderStateDiscarder.java b/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/BuilderStateDiscarder.java index 5437f96446..4cf7307176 100644 --- a/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/BuilderStateDiscarder.java +++ b/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/BuilderStateDiscarder.java @@ -10,12 +10,8 @@ import java.util.Map; -import org.apache.log4j.Logger; import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IncrementalProjectBuilder; -import org.eclipse.core.runtime.CoreException; -import org.eclipse.core.runtime.Status; -import org.eclipse.core.runtime.jobs.Job; import com.google.common.annotations.Beta; @@ -33,50 +29,42 @@ @Beta public class BuilderStateDiscarder { - private static final Logger logger = Logger.getLogger(BuilderStateDiscarder.class); - /** * Returns true, if the given builderArguments indicate that we should discard the built state * for the given projects. */ public boolean forgetLastBuildState(Iterable toUpdate, Map builderArguments) { + /* + * Implementation note: + * This is an unsafe operation. There no guarantee that any lock is being acquired by the current thread + * while performing this operation. It is very well possible that this is running concurrently with a build. + * + * When we come here, we might, or might not + * - hold the AbstractBuilderState.loadLock + * - hold the org.eclipse.core.internal.resources.WorkManager.lock + * - come from a build and see a situation with org.eclipse.core.internal.resources.Workspace.treeLocked being the case + * - might or might not have a rule stack available on org.eclipse.core.internal.jobs.ThreadJob.ruleStack + * + * It is desired that we only discard the build information of the XtextBuilder and not of all the builders + * of a project. Therefore, we try to keep the interaction with the resource model to a minimum. + * + * The strategy is coupled to the internals of the XtextBuilder where state is maintained + * that allows to schedule an initial full build for a project. + */ if (canHandleBuildFlag(builderArguments)) { for (IProject project : toUpdate) { - XtextBuilder builder = BuildManagerAccess.findBuilder(project); - if (builder != null) { - builder.forgetLastBuiltState(); - touchProject(project); + if (project.isAccessible()) { + XtextBuilder builder = BuildManagerAccess.findBuilder(project); + if (builder != null) { + builder.requestFullBuild(IBuildFlag.FORGET_BUILD_STATE_ONLY.isSet(builderArguments)); + } } } return true; } return false; } - - /** - * Touch a given project such that the auto-build knows it should be rebuild. - * - * @since 2.26 - */ - protected void touchProject(IProject project) { - Job touchJob = Job.create("Touch project " + project.getName(), progressMonitor -> { - try { - if (project.isAccessible()) { - project.touch(progressMonitor); - } - return Status.OK_STATUS; - } catch (CoreException e) { - logger.error("Failed to refresh project while forgetting its builder state", e); - return Status.CANCEL_STATUS; - } - }); - touchJob.setRule(project); - touchJob.setUser(false); - touchJob.setSystem(true); - touchJob.schedule(0); - - } - + protected boolean canHandleBuildFlag(Map builderArguments) { return IBuildFlag.FORGET_BUILD_STATE_ONLY.isSet(builderArguments) || IBuildFlag.RECOVERY_BUILD.isSet(builderArguments); diff --git a/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/IBuildFlag.java b/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/IBuildFlag.java index 5788edc30a..0c11290e15 100644 --- a/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/IBuildFlag.java +++ b/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/IBuildFlag.java @@ -28,7 +28,7 @@ public interface IBuildFlag { * requested. */ static final IBuildFlag FORGET_BUILD_STATE_ONLY = new Impl("ForgetBuildState"); - + void addToMap(Map buildArgs); boolean isSet(Map buildArgs); diff --git a/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/XtextBuilder.java b/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/XtextBuilder.java index 805b31b70b..289741e273 100644 --- a/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/XtextBuilder.java +++ b/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/XtextBuilder.java @@ -8,6 +8,7 @@ *******************************************************************************/ package org.eclipse.xtext.builder.impl; +import java.lang.reflect.Field; import java.util.Arrays; import java.util.Collections; import java.util.Map; @@ -158,6 +159,7 @@ public static enum SchedulingOption { * all projects with Xtext nature in the workspace, extended by JDTs External Folder Project * @see org.eclipse.jdt.internal.core.ExternalFoldersManager * */ + @SuppressWarnings("restriction") ALL_XTEXT_PROJECTS_AND_JDTEXTFOLDER, /** the currently building project */ PROJECT, @@ -165,13 +167,127 @@ public static enum SchedulingOption { NULL; } + /** + * Used to reflectively set the flag that will instruct the Eclipse infrastructure to also + * consider this builder if the delta is empty. An XtextBuilder might need to run a build even + * if no resource changes have been recorded, e.g. if the index wasn't deserialized successfully + * but the workspace state is still consistent from Eclipse's POV. + * + * @since 2.26 + */ + private static final Field callOnEmptyData; + static { + try { + @SuppressWarnings("restriction") + Field fld = org.eclipse.core.internal.events.InternalBuilder.class.getDeclaredField("callOnEmptyDelta"); + fld.setAccessible(true); + callOnEmptyData = fld; + } catch (ReflectiveOperationException e) { + throw new ExceptionInInitializerError(e); + } + } + + /** + * @since 2.26 + */ + private boolean nextBuildIsFullBuild = false; + /** + * @since 2.26 + */ + private boolean allowRequestFullBuild = true; + + /** + * The lock protects access two {@link #nextBuildIsFullBuild} and {@link #allowRequestFullBuild}. + * @since 2.26 + */ + private final Object requestFullBuildLock = new Object(); + + /** + * An external (to this builder and the Eclipse BuildManager) service may want to trigger an initial + * full build for this project if it was detected that the project state is not in sync with the builder + * state. This can be done once in the life-cycle of an XtextBuilder which is bound to an IProject. + * + * @since 2.26 + */ + protected void requestFullBuild(boolean force) { + /* + * Synchronize here to avoid a build being triggered while a build was running and completed successfully. + * Maintaining the flags must be coherent. + */ + synchronized (requestFullBuildLock) { + if (allowRequestFullBuild || force) { + allowRequestFullBuild = false; + nextBuildIsFullBuild = true; + try { + callOnEmptyData.set(this, true); + } catch (IllegalArgumentException | IllegalAccessException e) { + log.error(e.getMessage(), e); + } + BuildManagerAccess.scheduleAutoBuild(); + } + } + } + + /** + * Answer if a full build was already requested externally and not yet done. + * + * @since 2.26 + */ + protected boolean wasFullBuildRequested() { + synchronized (requestFullBuildLock) { + return nextBuildIsFullBuild; + } + } + + /** + * After a full build was run, reset the externally requested full build. + * Don't reset, if the current build was an incremental build but a full build was requested + * in the meantime. + * + * @since 2.26 + */ + protected void unsetWasFullBuildRequested(boolean wasFullBuildRequested) { + synchronized (requestFullBuildLock) { + if (nextBuildIsFullBuild && !wasFullBuildRequested) { + // an intermediate operation requested an enforced full build, e.g. due to a class-path change + // make the next build a full build + return; + } + allowRequestFullBuild = false; + nextBuildIsFullBuild = false; + try { + callOnEmptyData.set(this, false); + } catch (IllegalArgumentException | IllegalAccessException e) { + log.error(e.getMessage(), e); + } + } + } + + /** + * Attempt to access the builder state. If it wasn't loaded yet, an attempt to load it is made. + * This may trigger the {@link BuilderStateDiscarder#forgetLastBuildState(Iterable, Map)} if there + * is no build state available. + * Since we are currently building this project, we turn the current build into a full build if it + * was an auto build or an incremental build instead. + * + * @since 2.26 + */ + protected void ensureBuilderStateLoaded() { + // ensure the state was loaded or load it now or wait until it was successfully loaded + builderState.isEmpty(); + } + @SuppressWarnings("rawtypes") @Override protected IProject[] build(final int kind, Map args, IProgressMonitor monitor) throws CoreException { - if (IBuildFlag.FORGET_BUILD_STATE_ONLY.isSet(args)) { + ensureBuilderStateLoaded(); + + boolean wasFullBuildRequested = wasFullBuildRequested(); + if (!wasFullBuildRequested && IBuildFlag.FORGET_BUILD_STATE_ONLY.isSet(args)) { forgetLastBuiltState(); return getReferencedProjects(); } + long startTime = System.currentTimeMillis(); StoppedTask task = Stopwatches.forTask(String.format("XtextBuilder.build[%s]", getKindAsString(kind))); try { @@ -198,16 +314,18 @@ public boolean isCanceled() { }; } SubMonitor progress = SubMonitor.convert(monitor, 1); - if (kind == FULL_BUILD) { - fullBuild(progress.split(1), IBuildFlag.RECOVERY_BUILD.isSet(args)); + if (kind == FULL_BUILD || wasFullBuildRequested) { + fullBuild(progress.split(1), !wasFullBuildRequested && IBuildFlag.RECOVERY_BUILD.isSet(args)); } else { IResourceDelta delta = getDelta(getProject()); if (delta == null || isOpened(delta)) { - fullBuild(progress.split(1), IBuildFlag.RECOVERY_BUILD.isSet(args)); + fullBuild(progress.split(1), !wasFullBuildRequested && IBuildFlag.RECOVERY_BUILD.isSet(args)); } else { incrementalBuild(delta, progress.split(1)); } } + // If no exceptions have been thrown, fix the information about the requested full builds + unsetWasFullBuildRequested(wasFullBuildRequested); } catch (CoreException e) { log.error(e.getMessage(), e); throw e; @@ -251,7 +369,7 @@ private boolean shouldCancelBuild(int buildKind) { } private void handleCanceled(Throwable t) { - // If the cancelation happens due to an external interruption, don't pass an + // If the cancellation happens due to an external interruption, don't pass an // OperationCanceledException on to the BuildManager as it would force a full // build in the next round. Instead, save the resource deltas to be reprocessed // next time. @@ -376,7 +494,7 @@ protected void doBuild(ToBeBuilt toBeBuilt, IProgressMonitor monitor, BuildType protected void doBuild(ToBeBuilt toBeBuilt, Set removedProjects, IProgressMonitor monitor, BuildType type) throws CoreException { buildLogger.log("Building " + getProject().getName()); // return early if there's nothing to do. - // we reuse the isEmpty() impl from BuildData assuming that it doesnT access the ResourceSet which is still null + // we reuse the isEmpty() implementation from BuildData assuming that it doesnT access the ResourceSet which is still null // and would be expensive to create. boolean indexingOnly = type == BuildType.RECOVERY; if (new BuildData(getProject().getName(), null, toBeBuilt, queuedBuildData, indexingOnly, this::needRebuild, removedProjects).isEmpty()) @@ -488,6 +606,7 @@ protected void clean(IProgressMonitor monitor) throws CoreException { task.reschedule(); throw e; } + unsetWasFullBuildRequested(true); } finally { if (monitor != null) monitor.done();