From 723f7a6593ab372163cb92782c2645a9e9ba922b Mon Sep 17 00:00:00 2001 From: Jozef Vilcek Date: Sat, 18 Jul 2020 11:07:36 +0200 Subject: [PATCH] Improve performance of currentVersion by caching --- .../infrastructure/DryRepository.groovy | 5 + .../infrastructure/DummyRepository.groovy | 5 + .../release/infrastructure/git/ScmCache.java | 106 ++++++++++++++++++ .../axion/release/domain/VersionResolver.java | 3 +- .../release/domain/scm/ScmRepository.java | 2 + .../infrastructure/git/GitRepository.java | 82 ++++++-------- 6 files changed, 154 insertions(+), 49 deletions(-) create mode 100644 src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/git/ScmCache.java diff --git a/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/DryRepository.groovy b/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/DryRepository.groovy index 736c5fba..ec248325 100644 --- a/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/DryRepository.groovy +++ b/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/DryRepository.groovy @@ -20,6 +20,11 @@ class DryRepository implements ScmRepository { this.delegateRepository = delegateRepository } + @Override + String id() { + return delegateRepository.id(); + } + @Override void fetchTags(ScmIdentity identity, String remoteName) { log("fetching tags from remote") diff --git a/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/DummyRepository.groovy b/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/DummyRepository.groovy index 555fa4bd..9b7a86e7 100644 --- a/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/DummyRepository.groovy +++ b/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/DummyRepository.groovy @@ -21,6 +21,11 @@ class DummyRepository implements ScmRepository { logger.quiet("Couldn't perform $commandName command on uninitialized repository") } + @Override + String id() { + return "DummyRepository" + } + @Override void fetchTags(ScmIdentity identity, String remoteName) { log('fetch tags') diff --git a/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/git/ScmCache.java b/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/git/ScmCache.java new file mode 100644 index 00000000..afabe932 --- /dev/null +++ b/src/main/groovy/pl/allegro/tech/build/axion/release/infrastructure/git/ScmCache.java @@ -0,0 +1,106 @@ +package pl.allegro.tech.build.axion.release.infrastructure.git; + +import groovy.lang.Tuple2; +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; +import pl.allegro.tech.build.axion.release.domain.scm.ScmException; +import pl.allegro.tech.build.axion.release.domain.scm.ScmRepository; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * Provides cached version for some operations on {@link ScmRepository} + */ +public class ScmCache { + + /** + * Since this cache is statis and Gradle Demon might keep JVM process in background for a long + * time, we have to put some TTL for cached values. + */ + private static final long INVALIDATE_AFTER_MILLIS = 1000 * 60; + + private static final ScmCache CACHE = new ScmCache(); + + public static ScmCache getInstance() { + return CACHE; + } + + private ScmCache() { } + + private final Map cache = new HashMap<>(); + + synchronized void invalidate(ScmRepository repository) { + cache.remove(repository.id()); + } + + public synchronized boolean checkUncommittedChanges(ScmRepository repository) { + CachedState state = retrieveCachedStateFor(repository); + if (state.hasUncommittedChanges == null) { + state.hasUncommittedChanges = repository.checkUncommittedChanges(); + } + return state.hasUncommittedChanges; + } + + synchronized List> parsedTagList(ScmRepository repository, Git git, RevWalk walker) throws GitAPIException { + CachedState state = retrieveCachedStateFor(repository); + if (state.tags == null) { + List> list = new ArrayList<>(); + for (Ref tag : git.tagList().call()) { + try { + list.add(new Tuple2<>(tag, walker.parseCommit(tag.getObjectId()))); + } catch (IOException e) { + throw new ScmException(e); + } + } + state.tags = list; + } + return state.tags; + } + + private CachedState retrieveCachedStateFor(ScmRepository scmRepository) { + String key = scmRepository.id(); + String currentHeadRevision = scmRepository.currentPosition().getRevision(); + long currentTime = System.currentTimeMillis(); + CachedState state = cache.get(key); + if (state == null) { + state = new CachedState(currentHeadRevision); + cache.put(key, state); + } else { + if (!currentHeadRevision.equals(state.headRevision) || (state.createTimestamp + INVALIDATE_AFTER_MILLIS) < currentTime) { + state = new CachedState(currentHeadRevision); + cache.put(key, state); + } + } + return state; + } + + /** + * Helper object holding cached values per SCM repository + */ + private static class CachedState { + + final long createTimestamp; + + /** + * HEAD revision of repo for which this cache was created. Cache has to be invalidated + * if HEAD changes. + */ + final String headRevision; + + Boolean hasUncommittedChanges; + + List> tags; + + CachedState(String headRevision) { + createTimestamp = System.currentTimeMillis(); + this.headRevision = headRevision; + } + } +} diff --git a/src/main/java/pl/allegro/tech/build/axion/release/domain/VersionResolver.java b/src/main/java/pl/allegro/tech/build/axion/release/domain/VersionResolver.java index 4a298512..bd6a67e8 100644 --- a/src/main/java/pl/allegro/tech/build/axion/release/domain/VersionResolver.java +++ b/src/main/java/pl/allegro/tech/build/axion/release/domain/VersionResolver.java @@ -7,6 +7,7 @@ import pl.allegro.tech.build.axion.release.domain.scm.ScmPosition; import pl.allegro.tech.build.axion.release.domain.scm.ScmRepository; import pl.allegro.tech.build.axion.release.domain.scm.TaggedCommits; +import pl.allegro.tech.build.axion.release.infrastructure.git.ScmCache; import java.util.regex.Pattern; @@ -55,7 +56,7 @@ public VersionContext resolveVersion(VersionProperties versionProperties, TagPro versions.onReleaseTag, versions.onNextVersionTag, versions.noTagsFound, - repository.checkUncommittedChanges() + ScmCache.getInstance().checkUncommittedChanges(repository) ); VersionFactory.FinalVersion finalVersion = versionFactory.createFinalVersion(scmState, versions.current); diff --git a/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmRepository.java b/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmRepository.java index e77d2f23..0d98caf6 100644 --- a/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmRepository.java +++ b/src/main/java/pl/allegro/tech/build/axion/release/domain/scm/ScmRepository.java @@ -5,6 +5,8 @@ public interface ScmRepository { + String id(); + void fetchTags(ScmIdentity identity, String remoteName); void tag(String tagName); diff --git a/src/main/java/pl/allegro/tech/build/axion/release/infrastructure/git/GitRepository.java b/src/main/java/pl/allegro/tech/build/axion/release/infrastructure/git/GitRepository.java index fae074c3..24aa89ba 100644 --- a/src/main/java/pl/allegro/tech/build/axion/release/infrastructure/git/GitRepository.java +++ b/src/main/java/pl/allegro/tech/build/axion/release/infrastructure/git/GitRepository.java @@ -1,5 +1,6 @@ package pl.allegro.tech.build.axion.release.infrastructure.git; +import groovy.lang.Tuple2; import org.eclipse.jgit.api.*; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.NoHeadException; @@ -9,6 +10,7 @@ import org.eclipse.jgit.revwalk.RevSort; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.*; +import org.eclipse.jgit.util.FS; import pl.allegro.tech.build.axion.release.domain.logging.ReleaseLogger; import pl.allegro.tech.build.axion.release.domain.scm.*; @@ -16,6 +18,7 @@ import java.io.IOException; import java.net.URISyntaxException; import java.util.*; +import java.util.function.Supplier; import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -33,7 +36,8 @@ public class GitRepository implements ScmRepository { public GitRepository(ScmProperties properties) { try { this.repositoryDir = properties.getDirectory(); - this.jgitRepository = Git.open(repositoryDir); + RepositoryCache.FileKey key = RepositoryCache.FileKey.lenient(repositoryDir, FS.DETECTED); + this.jgitRepository = Git.wrap(RepositoryCache.open(key, true)); this.properties = properties; } catch (RepositoryNotFoundException exception) { throw new ScmRepositoryUnavailableException(exception); @@ -51,6 +55,11 @@ public GitRepository(ScmProperties properties) { } + @Override + public String id() { + return repositoryDir.getAbsolutePath(); + } + /** * This fetch method behaves like git fetch, meaning it only fetches thing without merging. * As a result, any fetched tags will not be visible via GitRepository tag listing methods @@ -93,6 +102,7 @@ public void tag(final String tagName) { if (!isOnExistingTag) { jgitRepository.tag().setName(tagName).call(); + ScmCache.getInstance().invalidate(this); } else { logger.debug("The head commit " + headId + " already has the tag " + tagName + "."); } @@ -109,6 +119,7 @@ private ObjectId head() throws IOException { public void dropTag(String tagName) { try { jgitRepository.tagDelete().setTags(GIT_TAG_PREFIX + tagName).call(); + ScmCache.getInstance().invalidate(this); } catch (GitAPIException e) { throw new ScmException(e); } @@ -139,7 +150,9 @@ public ScmPushResult push(ScmIdentity identity, ScmPushOptions pushOptions, bool private Iterable callPush(PushCommand pushCommand) { try { - return pushCommand.call(); + Iterable result = pushCommand.call(); + ScmCache.getInstance().invalidate(this); + return result; } catch (GitAPIException e) { throw new ScmException(e); } @@ -204,6 +217,7 @@ public void commit(List patterns, String message) { } jgitRepository.commit().setMessage(message).call(); + ScmCache.getInstance().invalidate(this); } catch (GitAPIException | IOException e) { throw new ScmException(e); } @@ -316,9 +330,8 @@ public List taggedCommits(Pattern pattern) { } private List taggedCommitsInternal(Pattern pattern, String maybeSinceCommit, boolean inclusive, boolean stopOnFirstTag) { - List taggedCommits = new ArrayList<>(); if (!hasCommits()) { - return taggedCommits; + return new ArrayList<>(); } try { @@ -333,38 +346,18 @@ private List taggedCommitsInternal(Pattern pattern, String maybeSi RevWalk walk = walker(startingCommit); - if (!inclusive) { - walk.next(); - } - - Map> allTags = tagsMatching(pattern, walk); - - RevCommit currentCommit; - List currentTagsList; - for (currentCommit = walk.next(); currentCommit != null; currentCommit = walk.next()) { - currentTagsList = allTags.get(currentCommit.getId().getName()); - - if (currentTagsList != null) { - TagsOnCommit taggedCommit = new TagsOnCommit( - currentCommit.getId().name(), - currentTagsList - ); - taggedCommits.add(taggedCommit); - - if (stopOnFirstTag) { - break; - } + try { + if (!inclusive) { + walk.next(); } - + return tagsMatching(pattern, walk, headId); + } finally { + walk.dispose(); } - - walk.dispose(); } catch (IOException | GitAPIException e) { throw new ScmException(e); } - - return taggedCommits; } private RevWalk walker(ObjectId startingCommit) throws IOException { @@ -379,28 +372,21 @@ private RevWalk walker(ObjectId startingCommit) throws IOException { return walk; } - private Map> tagsMatching(Pattern pattern, RevWalk walk) throws GitAPIException { - List tags = jgitRepository.tagList().call(); + private List tagsMatching(Pattern pattern, RevWalk walk, ObjectId head) throws GitAPIException { + List> parsedTagList = ScmCache.getInstance().parsedTagList(this, jgitRepository, walk); - return tags.stream() - .map(tag -> new TagNameAndId( - tag.getName().substring(GIT_TAG_PREFIX.length()), - parseCommitSafe(walk, tag.getObjectId()) - )) + return parsedTagList.stream() + .map(pair -> new TagNameAndId( + pair.getFirst().getName().substring(GIT_TAG_PREFIX.length()), + pair.getSecond().getName())) .filter(t -> pattern.matcher(t.name).matches()) .collect( - HashMap::new, + (Supplier>>) HashMap::new, (m, t) -> m.computeIfAbsent(t.id, (s) -> new ArrayList<>()).add(t.name), - HashMap::putAll - ); - } - - private String parseCommitSafe(RevWalk walk, AnyObjectId commitId) { - try { - return walk.parseCommit(commitId).getName(); - } catch (IOException e) { - throw new ScmException(e); - } + HashMap::putAll) + .entrySet() + .stream().map(entry -> new TagsOnCommit(entry.getKey(), entry.getValue())) + .collect(Collectors.toList()); } private final static class TagNameAndId {