From de3f435db5256bd5a106462dcfc0a2ccdce95450 Mon Sep 17 00:00:00 2001 From: Tom Date: Wed, 26 Oct 2016 21:37:19 +0200 Subject: [PATCH] Issue #1076: load commit cache in a background thread * Make the CommitCache fully thread-safe. It was using a ConcurrentHashMap containing lists, but then handed out these lists. It also did multiple operations on that map that as a whole should be atomic. * Use isEmpty() instead of size() == 0. * Run the loading of the commit cache in a background daemon thread --- .../gitblit/manager/RepositoryManager.java | 62 ++++++---- .../java/com/gitblit/utils/ArrayUtils.java | 2 +- .../java/com/gitblit/utils/CommitCache.java | 117 ++++++++++-------- 3 files changed, 103 insertions(+), 78 deletions(-) diff --git a/src/main/java/com/gitblit/manager/RepositoryManager.java b/src/main/java/com/gitblit/manager/RepositoryManager.java index e9bf5b84..ed26c775 100644 --- a/src/main/java/com/gitblit/manager/RepositoryManager.java +++ b/src/main/java/com/gitblit/manager/RepositoryManager.java @@ -1952,39 +1952,47 @@ public class RepositoryManager implements IRepositoryManager { } protected void configureCommitCache() { - int daysToCache = settings.getInteger(Keys.web.activityCacheDays, 14); + final int daysToCache = settings.getInteger(Keys.web.activityCacheDays, 14); if (daysToCache <= 0) { logger.info("Commit cache is disabled"); - } else { - long start = System.nanoTime(); - long repoCount = 0; - long commitCount = 0; - logger.info(MessageFormat.format("Preparing {0} day commit cache. please wait...", daysToCache)); - CommitCache.instance().setCacheDays(daysToCache); - Date cutoff = CommitCache.instance().getCutoffDate(); - for (String repositoryName : getRepositoryList()) { - RepositoryModel model = getRepositoryModel(repositoryName); - if (model != null && model.hasCommits && model.lastChange.after(cutoff)) { - repoCount++; - Repository repository = getRepository(repositoryName); - for (RefModel ref : JGitUtils.getLocalBranches(repository, true, -1)) { - if (!ref.getDate().after(cutoff)) { - // branch not recently updated - continue; - } - List commits = CommitCache.instance().getCommits(repositoryName, repository, ref.getName()); - if (commits.size() > 0) { - logger.info(MessageFormat.format(" cached {0} commits for {1}:{2}", - commits.size(), repositoryName, ref.getName())); - commitCount += commits.size(); + return; + } + logger.info(MessageFormat.format("Preparing {0} day commit cache...", daysToCache)); + CommitCache.instance().setCacheDays(daysToCache); + Thread loader = new Thread() { + @Override + public void run() { + long start = System.nanoTime(); + long repoCount = 0; + long commitCount = 0; + Date cutoff = CommitCache.instance().getCutoffDate(); + for (String repositoryName : getRepositoryList()) { + RepositoryModel model = getRepositoryModel(repositoryName); + if (model != null && model.hasCommits && model.lastChange.after(cutoff)) { + repoCount++; + Repository repository = getRepository(repositoryName); + for (RefModel ref : JGitUtils.getLocalBranches(repository, true, -1)) { + if (!ref.getDate().after(cutoff)) { + // branch not recently updated + continue; + } + List commits = CommitCache.instance().getCommits(repositoryName, repository, ref.getName()); + if (commits.size() > 0) { + logger.info(MessageFormat.format(" cached {0} commits for {1}:{2}", + commits.size(), repositoryName, ref.getName())); + commitCount += commits.size(); + } } + repository.close(); } - repository.close(); } + logger.info(MessageFormat.format("built {0} day commit cache of {1} commits across {2} repositories in {3} msecs", + daysToCache, commitCount, repoCount, TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start))); } - logger.info(MessageFormat.format("built {0} day commit cache of {1} commits across {2} repositories in {3} msecs", - daysToCache, commitCount, repoCount, TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start))); - } + }; + loader.setName("CommitCacheLoader"); + loader.setDaemon(true); + loader.start(); } protected void confirmWriteAccess() { diff --git a/src/main/java/com/gitblit/utils/ArrayUtils.java b/src/main/java/com/gitblit/utils/ArrayUtils.java index 1402ad5e..b850ccc9 100644 --- a/src/main/java/com/gitblit/utils/ArrayUtils.java +++ b/src/main/java/com/gitblit/utils/ArrayUtils.java @@ -42,7 +42,7 @@ public class ArrayUtils { } public static boolean isEmpty(Collection collection) { - return collection == null || collection.size() == 0; + return collection == null || collection.isEmpty(); } public static String toString(Collection collection) { diff --git a/src/main/java/com/gitblit/utils/CommitCache.java b/src/main/java/com/gitblit/utils/CommitCache.java index a3963f50..53b8de19 100644 --- a/src/main/java/com/gitblit/utils/CommitCache.java +++ b/src/main/java/com/gitblit/utils/CommitCache.java @@ -19,9 +19,9 @@ import java.text.MessageFormat; 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; @@ -58,7 +58,7 @@ public class CommitCache { } protected CommitCache() { - cache = new ConcurrentHashMap>>(); + cache = new HashMap<>(); } /** @@ -93,7 +93,9 @@ public class CommitCache { * */ public void clear() { - cache.clear(); + synchronized (cache) { + cache.clear(); + } } /** @@ -103,8 +105,11 @@ public class CommitCache { */ public void clear(String repositoryName) { String repoKey = repositoryName.toLowerCase(); - ObjectCache> 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)); } } @@ -117,13 +122,17 @@ public class CommitCache { */ public void clear(String repositoryName, String branch) { String repoKey = repositoryName.toLowerCase(); - ObjectCache> repoCache = cache.get(repoKey); - if (repoCache != null) { - List 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> repoCache = cache.get(repoKey); + if (repoCache != null) { + List commits = repoCache.remove(branch.toLowerCase()); + hadEntries = !ArrayUtils.isEmpty(commits); } } + if (hadEntries) { + logger.info(MessageFormat.format("{0}:{1} commit cache cleared", repositoryName, branch)); + } } /** @@ -156,49 +165,55 @@ public class CommitCache { 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>()); - } - - ObjectCache> repoCache = cache.get(repoKey); String branchKey = branch.toLowerCase(); RevCommit tip = JGitUtils.getCommit(repository, branch); Date tipDate = JGitUtils.getCommitDate(tip); - List 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 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; + ObjectCache> repoCache; + synchronized (cache) { + repoCache = cache.get(repoKey); + if (repoCache == null) { + repoCache = new ObjectCache<>(); + cache.put(repoKey, repoCache); } - } 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); } + synchronized (repoCache) { + List 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 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); + } - if (sinceDate.equals(cacheCutoffDate)) { - list = commits; - } else { - // reduce the commits to those since the specified date - list = reduce(commits, sinceDate); + if (sinceDate.equals(cacheCutoffDate)) { + // Mustn't hand out the cached list; that's not thread-safe + list = new ArrayList<>(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))); @@ -222,8 +237,9 @@ public class CommitCache { */ protected List get(String repositoryName, Repository repository, String branch, Date sinceDate) { Map> allRefs = JGitUtils.getAllRefs(repository, false); - List commits = new ArrayList(); - for (RevCommit commit : JGitUtils.getRevLog(repository, branch, sinceDate)) { + List revLog = JGitUtils.getRevLog(repository, branch, sinceDate); + List commits = new ArrayList(revLog.size()); + for (RevCommit commit : revLog) { RepositoryCommit commitModel = new RepositoryCommit(repositoryName, branch, commit); List commitRefs = allRefs.get(commitModel.getId()); commitModel.setRefs(commitRefs); @@ -243,8 +259,9 @@ public class CommitCache { */ protected List get(String repositoryName, Repository repository, String branch, ObjectId sinceCommit) { Map> allRefs = JGitUtils.getAllRefs(repository, false); - List commits = new ArrayList(); - for (RevCommit commit : JGitUtils.getRevLog(repository, sinceCommit.getName(), branch)) { + List revLog = JGitUtils.getRevLog(repository, sinceCommit.getName(), branch); + List commits = new ArrayList(revLog.size()); + for (RevCommit commit : revLog) { RepositoryCommit commitModel = new RepositoryCommit(repositoryName, branch, commit); List commitRefs = allRefs.get(commitModel.getId()); commitModel.setRefs(commitRefs); @@ -261,7 +278,7 @@ public class CommitCache { * @return a list of commits */ protected List reduce(List commits, Date sinceDate) { - List filtered = new ArrayList(); + List filtered = new ArrayList(commits.size()); for (RepositoryCommit commit : commits) { if (commit.getCommitDate().compareTo(sinceDate) >= 0) { filtered.add(commit); -- 2.39.5