From 620e22c7cfa2c38dd067aee66cbe6d36912cf6b3 Mon Sep 17 00:00:00 2001 From: Morten Telling Date: Mon, 22 Jan 2024 18:53:43 +0100 Subject: [PATCH 1/4] Add instrumentation to allow warning on files that take long to format --- .../javaformat/java/CommandLineOptions.java | 19 ++++++++- .../java/InstrumentedFormatFileCallable.java | 41 +++++++++++++++++++ .../com/palantir/javaformat/java/Main.java | 24 +++++++++-- 3 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 palantir-java-format/src/main/java/com/palantir/javaformat/java/InstrumentedFormatFileCallable.java 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..3eb7c2fc8 --- /dev/null +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/InstrumentedFormatFileCallable.java @@ -0,0 +1,41 @@ +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..9caf9f027 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() + && parameters.warnOnExpensiveFileDurationMillis().get() + > fileResult.duration().toMillis()) { + 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 { From 146ca0faf037218c05ebf75149c7afa36769a35d Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Mon, 22 Jan 2024 18:00:10 +0000 Subject: [PATCH 2/4] Add generated changelog entries --- changelog/@unreleased/pr-989.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-989.v2.yml 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 From 480a2e8b2d19f4e0a1ff15618e8c87afcf0abdb2 Mon Sep 17 00:00:00 2001 From: Morten Telling Date: Mon, 22 Jan 2024 19:09:07 +0100 Subject: [PATCH 3/4] Fix spotless --- .../java/InstrumentedFormatFileCallable.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) 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 index 3eb7c2fc8..6eacc70ed 100644 --- 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 @@ -1,3 +1,18 @@ +/* + * (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; From 0b8928de8bfe52b0a14da129e1a43255583f2bea Mon Sep 17 00:00:00 2001 From: Morten Telling Date: Mon, 22 Jan 2024 19:09:35 +0100 Subject: [PATCH 4/4] Fix condition --- .../src/main/java/com/palantir/javaformat/java/Main.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 9caf9f027..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 @@ -185,8 +185,8 @@ private int formatFiles(CommandLineOptions parameters, JavaFormatterOptions opti private void warnIfDurationExceedsThreshold(CommandLineOptions parameters, Path path, FormatFileResult fileResult) { if (parameters.warnOnExpensiveFileDurationMillis().isPresent() - && parameters.warnOnExpensiveFileDurationMillis().get() - > fileResult.duration().toMillis()) { + && 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");