Skip to content

Commit

Permalink
Improve performance of currentVersion by caching
Browse files Browse the repository at this point in the history
  • Loading branch information
Jozef Vilcek committed Jul 18, 2020
1 parent 21368b6 commit 85f5be6
Show file tree
Hide file tree
Showing 7 changed files with 165 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String, CachedState> 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<Tuple2<Ref, RevCommit>> parsedTagList(ScmRepository repository, Git git, RevWalk walker) throws GitAPIException {
CachedState state = retrieveCachedStateFor(repository);
if (state.tags == null) {
List<Tuple2<Ref, RevCommit>> 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<Tuple2<Ref, RevCommit>> tags;

CachedState(String headRevision) {
createTimestamp = System.currentTimeMillis();
this.headRevision = headRevision;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

public interface ScmRepository {

String id();

void fetchTags(ScmIdentity identity, String remoteName);

void tag(String tagName);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -9,13 +10,15 @@
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.*;

import java.io.File;
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;
Expand All @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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 + ".");
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -139,7 +150,9 @@ public ScmPushResult push(ScmIdentity identity, ScmPushOptions pushOptions, bool

private Iterable<PushResult> callPush(PushCommand pushCommand) {
try {
return pushCommand.call();
Iterable<PushResult> result = pushCommand.call();
ScmCache.getInstance().invalidate(this);
return result;
} catch (GitAPIException e) {
throw new ScmException(e);
}
Expand Down Expand Up @@ -204,6 +217,7 @@ public void commit(List<String> patterns, String message) {
}

jgitRepository.commit().setMessage(message).call();
ScmCache.getInstance().invalidate(this);
} catch (GitAPIException | IOException e) {
throw new ScmException(e);
}
Expand Down Expand Up @@ -331,39 +345,36 @@ private List<TagsOnCommit> taggedCommitsInternal(Pattern pattern, String maybeSi
startingCommit = headId;
}


RevWalk walk = walker(startingCommit);
if (!inclusive) {
walk.next();
}

Map<String, List<String>> allTags = tagsMatching(pattern, walk);

RevCommit currentCommit;
List<String> 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();
}
Map<String, List<String>> allTags = tagsMatching(pattern, walk);
if (stopOnFirstTag) {
// stopOnFirstTag needs to get latest tag, therefore the order does matter
// order is given by walking the repository commits. this can be slower in some
// situations than returning all tagged commits
RevCommit currentCommit;
for (currentCommit = walk.next(); currentCommit != null; currentCommit = walk.next()) {
List<String> tagList = allTags.get(currentCommit.getId().getName());
if (tagList != null) {
TagsOnCommit taggedCommit = new TagsOnCommit(currentCommit.getId().name(), tagList);
taggedCommits.add(taggedCommit);
break;
}
}

} else {
// order not needed, we can just return all tagged commits
allTags.forEach((key, value) -> taggedCommits.add(new TagsOnCommit(key, value)));
}

} finally {
walk.dispose();
}

walk.dispose();
} catch (IOException | GitAPIException e) {
throw new ScmException(e);
}

return taggedCommits;
}

Expand All @@ -380,27 +391,17 @@ private RevWalk walker(ObjectId startingCommit) throws IOException {
}

private Map<String, List<String>> tagsMatching(Pattern pattern, RevWalk walk) throws GitAPIException {
List<Ref> tags = jgitRepository.tagList().call();
List<Tuple2<Ref, RevCommit>> 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,
(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);
}

private final static class TagNameAndId {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ class GitRepositoryTest extends Specification {
List<TagsOnCommit> allTaggedCommits = repository.taggedCommits(~/^release.*/)

then:
allTaggedCommits.collect { c -> c.tags[0] } == ['release-3', 'release-4', 'release-2', 'release-1']
allTaggedCommits.collect { c -> c.tags[0] }.toSet() == ['release-3', 'release-4', 'release-2', 'release-1'].toSet()
}

def "should return only tags that match with prefix"() {
Expand Down

0 comments on commit 85f5be6

Please sign in to comment.