Skip to content

Commit

Permalink
Load the commit cache in the background
Browse files Browse the repository at this point in the history 
This commit cache is used for the activity page and for the activity
graph top-right on project pages.

Apparently loading it can be expensive and take quite a while. The cache
is completely optional; if it isn't there, some pages may take somewhat
longer to display, but that appears less critical than spending all that
time in Gerrit startup. (The cache initialization happens when the
plugin is started, and Gerrit becomes fully operational only once all
its plugins are ready.)

Three things:

* Use isEmpty() instead of size() == 0; it may be faster. (For instance
  on ConcurrentHashMap.)
* Make the commit cache fully thread-safe. It was using a
  ConcurrentHashMap, but then performed several operations on it that
  should have been atomic, and moreover caches lists that were then
  manipulated directly.
* Finally, run the cache initialization in a background daemon thread.
  • Loading branch information
@tomaswolf
tomaswolf committed Aug 22, 2016
1 parent b2fdd97 commit db57941
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 54 deletions.
4 changes: 2 additions & 2 deletions docu/index.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ with full SSO through Gerrit.

* License: [Apache Public License 2.0](http://www.apache.org/licenses/LICENSE-2.0)
* [Home page](https://github.com/tomaswolf/gerrit-gitblit-plugin)
* Installed plugin version: <em id='gerrit-gitblit-current-version'>2.12.171.2-SNAPSHOT</em> &mdash; <a id='gerrit-gitblit-version-check' style='display:none;' href='#'>Check for updates</a>
* Installed plugin version: <em id='gerrit-gitblit-current-version'>2.12.171.3-SNAPSHOT</em> &mdash; <a id='gerrit-gitblit-version-check' style='display:none;' href='#'>Check for updates</a>

For a list of contributors, see at [GitHub](https://github.com/tomaswolf/gerrit-gitblit-plugin/graphs/contributors).

Expand Down Expand Up @@ -111,6 +111,6 @@ Report bugs or make feature requests at the [GitHub issue tracker](https://githu

<hr style="color: #C0C0C0; background-color: #C0C0C0; border-color: #C0C0C0; height: 2px;" />
<div style="float:right;">
<a href="https://github.com/tomaswolf/gerrit-gitblit-plugin" target="_blank">GitBlit plugin 2.12.171.2-SNAPSHOT</a>
<a href="https://github.com/tomaswolf/gerrit-gitblit-plugin" target="_blank">GitBlit plugin 2.12.171.3-SNAPSHOT</a>
</div>
<script type="text/javascript" src="version_check.js"></script>
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ limitations under the License.
<artifactId>gitblit-plugin</artifactId>
<description>GitBlit for Gerrit integrated as a plugin</description>
<name>Gerrit - GitBlit Plugin</name>
<version>2.12.171.2-SNAPSHOT</version><!-- Gerrit API version followed by collapsed GitBlit version, followed by plugin version -->
<version>2.12.171.3-SNAPSHOT</version><!-- Gerrit API version followed by collapsed GitBlit version, followed by plugin version -->
<licenses>
<license>
<name>Apache License 2.0</name>
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/gitblit/utils/ArrayUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public static boolean isEmpty(Object [] array) {
}

public static boolean isEmpty(Collection<?> collection) {
return collection == null || collection.size() == 0;
return collection == null || collection.isEmpty();
}

public static String toString(Collection<?> collection) {
Expand Down
118 changes: 68 additions & 50 deletions src/main/java/com/gitblit/utils/CommitCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;

import org.eclipse.jgit.lib.ObjectId;
Expand Down Expand Up @@ -58,7 +58,9 @@ public static CommitCache instance() {
}

protected CommitCache() {
cache = new ConcurrentHashMap<String, ObjectCache<List<RepositoryCommit>>>();
// Using a ConcurrentHashMap of lists and then manipulating those lists and even handing them out is not thread-safe at all.
// cache = new ConcurrentHashMap<String, ObjectCache<List<RepositoryCommit>>>();
cache = new HashMap<>();
}

/**
Expand Down Expand Up @@ -93,7 +95,9 @@ public synchronized void setCacheDays(int days) {
*
*/
public void clear() {
cache.clear();
synchronized (cache) {
cache.clear();
}
}

/**
Expand All @@ -103,8 +107,11 @@ public void clear() {
*/
public void clear(String repositoryName) {
String repoKey = repositoryName.toLowerCase();
ObjectCache<List<RepositoryCommit>> repoCache = cache.remove(repoKey);
if (repoCache != null) {
boolean hadEntries = false;
synchronized (cache) {
hadEntries = cache.remove(repoKey) != null;
}
if (hadEntries) {
logger.info(MessageFormat.format("{0} commit cache cleared", repositoryName));
}
}
Expand All @@ -117,13 +124,17 @@ public void clear(String repositoryName) {
*/
public void clear(String repositoryName, String branch) {
String repoKey = repositoryName.toLowerCase();
ObjectCache<List<RepositoryCommit>> repoCache = cache.get(repoKey);
if (repoCache != null) {
List<RepositoryCommit> commits = repoCache.remove(branch.toLowerCase());
if (!ArrayUtils.isEmpty(commits)) {
logger.info(MessageFormat.format("{0}:{1} commit cache cleared", repositoryName, branch));
boolean hadEntries = false;
synchronized (cache) {
ObjectCache<List<RepositoryCommit>> repoCache = cache.get(repoKey);
if (repoCache != null) {
List<RepositoryCommit> commits = repoCache.remove(branch.toLowerCase());
hadEntries = !ArrayUtils.isEmpty(commits);
}
}
if (hadEntries) {
logger.info(MessageFormat.format("{0}:{1} commit cache cleared", repositoryName, branch));
}
}

/**
Expand Down Expand Up @@ -156,49 +167,54 @@ public List<RepositoryCommit> getCommits(String repositoryName, Repository repos
if (cacheDays > 0 && (sinceDate.getTime() >= cacheCutoffDate.getTime())) {
// request fits within the cache window
String repoKey = repositoryName.toLowerCase();
if (!cache.containsKey(repoKey)) {
cache.put(repoKey, new ObjectCache<List<RepositoryCommit>>());
}

ObjectCache<List<RepositoryCommit>> repoCache = cache.get(repoKey);
String branchKey = branch.toLowerCase();

RevCommit tip = JGitUtils.getCommit(repository, branch);
Date tipDate = JGitUtils.getCommitDate(tip);

List<RepositoryCommit> commits;
if (!repoCache.hasCurrent(branchKey, tipDate)) {
commits = repoCache.getObject(branchKey);
if (ArrayUtils.isEmpty(commits)) {
// we don't have any cached commits for this branch, reload
commits = get(repositoryName, repository, branch, cacheCutoffDate);
ObjectCache<List<RepositoryCommit>> repoCache;
synchronized (cache) {
repoCache = cache.get(repoKey);
if (repoCache == null) {
repoCache = new ObjectCache<>();
cache.put(repoKey, repoCache);
}
}
synchronized (repoCache) {
List<RepositoryCommit> commits;
if (!repoCache.hasCurrent(branchKey, tipDate)) {
commits = repoCache.getObject(branchKey);
if (ArrayUtils.isEmpty(commits)) {
// we don't have any cached commits for this branch, reload
commits = get(repositoryName, repository, branch, cacheCutoffDate);
repoCache.updateObject(branchKey, tipDate, commits);
logger.debug(MessageFormat.format("parsed {0} commits from {1}:{2} since {3,date,yyyy-MM-dd} in {4} msecs", commits.size(),
repositoryName, branch, cacheCutoffDate, TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start)));
} else {
// incrementally update cache since the last cached commit
ObjectId sinceCommit = commits.get(0).getId();
List<RepositoryCommit> incremental = get(repositoryName, repository, branch, sinceCommit);
logger.info(MessageFormat.format("incrementally added {0} commits to cache for {1}:{2} in {3} msecs", incremental.size(),
repositoryName, branch, TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start)));
incremental.addAll(commits);
repoCache.updateObject(branchKey, tipDate, incremental);
commits = incremental;
}
} else {
// cache is current
commits = repoCache.getObject(branchKey);
// evict older commits outside the cache window
commits = reduce(commits, cacheCutoffDate);
// update cache
repoCache.updateObject(branchKey, tipDate, commits);
logger.debug(MessageFormat.format("parsed {0} commits from {1}:{2} since {3,date,yyyy-MM-dd} in {4} msecs",
commits.size(), repositoryName, branch, cacheCutoffDate, TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start)));
}
if (sinceDate.equals(cacheCutoffDate)) {
// Mustn't hand out the cached list; that's not thread-safe
list = new ArrayList<>(commits);
} else {
// incrementally update cache since the last cached commit
ObjectId sinceCommit = commits.get(0).getId();
List<RepositoryCommit> incremental = get(repositoryName, repository, branch, sinceCommit);
logger.info(MessageFormat.format("incrementally added {0} commits to cache for {1}:{2} in {3} msecs",
incremental.size(), repositoryName, branch, TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start)));
incremental.addAll(commits);
repoCache.updateObject(branchKey, tipDate, incremental);
commits = incremental;
// reduce the commits to those since the specified date
list = reduce(commits, sinceDate);
}
} else {
// cache is current
commits = repoCache.getObject(branchKey);
// evict older commits outside the cache window
commits = reduce(commits, cacheCutoffDate);
// update cache
repoCache.updateObject(branchKey, tipDate, commits);
}

if (sinceDate.equals(cacheCutoffDate)) {
list = commits;
} else {
// reduce the commits to those since the specified date
list = reduce(commits, sinceDate);
}
logger.debug(MessageFormat.format("retrieved {0} commits from cache of {1}:{2} since {3,date,yyyy-MM-dd} in {4} msecs",
list.size(), repositoryName, branch, sinceDate, TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start)));
Expand All @@ -222,8 +238,9 @@ public List<RepositoryCommit> getCommits(String repositoryName, Repository repos
*/
protected List<RepositoryCommit> get(String repositoryName, Repository repository, String branch, Date sinceDate) {
Map<ObjectId, List<RefModel>> allRefs = JGitUtils.getAllRefs(repository, false);
List<RepositoryCommit> commits = new ArrayList<RepositoryCommit>();
for (RevCommit commit : JGitUtils.getRevLog(repository, branch, sinceDate)) {
List<RevCommit> revLog = JGitUtils.getRevLog(repository, branch, sinceDate);
List<RepositoryCommit> commits = new ArrayList<RepositoryCommit>(revLog.size());
for (RevCommit commit : revLog) {
RepositoryCommit commitModel = new RepositoryCommit(repositoryName, branch, commit);
List<RefModel> commitRefs = allRefs.get(commitModel.getId());
commitModel.setRefs(commitRefs);
Expand All @@ -243,8 +260,9 @@ protected List<RepositoryCommit> get(String repositoryName, Repository repositor
*/
protected List<RepositoryCommit> get(String repositoryName, Repository repository, String branch, ObjectId sinceCommit) {
Map<ObjectId, List<RefModel>> allRefs = JGitUtils.getAllRefs(repository, false);
List<RepositoryCommit> commits = new ArrayList<RepositoryCommit>();
for (RevCommit commit : JGitUtils.getRevLog(repository, sinceCommit.getName(), branch)) {
List<RevCommit> revLog = JGitUtils.getRevLog(repository, sinceCommit.getName(), branch);
List<RepositoryCommit> commits = new ArrayList<RepositoryCommit>(revLog.size());
for (RevCommit commit : revLog) {
RepositoryCommit commitModel = new RepositoryCommit(repositoryName, branch, commit);
List<RefModel> commitRefs = allRefs.get(commitModel.getId());
commitModel.setRefs(commitRefs);
Expand All @@ -261,7 +279,7 @@ protected List<RepositoryCommit> get(String repositoryName, Repository repositor
* @return a list of commits
*/
protected List<RepositoryCommit> reduce(List<RepositoryCommit> commits, Date sinceDate) {
List<RepositoryCommit> filtered = new ArrayList<RepositoryCommit>();
List<RepositoryCommit> filtered = new ArrayList<RepositoryCommit>(commits.size());
for (RepositoryCommit commit : commits) {
if (commit.getCommitDate().compareTo(sinceDate) >= 0) {
filtered.add(commit);
Expand Down
32 changes: 32 additions & 0 deletions ...ain/java/com/googlesource/gerrit/plugins/gitblit/auth/GerritGitBlitRepositoryManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,33 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.gitblit.auth;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.gitblit.Keys;
import com.gitblit.manager.IPluginManager;
import com.gitblit.manager.IRuntimeManager;
import com.gitblit.manager.IUserManager;
import com.gitblit.manager.RepositoryManager;
import com.gitblit.models.RepositoryModel;
import com.gitblit.models.UserModel;
import com.gitblit.utils.CommitCache;
import com.google.inject.Inject;
import com.google.inject.Singleton;

@Singleton
public class GerritGitBlitRepositoryManager extends RepositoryManager {

private static final Logger log = LoggerFactory.getLogger(GerritGitBlitRepositoryManager.class);

private final IRuntimeManager runtimeManager;

private final IUserManager userManager;

@Inject
public GerritGitBlitRepositoryManager(final IRuntimeManager runtimeManager, final IPluginManager pluginManager, final IUserManager userManager) {
super(runtimeManager, pluginManager, userManager);
this.runtimeManager = runtimeManager;
this.userManager = userManager;
}

Expand Down Expand Up @@ -83,4 +93,26 @@ public boolean deleteRepositoryModel(RepositoryModel model) {
// Just to be sure. Shouldn't be called anyway.
return true;
}

@Override
protected void configureCommitCache() {
int daysToCache = runtimeManager.getSettings().getInteger(Keys.web.activityCacheDays, 14);
if (daysToCache <= 0) {
log.info("Commit cache is disabled");
return;
}
CommitCache.instance().setCacheDays(daysToCache);
// Run this potentially long-running operation in the background
Thread loader = new Thread() {
@Override
public void run() {
log.info("Starting to populate commit cache in background");
GerritGitBlitRepositoryManager.super.configureCommitCache();
log.info("Done populating commit cache in background");
}
};
loader.setName("CommitCacheLoader");
loader.setDaemon(true);
loader.start();
}
}

0 comments on commit db57941

Please sign in to comment.