Speed up class uniqueness analyses by caching results of reading jars #1837

AlexLandau · 2021-07-22T03:14:51Z

Before this PR

Running checkClassUniqueness could take a long time. In a sample build of [big monorepo], six projects took over a minute each for this to run, and the total runtime across all projects was 143 minutes.

After this PR

We cache the class names and corresponding hashes found in each jar in a shared Gradle service that lasts for the duration of the Gradle invocation. In [big monorepo], this reduces the total runtime to 8 minutes.

As an incidental additional fix, a regex intended to strip ".class" from the end of class names was .class instead of \.class$ and could also remove "class" from the middle of fully qualified class names. This has been fixed.

==COMMIT_MSG==
Increase the speed of the checkClassUniqueness task, especially in large repos, by adding caching of jar information.

Fix class names listed in the baseline-class-uniqueness.lock when the class or package name contains the substring class. In rare cases, this may require running ./gradlew checkClassUniqueness --write-locks to update the files.
==COMMIT_MSG==

Possible downsides?

The cache could increase memory use during the Gradle run, in proportion to the total dependency footprint of the repository.

changelog-app · 2021-07-22T03:14:55Z

Generate changelog in `changelog/@unreleased`

Type

Description

Increase the speed of the checkClassUniqueness task, especially in large repos, by adding caching of jar information.

Fix class names listed in the baseline-class-uniqueness.lock when the class or package name contains class. In rare cases, this may require running ./gradlew checkClassUniqueness --write-locks to update the files.

Check the box to generate changelog(s)

Generate changelog entry

This will likely close to double the runtime performance of a single task (sharing results between main and test classpaths) and should significantly improve the performance of these tasks in monorepos.

AlexLandau · 2021-07-22T03:35:49Z

gradle-baseline-java/src/main/groovy/com/palantir/baseline/services/JarClassHasher.java

+            Caffeine.newBuilder().build();
+
+    public static class Result {
+        private final ImmutableSetMultimap<String, HashCode> hashesByClassName;


I originally had this as just an ImmutableMap, but it failed on a case where it tried to enter the same key/value pair twice for some reason. I didn't collect the jar name, but I can share the class name internally.

Did you find out why that is? It means that we iterate over the same class name and class hash twice?

If you can reproduce, it might be interesting to retrieve the jar and class names.

I sent you the details internally.

It looks like the issue is because the published jar is malformed and contains duplicate files. A problem with the publisher and not with gradle-baseline so no need to block this PR based on that.

As a final thing, can we add a log when this happens? Ideally should hunt down these bad jars.

carterkozak

This feature seems like a good candidate for Gradle artifact transformations which are cached by gradle, and shouldn't require us to implement the caching/reuse logic. What do you think?

AlexLandau · 2021-07-22T22:24:11Z

This feature seems like a good candidate for Gradle artifact transformations which are cached by gradle, and shouldn't require us to implement the caching/reuse logic. What do you think?

That's an interesting feature, but it seems like it would be over-engineering to use it here. We'd have to add a new configuration for each requested configuration and resolve them to use the custom attribute, and we'd need serialization and deserialization of the results (each task would be reading the computed information back from disk).

There might be a more appropriate cache key than the ModuleVersionIdentifier, but we were already counting on that being unique in the existing logic.

AlexLandau · 2021-08-19T18:25:00Z

@carterkozak Any follow-up here?

The completion of my thought above is that using this type of in-memory caching is well-insulated from unintended interactions with other plugins (while still being reusable if we want to), whereas adding new configurations and dependencies (in order to use artifact transformations) would be more of a public API change.

carterkozak · 2021-08-19T21:34:52Z

I've added + pinged folks who are more knowledgeable on gradle than myself, I'd feel more comfortable if they signed off.

fawind · 2021-08-20T10:45:51Z

gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java

@@ -60,43 +56,26 @@ public void analyzeConfiguration(Configuration configuration) {
        Map<String, Set<ModuleVersionIdentifier>> classToJars = new HashMap<>();
        Map<String, Set<HashCode>> tempClassToHashCodes = new HashMap<>();


Any reason why we don't use a SetMultimap here as well? That way we also could get rid of the multiMapPut method.

I added this refactor to the PR. The section that looks for singleton-set values requires an asMap(), but that's the only wrinkle.

fawind · 2021-08-20T10:46:34Z

...seline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java

-        TaskProvider<CheckClassUniquenessLockTask> checkClassUniqueness =
-                project.getTasks().register("checkClassUniqueness", CheckClassUniquenessLockTask.class);
+        Provider<JarClassHasher> jarClassHasher = project.getGradle()
+                .getSharedServices()


The reason we use a BuildService here is to reuse the cache between tasks of different projects?

Correct. The other alternative I'm aware of is putting an extension on the root project and locating the cache there, which should also work.

fawind · 2021-08-20T10:50:04Z

gradle-baseline-java/src/main/groovy/com/palantir/baseline/services/JarClassHasher.java

+    private final Cache<ModuleVersionIdentifier, Result> cache =
+            Caffeine.newBuilder().build();
+
+    public static class Result {


Suggested change

public static class Result {

public static final class Result {

nit

carterkozak

lgtm

svc-autorelease · 2021-09-07T21:15:54Z

Released 4.24.0

AlexLandau force-pushed the alandau/cache-class-uniqueness-hashes branch 2 times, most recently from 3cad2d5 to 44ca021 Compare July 22, 2021 03:22

Speed up class uniqueness analyses by caching results of reading jars

8261c85

This will likely close to double the runtime performance of a single task (sharing results between main and test classpaths) and should significantly improve the performance of these tasks in monorepos.

AlexLandau force-pushed the alandau/cache-class-uniqueness-hashes branch from 44ca021 to 8261c85 Compare July 22, 2021 03:24

svc-changelog and others added 2 commits July 22, 2021 03:31

Add generated changelog entries

00a24c8

Small changelog edit

b27b64c

AlexLandau commented Jul 22, 2021

View reviewed changes

AlexLandau marked this pull request as ready for review July 22, 2021 03:37

policy-bot bot requested a review from tpetracca July 22, 2021 03:37

carterkozak reviewed Jul 22, 2021

View reviewed changes

carterkozak requested review from fawind and CRogers August 19, 2021 21:32

fawind reviewed Aug 20, 2021

View reviewed changes

Alex Landau added 3 commits August 20, 2021 13:07

Merge remote-tracking branch 'origin/develop' into cache-hashes

8366a4a

Address code review feedback

d097af1

Add logging when a jar has multiple entries for one class

7e73be8

policy-bot bot requested a review from carterkozak September 7, 2021 19:46

carterkozak approved these changes Sep 7, 2021

View reviewed changes

carterkozak added autorelease merge when ready labels Sep 7, 2021

bulldozer-bot bot merged commit f96d194 into develop Sep 7, 2021

bulldozer-bot bot deleted the alandau/cache-class-uniqueness-hashes branch September 7, 2021 21:15

svc-excavator-bot mentioned this pull request Sep 8, 2021

Excavator: Upgrades Baseline to the latest version palantir/witchcraft-java-logging#42

Merged

svc-excavator-bot mentioned this pull request Nov 16, 2021

Excavator: Upgrades Baseline to the latest version palantir/conjure-java-example#548

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Speed up class uniqueness analyses by caching results of reading jars #1837

Speed up class uniqueness analyses by caching results of reading jars #1837

AlexLandau commented Jul 22, 2021 •

edited

Loading

changelog-app bot commented Jul 22, 2021 •

edited by AlexLandau

Loading

AlexLandau Jul 22, 2021

fawind Aug 20, 2021

AlexLandau Aug 27, 2021

fawind Sep 6, 2021

fawind Sep 6, 2021

carterkozak left a comment

AlexLandau commented Jul 22, 2021

AlexLandau commented Aug 19, 2021

carterkozak commented Aug 19, 2021

fawind Aug 20, 2021

AlexLandau Aug 20, 2021

fawind Aug 20, 2021

AlexLandau Aug 20, 2021

fawind Aug 20, 2021

AlexLandau Aug 20, 2021

carterkozak left a comment

svc-autorelease commented Sep 7, 2021

		@@ -60,43 +56,26 @@ public void analyzeConfiguration(Configuration configuration) {
		Map<String, Set<ModuleVersionIdentifier>> classToJars = new HashMap<>();
		Map<String, Set<HashCode>> tempClassToHashCodes = new HashMap<>();

	public static class Result {
	public static final class Result {

Speed up class uniqueness analyses by caching results of reading jars #1837

Speed up class uniqueness analyses by caching results of reading jars #1837

Conversation

AlexLandau commented Jul 22, 2021 • edited Loading

Before this PR

After this PR

Possible downsides?

changelog-app bot commented Jul 22, 2021 • edited by AlexLandau Loading

Generate changelog in changelog/@unreleased

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carterkozak left a comment

Choose a reason for hiding this comment

AlexLandau commented Jul 22, 2021

AlexLandau commented Aug 19, 2021

carterkozak commented Aug 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carterkozak left a comment

Choose a reason for hiding this comment

svc-autorelease commented Sep 7, 2021

AlexLandau commented Jul 22, 2021 •

edited

Loading

changelog-app bot commented Jul 22, 2021 •

edited by AlexLandau

Loading

Generate changelog in `changelog/@unreleased`