Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Commit

Permalink
Fix error prone issues.
Browse files Browse the repository at this point in the history
Summary:
Miscellaneous issues detected with error prone.

Extracted from #1047 .

Test Plan: CI

Reviewed By: illicitonion, Coneko

fbshipit-source-id: c55ad90
  • Loading branch information
davido authored and facebook-github-bot committed Mar 10, 2017
1 parent a1228fc commit cccc595
Show file tree
Hide file tree
Showing 11 changed files with 108 additions and 99 deletions.
100 changes: 53 additions & 47 deletions src/com/facebook/buck/event/listener/ChromeTraceBuildListener.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@
import java.util.Locale;
import java.util.Optional;
import java.util.TimeZone;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -258,18 +258,18 @@ public void outputTrace(BuildId buildId) {

@Subscribe
public void commandStarted(CommandEvent.Started started) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
started.getCommandName(),
ChromeTraceEvent.Phase.BEGIN,
ImmutableMap.of(
"command_args", Joiner.on(' ').join(started.getArgs())
),
ImmutableMap.of("command_args", Joiner.on(' ').join(started.getArgs())),
started);
}

@Subscribe
public void commandFinished(CommandEvent.Finished finished) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
finished.getCommandName(),
ChromeTraceEvent.Phase.END,
ImmutableMap.of(
Expand All @@ -280,7 +280,8 @@ public void commandFinished(CommandEvent.Finished finished) {

@Subscribe
public void buildStarted(BuildEvent.Started started) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
"build",
ChromeTraceEvent.Phase.BEGIN,
ImmutableMap.of(),
Expand All @@ -289,7 +290,8 @@ public void buildStarted(BuildEvent.Started started) {

@Subscribe
public synchronized void buildFinished(BuildEvent.Finished finished) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
"build",
ChromeTraceEvent.Phase.END,
ImmutableMap.of(),
Expand All @@ -299,7 +301,8 @@ public synchronized void buildFinished(BuildEvent.Finished finished) {
@Subscribe
public void ruleStarted(BuildRuleEvent.Started started) {
BuildRule buildRule = started.getBuildRule();
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
buildRule.getFullyQualifiedName(),
ChromeTraceEvent.Phase.BEGIN,
ImmutableMap.of(),
Expand All @@ -313,9 +316,7 @@ public void ruleFinished(BuildRuleEvent.Finished finished) {
ChromeTraceEvent.Phase.END,
ImmutableMap.of(
"cache_result", finished.getCacheResult().toString().toLowerCase(),
"success_type",
finished.getSuccessType().map(Object::toString).orElse("failed")
),
"success_type", finished.getSuccessType().map(Object::toString).orElse("failed")),
finished);
}

Expand All @@ -333,7 +334,8 @@ public void ruleResumed(BuildRuleEvent.Resumed resumed) {
@Subscribe
public void ruleSuspended(BuildRuleEvent.Suspended suspended) {
BuildRule buildRule = suspended.getBuildRule();
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
buildRule.getFullyQualifiedName(),
ChromeTraceEvent.Phase.END,
ImmutableMap.of("rule_key", suspended.getRuleKey()),
Expand All @@ -342,7 +344,8 @@ public void ruleSuspended(BuildRuleEvent.Suspended suspended) {

@Subscribe
public void stepStarted(StepEvent.Started started) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
started.getShortStepName(),
ChromeTraceEvent.Phase.BEGIN,
ImmutableMap.of(),
Expand All @@ -351,7 +354,8 @@ public void stepStarted(StepEvent.Started started) {

@Subscribe
public void stepFinished(StepEvent.Finished finished) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
finished.getShortStepName(),
ChromeTraceEvent.Phase.END,
ImmutableMap.of(
Expand All @@ -374,9 +378,7 @@ public void parseFinished(ParseEvent.Finished finished) {
writeChromeTraceEvent("buck",
"parse",
ChromeTraceEvent.Phase.END,
ImmutableMap.of(
"targets",
Joiner.on(",").join(finished.getBuildTargets())),
ImmutableMap.of("targets", Joiner.on(",").join(finished.getBuildTargets())),
finished);
}

Expand Down Expand Up @@ -404,8 +406,7 @@ public void simplePerfEvent(SimplePerfEvent perfEvent) {
"buck",
CONVERTED_EVENT_ID_CACHE.get(perfEvent.getEventId().getValue().intern()),
phase,
ImmutableMap.copyOf(
Maps.transformValues(perfEvent.getEventInfo(), Object::toString)),
ImmutableMap.copyOf(Maps.transformValues(perfEvent.getEventInfo(), Object::toString)),
perfEvent);
} catch (ExecutionException e) {
LOG.warn("Unable to log perf event " + perfEvent, e);
Expand All @@ -418,9 +419,7 @@ public void parseBuckFileStarted(ParseBuckFileEvent.Started started) {
"buck",
"parse_file",
ChromeTraceEvent.Phase.BEGIN,
ImmutableMap.of(
"path",
started.getBuckFilePath().toString()),
ImmutableMap.of("path", started.getBuckFilePath().toString()),
started);
}

Expand Down Expand Up @@ -482,7 +481,8 @@ public void actionGraphCacheMiss(ActionGraphEvent.Cache.Miss miss) {

@Subscribe
public void installStarted(InstallEvent.Started started) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
"install",
ChromeTraceEvent.Phase.BEGIN,
ImmutableMap.of(),
Expand All @@ -491,7 +491,8 @@ public void installStarted(InstallEvent.Started started) {

@Subscribe
public void installFinished(InstallEvent.Finished finished) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
"install",
ChromeTraceEvent.Phase.END,
ImmutableMap.of(
Expand All @@ -502,7 +503,8 @@ public void installFinished(InstallEvent.Finished finished) {

@Subscribe
public void startActivityStarted(StartActivityEvent.Started started) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
"start_activity",
ChromeTraceEvent.Phase.BEGIN,
ImmutableMap.of(),
Expand All @@ -511,7 +513,8 @@ public void startActivityStarted(StartActivityEvent.Started started) {

@Subscribe
public void startActivityFinished(StartActivityEvent.Finished finished) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
"start_activity",
ChromeTraceEvent.Phase.END,
ImmutableMap.of(
Expand All @@ -523,7 +526,8 @@ public void startActivityFinished(StartActivityEvent.Finished finished) {

@Subscribe
public void uninstallStarted(UninstallEvent.Started started) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
"uninstall",
ChromeTraceEvent.Phase.BEGIN,
ImmutableMap.of(),
Expand All @@ -532,7 +536,8 @@ public void uninstallStarted(UninstallEvent.Started started) {

@Subscribe
public void uninstallFinished(UninstallEvent.Finished finished) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
"uninstall",
ChromeTraceEvent.Phase.END,
ImmutableMap.of(
Expand Down Expand Up @@ -590,7 +595,8 @@ public void writeArtifactCompressionEvent(

@Subscribe
public void artifactConnectStarted(ArtifactCacheConnectEvent.Started started) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
"artifact_connect",
ChromeTraceEvent.Phase.BEGIN,
ImmutableMap.of(),
Expand All @@ -599,7 +605,8 @@ public void artifactConnectStarted(ArtifactCacheConnectEvent.Started started) {

@Subscribe
public void artifactConnectFinished(ArtifactCacheConnectEvent.Finished finished) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
"artifact_connect",
ChromeTraceEvent.Phase.END,
ImmutableMap.of(),
Expand Down Expand Up @@ -684,12 +691,12 @@ public void memoryPerfStats(PerfStatsTracking.MemoryPerfStatsEvent memory) {
Long.toString(
TimeUnit.MILLISECONDS.toSeconds(memory.getTimeSpentInGcMs())))
.putAll(
memory.getCurrentMemoryBytesUsageByPool().entrySet().stream().map(
e -> Maps.immutableEntry(
"pool_" + e.getKey() + "_mb",
Long.toString(SizeUnit.BYTES.toMegabytes(e.getValue()))))
.collect(Collectors.toList())
)
memory.getCurrentMemoryBytesUsageByPool().entrySet().stream()
.map(
e -> Maps.immutableEntry(
"pool_" + e.getKey() + "_mb",
Long.toString(SizeUnit.BYTES.toMegabytes(e.getValue()))))
.collect(Collectors.toList()))
.build(),
memory);
}
Expand Down Expand Up @@ -725,13 +732,13 @@ public void processResourceConsumption(ProcessTracker.ProcessResourceConsumption
"process",
ChromeTraceEvent.Phase.COUNTER,
builder.build(),
event
);
event);
}

@Subscribe
public void testStartedEvent(TestSummaryEvent.Started started) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
"test",
ChromeTraceEvent.Phase.BEGIN,
ImmutableMap.of(
Expand All @@ -742,7 +749,8 @@ public void testStartedEvent(TestSummaryEvent.Started started) {

@Subscribe
public void testFinishedEvent(TestSummaryEvent.Finished finished) {
writeChromeTraceEvent("buck",
writeChromeTraceEvent(
"buck",
"test",
ChromeTraceEvent.Phase.END,
ImmutableMap.of(
Expand Down Expand Up @@ -789,20 +797,18 @@ private void writeChromeTraceEvent(String category,

@SuppressWarnings("PMD.EmptyCatchBlock")
private void submitTraceEvent(final ChromeTraceEvent chromeTraceEvent) {
outputExecutor.submit(new Callable<Void>() {
@Override
public Void call() throws Exception {
@SuppressWarnings("unused") Future<?> unused =
outputExecutor.submit(() -> {
try {
mapper.writeValue(jsonGenerator, chromeTraceEvent);
} catch (IOException e) {
// Swallow any failures to write.
}
return null;
}
});
});
}

private class TracePathAndStream {
private static class TracePathAndStream {
private final Path path;
private final OutputStream stream;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import java.nio.file.Path;
import java.util.OptionalInt;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;

public class MachineReadableLoggerListener implements BuckEventListener {
Expand Down Expand Up @@ -184,7 +185,7 @@ private void writeToLog(final String prefix, final Object obj) {
public void outputTrace(BuildId buildId) throws InterruptedException {
// IMPORTANT: logging the ExitCode must happen on the executor, otherwise random
// log lines will be overwritten as outputStream access is not thread safe.
executor.submit(() -> {
@SuppressWarnings("unused") Future<?> unused = executor.submit(() -> {
try {
outputStream.write(
String.format("ExitCode {\"exitCode\":%d}", exitCode.orElse(-1))
Expand Down
9 changes: 6 additions & 3 deletions src/com/facebook/buck/event/listener/NetworkStatsKeeper.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.time.Duration;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;

Expand Down Expand Up @@ -58,16 +59,18 @@ public NetworkStatsKeeper() {
this.totalDownloadTimeMillis = 0;
this.currentIntervalDownloadTimeMillis = 0;
this.clock = new DefaultClock();
this.scheduler = Executors.newScheduledThreadPool(1,
new ThreadFactoryBuilder().setNameFormat(getClass().getSimpleName() + "-%d").build());
this.scheduler =
Executors.newScheduledThreadPool(
1,
new ThreadFactoryBuilder().setNameFormat(getClass().getSimpleName() + "-%d").build());
scheduleDownloadSpeedCalculation();
}

private void scheduleDownloadSpeedCalculation() {
long calculationInterval = DOWNLOAD_SPEED_CALCULATION_INTERVAL.toMillis();
TimeUnit timeUnit = TimeUnit.MILLISECONDS;

scheduler.scheduleAtFixedRate(
@SuppressWarnings("unused") ScheduledFuture<?> unused = scheduler.scheduleAtFixedRate(
this::calculateDownloadSpeedInLastInterval,
/* initialDelay */ calculationInterval,
/* period */ calculationInterval,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public synchronized void commandFinished(CommandEvent.Finished finished) {
}


public class PerfTimesEvent extends AbstractBuckEvent {
public static class PerfTimesEvent extends AbstractBuckEvent {

@JsonView(JsonViews.MachineReadableLog.class)
private PerfTimesStats perfTimesStats;
Expand Down
29 changes: 15 additions & 14 deletions src/com/facebook/buck/event/listener/RuleKeyLoggerListener.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@

import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;

public class RuleKeyLoggerListener implements BuckEventListener {
private static final Logger LOG = Logger.get(RuleKeyLoggerListener.class);
Expand Down Expand Up @@ -152,18 +154,16 @@ public Path getLogFilePath() {

private void flushLogLinesIfNeeded() {
if (logLinesCount > minLinesForAutoFlush) {
outputExecutor.submit(new Callable<Void>() {
@Override
public Void call() throws Exception {
try {
flushLogLines();
} catch (IOException e) {
LOG.error(e, "Failed to flush logLines from the outputExecutor.");
}

return null;
}
});
@SuppressWarnings("unused") Future<?> unused =
outputExecutor.submit(() -> {
try {
flushLogLines();
} catch (IOException e) {
LOG.error(e, "Failed to flush logLines from the outputExecutor.");
}

return null;
});
}
}

Expand All @@ -181,7 +181,8 @@ private void flushLogLines() throws IOException {
Path logFile = getLogFilePath();
projectFilesystem.createParentDirs(logFile);
try (FileOutputStream stream = new FileOutputStream(logFile.toString(), /* append */ true);
PrintWriter writer = new PrintWriter(stream)) {
OutputStreamWriter streamWriter = new OutputStreamWriter(stream, StandardCharsets.UTF_8);
PrintWriter writer = new PrintWriter(streamWriter)) {
for (String line : linesToFlush) {
writer.println(line);
}
Expand Down
Loading

0 comments on commit cccc595

Please sign in to comment.