diff --git a/changelog/@unreleased/pr-989.v2.yml b/changelog/@unreleased/pr-989.v2.yml new file mode 100644 index 000000000..75cf7d388 --- /dev/null +++ b/changelog/@unreleased/pr-989.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Add instrumentation to allow warning on files that take long to format + links: + - https://github.com/palantir/palantir-java-format/pull/989 diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java index 81defda5d..53cda5806 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java @@ -45,6 +45,7 @@ final class CommandLineOptions { private final Optional assumeFilename; private final boolean reflowLongStrings; private final boolean outputReplacements; + private final Optional warnOnExpensiveFileDurationMillis; CommandLineOptions( ImmutableList files, @@ -65,7 +66,8 @@ final class CommandLineOptions { boolean setExitIfChanged, Optional assumeFilename, boolean reflowLongStrings, - boolean outputReplacements) { + boolean outputReplacements, + Optional warnOnExpensiveFileDurationMillis) { this.files = files; this.inPlace = inPlace; this.lines = lines; @@ -85,6 +87,7 @@ final class CommandLineOptions { this.assumeFilename = assumeFilename; this.reflowLongStrings = reflowLongStrings; this.outputReplacements = outputReplacements; + this.warnOnExpensiveFileDurationMillis = warnOnExpensiveFileDurationMillis; } /** The files to format. */ @@ -185,6 +188,11 @@ boolean outputReplacements() { return outputReplacements; } + /** Returns the number of millis to allow processing of a single file to take before warning. */ + Optional warnOnExpensiveFileDurationMillis() { + return warnOnExpensiveFileDurationMillis; + } + static Builder builder() { return new Builder(); } @@ -210,6 +218,7 @@ static final class Builder { private Optional assumeFilename = Optional.empty(); private boolean reflowLongStrings = true; private boolean outputReplacements = false; + private Optional warnOnExpensiveFileDurationMillis = Optional.empty(); private Builder() {} @@ -305,6 +314,11 @@ Builder outputReplacements(boolean outputReplacements) { return this; } + Builder warnOnExpensiveFileDurationMillis(long warnOnExpensiveFileDurationMillis) { + this.warnOnExpensiveFileDurationMillis = Optional.of(warnOnExpensiveFileDurationMillis); + return this; + } + CommandLineOptions build() { Preconditions.checkArgument(!aosp || !palantirStyle, "Cannot use both aosp and palantir style"); return new CommandLineOptions( @@ -326,7 +340,8 @@ CommandLineOptions build() { setExitIfChanged, assumeFilename, reflowLongStrings, - outputReplacements); + outputReplacements, + warnOnExpensiveFileDurationMillis); } } } diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/InstrumentedFormatFileCallable.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/InstrumentedFormatFileCallable.java new file mode 100644 index 000000000..6eacc70ed --- /dev/null +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/InstrumentedFormatFileCallable.java @@ -0,0 +1,56 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.palantir.javaformat.java; + +import com.palantir.javaformat.java.InstrumentedFormatFileCallable.FormatFileResult; +import java.time.Duration; +import java.util.concurrent.Callable; +import org.immutables.value.Value.Immutable; + +/** + * Instrumentation on top of {@link FormatFileCallable} to track the time spent formatting a given file, and produce + * warnings + */ +public class InstrumentedFormatFileCallable implements Callable { + + private final FormatFileCallable delegate; + + public InstrumentedFormatFileCallable(FormatFileCallable delegate) { + this.delegate = delegate; + } + + @Override + public FormatFileResult call() throws Exception { + long start = System.currentTimeMillis(); + String result = delegate.call(); + Duration duration = Duration.ofMillis(System.currentTimeMillis() - start); + return FormatFileResult.of(result, duration); + } + + @Immutable + public interface FormatFileResult { + String result(); + + Duration duration(); + + static FormatFileResult of(String result, Duration duration) { + return ImmutableFormatFileResult.builder() + .result(result) + .duration(duration) + .build(); + } + } +} diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/Main.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/Main.java index c4a9067cf..17e602cd3 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/Main.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/Main.java @@ -17,6 +17,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.io.ByteStreams; +import com.palantir.javaformat.java.InstrumentedFormatFileCallable.FormatFileResult; import com.palantir.javaformat.java.JavaFormatterOptions.Style; import java.io.IOException; import java.io.InputStream; @@ -110,7 +111,7 @@ private int formatFiles(CommandLineOptions parameters, JavaFormatterOptions opti ExecutorService executorService = Executors.newFixedThreadPool(numThreads); Map inputs = new LinkedHashMap<>(); - Map> results = new LinkedHashMap<>(); + Map> results = new LinkedHashMap<>(); boolean allOk = true; for (String fileName : parameters.files()) { @@ -123,18 +124,23 @@ private int formatFiles(CommandLineOptions parameters, JavaFormatterOptions opti try { input = new String(Files.readAllBytes(path), UTF_8); inputs.put(path, input); - results.put(path, executorService.submit(new FormatFileCallable(parameters, input, options))); + results.put( + path, + executorService.submit(new InstrumentedFormatFileCallable( + new FormatFileCallable(parameters, input, options)))); } catch (IOException e) { errWriter.println(fileName + ": could not read file: " + e.getMessage()); allOk = false; } } - for (Map.Entry> result : results.entrySet()) { + for (Map.Entry> result : results.entrySet()) { Path path = result.getKey(); String formatted; try { - formatted = result.getValue().get(); + FormatFileResult fileResult = result.getValue().get(); + formatted = fileResult.result(); + warnIfDurationExceedsThreshold(parameters, path, fileResult); } catch (InterruptedException e) { errWriter.println(e.getMessage()); allOk = false; @@ -177,6 +183,16 @@ private int formatFiles(CommandLineOptions parameters, JavaFormatterOptions opti return allOk ? 0 : 1; } + private void warnIfDurationExceedsThreshold(CommandLineOptions parameters, Path path, FormatFileResult fileResult) { + if (parameters.warnOnExpensiveFileDurationMillis().isPresent() + && fileResult.duration().toMillis() + > parameters.warnOnExpensiveFileDurationMillis().get()) { + errWriter.println(path + ": took " + fileResult.duration().toMillis() + "ms to format, " + + "which is longer than the threshold of " + + parameters.warnOnExpensiveFileDurationMillis().get() + "ms"); + } + } + private int formatStdin(CommandLineOptions parameters, JavaFormatterOptions options) { String input; try {