Skip to content

Commit 3297d92

Browse files
Wyveralddmaclachjustinhorvitz
authored
Switch to ProcessHandle for getting the PID (bazelbuild#14842)
* RELNOTES: Refactor system suspend event handling. We are looking to add more build anomaly reporting, so refactor the current system suspend handling to match the new model where we have a module and an event that we can record. Note this deprecates the AnomalyRecord out of BES for the time being. PiperOrigin-RevId: 409502784 * Switch to `ProcessHandle` for getting the pid. Uses new Java API instead of a native method. PiperOrigin-RevId: 409973910 * fix test Co-authored-by: dmaclach <[email protected]> Co-authored-by: jhorvitz <[email protected]>
1 parent 0c74741 commit 3297d92

35 files changed

+423
-250
lines changed

src/main/java/com/google/devtools/build/lib/BUILD

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,6 @@ java_library(
290290
"//src/main/java/com/google/devtools/build/lib/packages",
291291
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
292292
"//src/main/java/com/google/devtools/build/lib/pkgcache",
293-
"//src/main/java/com/google/devtools/build/lib/platform:suspend_counter",
294293
"//src/main/java/com/google/devtools/build/lib/profiler",
295294
"//src/main/java/com/google/devtools/build/lib/profiler:google-auto-profiler-utils",
296295
"//src/main/java/com/google/devtools/build/lib/profiler/memory:allocationtracker",

src/main/java/com/google/devtools/build/lib/analysis/BUILD

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -874,7 +874,6 @@ java_library(
874874
":blaze_version_info",
875875
"//src/main/java/com/google/devtools/build/lib/buildeventstream",
876876
"//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
877-
"//src/main/java/com/google/devtools/build/lib/util:process",
878877
"//third_party:guava",
879878
],
880879
)

src/main/java/com/google/devtools/build/lib/analysis/NoBuildEvent.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId;
2323
import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent;
2424
import com.google.devtools.build.lib.buildeventstream.ProgressEvent;
25-
import com.google.devtools.build.lib.util.ProcessUtils;
26-
import java.util.Collection;
2725

2826
/** This event raised to indicate that no build will be happening for the given command. */
2927
public final class NoBuildEvent implements BuildEvent {
@@ -55,7 +53,7 @@ public NoBuildEvent() {
5553
}
5654

5755
@Override
58-
public Collection<BuildEventId> getChildrenEvents() {
56+
public ImmutableList<BuildEventId> getChildrenEvents() {
5957
if (separateFinishedEvent) {
6058
return ImmutableList.of(
6159
ProgressEvent.INITIAL_PROGRESS_UPDATE, BuildEventIdUtil.buildFinished());
@@ -83,7 +81,7 @@ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext convert
8381
if (id != null) {
8482
started.setUuid(id);
8583
}
86-
started.setServerPid(ProcessUtils.getpid());
84+
started.setServerPid(ProcessHandle.current().pid());
8785
return GenericBuildEvent.protoChaining(this).setStarted(started.build()).build();
8886
}
8987

src/main/java/com/google/devtools/build/lib/analysis/NoBuildRequestFinishedEvent.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,6 @@
2020
/** {@link BuildEvent} indicating that a request that does not involve building as finished. */
2121
public final class NoBuildRequestFinishedEvent extends BuildCompletingEvent {
2222
public NoBuildRequestFinishedEvent(ExitCode exitCode, long finishTimeMillis) {
23-
super(exitCode, finishTimeMillis, false);
23+
super(exitCode, finishTimeMillis);
2424
}
2525
}

src/main/java/com/google/devtools/build/lib/bazel/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ java_library(
145145
"//src/main/java/com/google/devtools/build/lib/outputfilter",
146146
"//src/main/java/com/google/devtools/build/lib/packages/metrics",
147147
"//src/main/java/com/google/devtools/build/lib/platform:sleep_prevention_module",
148+
"//src/main/java/com/google/devtools/build/lib/platform:system_suspension_module",
148149
"//src/main/java/com/google/devtools/build/lib/profiler/callcounts:callcounts_module",
149150
"//src/main/java/com/google/devtools/build/lib/profiler/memory:allocationtracker_module",
150151
"//src/main/java/com/google/devtools/build/lib/remote",

src/main/java/com/google/devtools/build/lib/bazel/Bazel.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public final class Bazel {
4444
com.google.devtools.build.lib.runtime.NoSpawnCacheModule.class,
4545
com.google.devtools.build.lib.runtime.CommandLogModule.class,
4646
com.google.devtools.build.lib.platform.SleepPreventionModule.class,
47+
com.google.devtools.build.lib.platform.SystemSuspensionModule.class,
4748
com.google.devtools.build.lib.runtime.BazelFileSystemModule.class,
4849
com.google.devtools.build.lib.runtime.mobileinstall.MobileInstallModule.class,
4950
com.google.devtools.build.lib.bazel.BazelWorkspaceStatusModule.class,

src/main/java/com/google/devtools/build/lib/buildeventstream/BuildCompletingEvent.java

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,24 +30,17 @@ public abstract class BuildCompletingEvent implements BuildEvent {
3030
private final ExitCode exitCode;
3131
private final long finishTimeMillis;
3232

33-
/** Was the build suspended mid-build (e.g. hardware sleep, SIGSTOP). */
34-
private final boolean wasSuspended;
35-
3633
private final Collection<BuildEventId> children;
3734

3835
public BuildCompletingEvent(
39-
ExitCode exitCode,
40-
long finishTimeMillis,
41-
Collection<BuildEventId> children,
42-
boolean wasSuspended) {
36+
ExitCode exitCode, long finishTimeMillis, Collection<BuildEventId> children) {
4337
this.exitCode = exitCode;
4438
this.finishTimeMillis = finishTimeMillis;
4539
this.children = children;
46-
this.wasSuspended = wasSuspended;
4740
}
4841

49-
public BuildCompletingEvent(ExitCode exitCode, long finishTimeMillis, boolean wasSuspended) {
50-
this(exitCode, finishTimeMillis, ImmutableList.of(), wasSuspended);
42+
public BuildCompletingEvent(ExitCode exitCode, long finishTimeMillis) {
43+
this(exitCode, finishTimeMillis, ImmutableList.of());
5144
}
5245

5346
public ExitCode getExitCode() {
@@ -72,18 +65,12 @@ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext convert
7265
.setCode(exitCode.getNumericExitCode())
7366
.build();
7467

75-
BuildEventStreamProtos.BuildFinished.AnomalyReport protoAnamolyReport =
76-
BuildEventStreamProtos.BuildFinished.AnomalyReport.newBuilder()
77-
.setWasSuspended(wasSuspended)
78-
.build();
79-
8068
BuildEventStreamProtos.BuildFinished finished =
8169
BuildEventStreamProtos.BuildFinished.newBuilder()
8270
.setOverallSuccess(ExitCode.SUCCESS.equals(exitCode))
8371
.setExitCode(protoExitCode)
8472
.setFinishTime(Timestamps.fromMillis(finishTimeMillis))
8573
.setFinishTimeMillis(finishTimeMillis)
86-
.setAnomalyReport(protoAnamolyReport)
8774
.build();
8875
return GenericBuildEvent.protoChaining(this).setFinished(finished).build();
8976
}

src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,7 @@ message BuildFinished {
792792
// Examples of suspensions are SIGSTOP, or the hardware being put to sleep.
793793
// If was_suspended is true, then most of the timings for this build are
794794
// suspect.
795+
// NOTE: This is no longer set and is deprecated.
795796
bool was_suspended = 1;
796797
}
797798

@@ -812,7 +813,7 @@ message BuildFinished {
812813
// End of the build.
813814
google.protobuf.Timestamp finish_time = 5;
814815

815-
AnomalyReport anomaly_report = 4;
816+
AnomalyReport anomaly_report = 4 [deprecated = true];
816817
}
817818

818819
message BuildMetrics {

src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,6 @@ public final class BuildResult {
4848
private long startTimeMillis = 0; // milliseconds since UNIX epoch.
4949
private long stopTimeMillis = 0;
5050

51-
private boolean wasSuspended = false;
52-
5351
private Throwable crash = null;
5452
private boolean catastrophe = false;
5553
private boolean stopOnFirstFailure;
@@ -97,16 +95,6 @@ public double getElapsedSeconds() {
9795
return (stopTimeMillis - startTimeMillis) / 1000.0;
9896
}
9997

100-
/** Record if the build was suspended (SIGSTOP or hardware put to sleep). */
101-
public void setWasSuspended(boolean wasSuspended) {
102-
this.wasSuspended = wasSuspended;
103-
}
104-
105-
/** Whether the build was suspended (SIGSTOP or hardware put to sleep). */
106-
public boolean getWasSuspended() {
107-
return wasSuspended;
108-
}
109-
11098
public void setDetailedExitCode(DetailedExitCode detailedExitCode) {
11199
this.detailedExitCode = detailedExitCode;
112100
}

src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.buildtool;
1515

16-
import static com.google.devtools.build.lib.platform.SuspendCounter.suspendCount;
17-
1816
import com.google.common.annotations.VisibleForTesting;
1917
import com.google.common.base.Preconditions;
2018
import com.google.common.base.Throwables;
@@ -438,7 +436,6 @@ public BuildResult processRequest(
438436
BuildRequest request, TargetValidator validator, PostBuildCallback postBuildCallback) {
439437
BuildResult result = new BuildResult(request.getStartTime());
440438
maybeSetStopOnFirstFailure(request, result);
441-
int startSuspendCount = suspendCount();
442439
Throwable crash = null;
443440
DetailedExitCode detailedExitCode = null;
444441
try {
@@ -536,7 +533,7 @@ public BuildResult processRequest(
536533
new IllegalStateException("Unspecified DetailedExitCode"));
537534
}
538535
try (SilentCloseable c = Profiler.instance().profile("stopRequest")) {
539-
stopRequest(result, crash, detailedExitCode, startSuspendCount);
536+
stopRequest(result, crash, detailedExitCode);
540537
}
541538
}
542539

@@ -576,16 +573,10 @@ private static boolean needsExecutionPhase(BuildRequestOptions options) {
576573
* @param crash any unexpected {@link RuntimeException} or {@link Error}, may be null
577574
* @param detailedExitCode describes the exit code and an optional detailed failure value to add
578575
* to {@code result}
579-
* @param startSuspendCount number of suspensions before the build started
580576
*/
581577
public void stopRequest(
582-
BuildResult result,
583-
@Nullable Throwable crash,
584-
DetailedExitCode detailedExitCode,
585-
int startSuspendCount) {
578+
BuildResult result, @Nullable Throwable crash, DetailedExitCode detailedExitCode) {
586579
Preconditions.checkState((crash == null) || !detailedExitCode.isSuccess());
587-
int stopSuspendCount = suspendCount();
588-
Preconditions.checkState(startSuspendCount <= stopSuspendCount);
589580
result.setUnhandledThrowable(crash);
590581
result.setDetailedExitCode(detailedExitCode);
591582

@@ -598,7 +589,6 @@ public void stopRequest(
598589
}
599590
// The stop time has to be captured before we send the BuildCompleteEvent.
600591
result.setStopTime(runtime.getClock().currentTimeMillis());
601-
result.setWasSuspended(stopSuspendCount > startSuspendCount);
602592

603593
// Skip the build complete events so that modules can run blazeShutdownOnCrash without thinking
604594
// that the build completed normally. BlazeCommandDispatcher will call handleCrash.

0 commit comments

Comments
 (0)