Skip to content

Commit 7056711

Browse files
larsrc-googlephilwo
authored andcommitted
Make WorkRequestHandler do a GC after some amount of CPU time has been used on requests. For Bazel and Blaze, defaults to 10s based on benchmarking.
Linux benchmarks show no significant regressions in wall or CPU time, and possibly a reduction in system time. RELNOTES: None. PiperOrigin-RevId: 362006101
1 parent c4e436a commit 7056711

File tree

2 files changed

+99
-10
lines changed

2 files changed

+99
-10
lines changed

src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.io.PrintWriter;
3131
import java.io.Writer;
3232
import java.nio.charset.Charset;
33+
import java.time.Duration;
3334
import java.util.Arrays;
3435
import java.util.List;
3536

@@ -46,7 +47,8 @@ public static void main(String[] args) {
4647
new WorkRequestHandler(
4748
builder::parseAndBuild,
4849
System.err,
49-
new ProtoWorkerMessageProcessor(System.in, System.out));
50+
new ProtoWorkerMessageProcessor(System.in, System.out),
51+
Duration.ofSeconds(10));
5052
try {
5153
workerHandler.processRequests();
5254
} catch (IOException e) {

src/main/java/com/google/devtools/build/lib/worker/WorkRequestHandler.java

+96-9
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,19 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.worker;
1515

16+
1617
import com.google.common.annotations.VisibleForTesting;
1718
import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest;
1819
import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse;
20+
import com.sun.management.OperatingSystemMXBean;
1921
import java.io.IOException;
2022
import java.io.PrintStream;
2123
import java.io.PrintWriter;
2224
import java.io.StringWriter;
25+
import java.lang.management.ManagementFactory;
26+
import java.time.Duration;
2327
import java.util.List;
28+
import java.util.concurrent.atomic.AtomicReference;
2429
import java.util.function.BiFunction;
2530

2631
/**
@@ -53,19 +58,50 @@ public interface WorkerMessageProcessor {
5358

5459
private final WorkerMessageProcessor messageProcessor;
5560

61+
private final CpuTimeBasedGcScheduler gcScheduler;
62+
5663
/**
5764
* Creates a {@code WorkRequestHandler} that will call {@code callback} for each WorkRequest
58-
* received. The first argument to {@code callback} is the set of command-line arguments, the
59-
* second is where all error messages and similar should be written to. The callback should return
60-
* an exit code indicating success (0) or failure (nonzero).
65+
* received.
66+
*
67+
* @param callback Callback method for executing a single WorkRequest in a thread. The first
68+
* argument to {@code callback} is the set of command-line arguments, the second is where all
69+
* error messages and other user-oriented messages should be written to. The callback must
70+
* return an exit code indicating success (zero) or failure (nonzero).
71+
* @param stderr Stream that log messages should be written to, typically the process' stderr.
72+
* @param messageProcessor Object responsible for parsing {@code WorkRequest}s from the server and
73+
* writing {@code WorkResponses} to the server.
6174
*/
6275
public WorkRequestHandler(
6376
BiFunction<List<String>, PrintWriter, Integer> callback,
6477
PrintStream stderr,
6578
WorkerMessageProcessor messageProcessor) {
79+
this(callback, stderr, messageProcessor, Duration.ZERO);
80+
}
81+
82+
/**
83+
* Creates a {@code WorkRequestHandler} that will call {@code callback} for each WorkRequest
84+
* received.
85+
*
86+
* @param callback Callback method for executing a single WorkRequest in a thread. The first
87+
* argument to {@code callback} is the set of command-line arguments, the second is where all
88+
* error messages and other user-oriented messages should be written to. The callback must
89+
* return an exit code indicating success (zero) or failure (nonzero).
90+
* @param stderr Stream that log messages should be written to, typically the process' stderr.
91+
* @param messageProcessor Object responsible for parsing {@code WorkRequest}s from the server and
92+
* writing {@code WorkResponses} to the server.
93+
* @param cpuUsageBeforeGc The minimum amount of CPU time between explicit garbage collection
94+
* calls.
95+
*/
96+
public WorkRequestHandler(
97+
BiFunction<List<String>, PrintWriter, Integer> callback,
98+
PrintStream stderr,
99+
WorkerMessageProcessor messageProcessor,
100+
Duration cpuUsageBeforeGc) {
66101
this.callback = callback;
67102
this.stderr = stderr;
68103
this.messageProcessor = messageProcessor;
104+
this.gcScheduler = new CpuTimeBasedGcScheduler(cpuUsageBeforeGc);
69105
}
70106

71107
/**
@@ -77,13 +113,13 @@ public WorkRequestHandler(
77113
public void processRequests() throws IOException {
78114
while (true) {
79115
WorkRequest request = messageProcessor.readWorkRequest();
80-
if (request == null) {
81-
break;
82-
}
83-
if (request.getRequestId() != 0) {
116+
if (request == null) {
117+
break;
118+
}
119+
if (request.getRequestId() != 0) {
84120
Thread t = createResponseThread(request);
85-
t.start();
86-
} else {
121+
t.start();
122+
} else {
87123
respondToRequest(request);
88124
}
89125
}
@@ -127,11 +163,62 @@ void respondToRequest(WorkRequest request) throws IOException {
127163
synchronized (this) {
128164
messageProcessor.writeWorkResponse(workResponse);
129165
}
166+
gcScheduler.maybePerformGc();
130167
}
131168
}
132169

133170
@Override
134171
public void close() throws IOException {
135172
messageProcessor.close();
136173
}
174+
175+
/**
176+
* Class that performs GC occasionally, based on how much CPU time has passed. This strikes a
177+
* compromise between blindly doing GC after e.g. every request, which takes too much CPU, and not
178+
* doing explicit GC at all, which causes poor garbage collection in some cases.
179+
*/
180+
private static class CpuTimeBasedGcScheduler {
181+
/**
182+
* After this much CPU time has elapsed, we may force a GC run. Set to {@link Duration#ZERO} to
183+
* disable.
184+
*/
185+
private final Duration cpuUsageBeforeGc;
186+
187+
/** The total process CPU time at the last GC run (or from the start of the worker). */
188+
private final AtomicReference<Duration> cpuTimeAtLastGc;
189+
190+
/** Used to get the CPU time used by this process. */
191+
private static final OperatingSystemMXBean bean =
192+
(OperatingSystemMXBean) ManagementFactory.getOperatingSystemMXBean();
193+
194+
/**
195+
* Creates a new {@link CpuTimeBasedGcScheduler} that may perform GC after {@code
196+
* cpuUsageBeforeGc} amount of CPU time has been used.
197+
*/
198+
public CpuTimeBasedGcScheduler(Duration cpuUsageBeforeGc) {
199+
this.cpuUsageBeforeGc = cpuUsageBeforeGc;
200+
this.cpuTimeAtLastGc = new AtomicReference<>(getCpuTime());
201+
}
202+
203+
private Duration getCpuTime() {
204+
return !cpuUsageBeforeGc.isZero()
205+
? Duration.ofNanos(bean.getProcessCpuTime())
206+
: Duration.ZERO;
207+
}
208+
209+
/** Call occasionally to perform a GC if enough CPU time has been used. */
210+
private void maybePerformGc() {
211+
if (!cpuUsageBeforeGc.isZero()) {
212+
Duration currentCpuTime = getCpuTime();
213+
Duration lastCpuTime = cpuTimeAtLastGc.get();
214+
// Do GC when enough CPU time has been used, but only if nobody else beat us to it.
215+
if (currentCpuTime.minus(lastCpuTime).compareTo(cpuUsageBeforeGc) > 0
216+
&& cpuTimeAtLastGc.compareAndSet(lastCpuTime, currentCpuTime)) {
217+
System.gc();
218+
// Avoid counting GC CPU time against CPU time before next GC.
219+
cpuTimeAtLastGc.compareAndSet(currentCpuTime, getCpuTime());
220+
}
221+
}
222+
}
223+
}
137224
}

0 commit comments

Comments
 (0)