From 26862bb834b90722d6290181f8776711d8a7d10d Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 3 Jun 2016 01:04:51 +0200 Subject: [PATCH 1/7] Extract work queue to allow reusing it Change-Id: I28f7800030a3b9db48e315061509af0746feffcc Signed-off-by: Matthias Sohn --- .../jgit/lib/BatchingProgressMonitor.java | 49 +-------- .../src/org/eclipse/jgit/lib/WorkQueue.java | 99 +++++++++++++++++++ 2 files changed, 102 insertions(+), 46 deletions(-) create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/lib/WorkQueue.java diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchingProgressMonitor.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchingProgressMonitor.java index 39856c0c9..a3859abb2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchingProgressMonitor.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchingProgressMonitor.java @@ -43,55 +43,11 @@ package org.eclipse.jgit.lib; -import java.util.concurrent.Executors; import java.util.concurrent.Future; -import java.util.concurrent.ScheduledThreadPoolExecutor; -import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; /** ProgressMonitor that batches update events. */ public abstract class BatchingProgressMonitor implements ProgressMonitor { - private static final ScheduledThreadPoolExecutor alarmQueue; - - static final Object alarmQueueKiller; - - static { - // To support garbage collection, start our thread but - // swap out the thread factory. When our class is GC'd - // the alarmQueueKiller will finalize and ask the executor - // to shutdown, ending the worker. - // - int threads = 1; - alarmQueue = new ScheduledThreadPoolExecutor(threads, - new ThreadFactory() { - private final ThreadFactory baseFactory = Executors - .defaultThreadFactory(); - - public Thread newThread(Runnable taskBody) { - Thread thr = baseFactory.newThread(taskBody); - thr.setName("JGit-AlarmQueue"); //$NON-NLS-1$ - thr.setDaemon(true); - return thr; - } - }); - alarmQueue.setContinueExistingPeriodicTasksAfterShutdownPolicy(false); - alarmQueue.setExecuteExistingDelayedTasksAfterShutdownPolicy(false); - alarmQueue.prestartAllCoreThreads(); - - // Now that the threads are running, its critical to swap out - // our own thread factory for one that isn't in the ClassLoader. - // This allows the class to GC. - // - alarmQueue.setThreadFactory(Executors.defaultThreadFactory()); - - alarmQueueKiller = new Object() { - @Override - protected void finalize() { - alarmQueue.shutdownNow(); - } - }; - } - private long delayStartTime; private TimeUnit delayStartUnit = TimeUnit.MILLISECONDS; @@ -219,7 +175,7 @@ private static class Task implements Runnable { void delay(long time, TimeUnit unit) { display = false; - timerFuture = alarmQueue.schedule(this, time, unit); + timerFuture = WorkQueue.getExecutor().schedule(this, time, unit); } public void run() { @@ -254,7 +210,8 @@ void update(BatchingProgressMonitor pm, int completed) { private void restartTimer() { display = false; - timerFuture = alarmQueue.schedule(this, 1, TimeUnit.SECONDS); + timerFuture = WorkQueue.getExecutor().schedule(this, 1, + TimeUnit.SECONDS); } void end(BatchingProgressMonitor pm) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/WorkQueue.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/WorkQueue.java new file mode 100644 index 000000000..9735d19e5 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/WorkQueue.java @@ -0,0 +1,99 @@ +/* + * Copyright (C) 2008-2016, Google Inc. + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.lib; + +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.ThreadFactory; + +/** + * Simple work queue to run tasks in the background + */ +class WorkQueue { + private static final ScheduledThreadPoolExecutor executor; + + static final Object executorKiller; + + static { + // To support garbage collection, start our thread but + // swap out the thread factory. When our class is GC'd + // the executorKiller will finalize and ask the executor + // to shutdown, ending the worker. + // + int threads = 1; + executor = new ScheduledThreadPoolExecutor(threads, + new ThreadFactory() { + private final ThreadFactory baseFactory = Executors + .defaultThreadFactory(); + + public Thread newThread(Runnable taskBody) { + Thread thr = baseFactory.newThread(taskBody); + thr.setName("JGit-WorkQueue"); //$NON-NLS-1$ + thr.setDaemon(true); + return thr; + } + }); + executor.setRemoveOnCancelPolicy(true); + executor.setContinueExistingPeriodicTasksAfterShutdownPolicy(false); + executor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false); + executor.prestartAllCoreThreads(); + + // Now that the threads are running, its critical to swap out + // our own thread factory for one that isn't in the ClassLoader. + // This allows the class to GC. + // + executor.setThreadFactory(Executors.defaultThreadFactory()); + + executorKiller = new Object() { + @Override + protected void finalize() { + executor.shutdownNow(); + } + }; + } + + static ScheduledThreadPoolExecutor getExecutor() { + return executor; + } +} From 93486301b3101f2fd403991ad3cd487903f62a70 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sun, 19 Jun 2016 12:45:37 +0200 Subject: [PATCH 2/7] Allow using JDK 7 bootclasspath when compiling JGit using Java 8 When compiling jgit using Java 8 set system property JDK_HOME to JAVA_HOME path of JDK7 installation to compile against JDK 7 class libraries. Otherwise jgit may hit runtime exceptions when running on Java 7 (e.g. return type of ConcurrentHashMap.keySet() in JDK 8 class library doesn't exist in JDK 7). Example: $ mvn clean install -DJDK_HOME=/.../jdk1.7.0_80.jdk/Contents/Home Bug: 496262 Change-Id: Ib6abe5e544e0492e08b342e1a34b182caf25f94f Signed-off-by: Matthias Sohn --- pom.xml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 17995b62b..81c04cea3 100644 --- a/pom.xml +++ b/pom.xml @@ -188,9 +188,10 @@ UTF-8 UTF-8 - yyyyMMddHHmm ${project.build.directory}/META-INF/MANIFEST.MF + + ${JAVA_HOME} 4.2.0.201601211800-r 0.1.53 @@ -264,6 +265,9 @@ UTF-8 1.7 1.7 + + ${JDK_HOME}${file.separator}jre${file.separator}lib${file.separator}rt.jar${path.separator}${JDK_HOME}${file.separator}jre${file.separator}lib${file.separator}jsse.jar${path.separator}${JDK_HOME}${file.separator}jre${file.separator}lib${file.separator}jce.jar + From 80cd8554435bdc311d9303677f0796554e5a2dbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Ar=C3=A8s?= Date: Wed, 15 Jun 2016 14:40:05 -0400 Subject: [PATCH 3/7] Config load should not fail on unsupported or nonexistent include path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1f86350 added initial support for include.path. Relative path and path with tilde are not yet supported but config load was failing if one of those 2 unsupported options was encountered. Another problem was that config load was failing if the include.path file did not exist. Change the behavior to be consistent with native git. Ignore unsupported or nonexistent include.path. Bug: 495505 Bug: 496732 Change-Id: I7285d0e7abb6389ba6983e9c46021bea4344af68 Signed-off-by: Hugo Arès --- .../tst/org/eclipse/jgit/lib/ConfigTest.java | 27 ++++++++++++++++--- .../eclipse/jgit/internal/JGitText.properties | 1 - .../org/eclipse/jgit/internal/JGitText.java | 1 - .../src/org/eclipse/jgit/lib/Config.java | 11 ++++++-- 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java index 6d07d34d3..c14c6d7f2 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java @@ -782,10 +782,31 @@ public void testIncludeEmptyValue() throws ConfigInvalidException { @Test public void testIncludeValuePathNotFound() throws ConfigInvalidException { + // we do not expect an exception, included path not found are ignored String notFound = "/not/found"; - expectedEx.expect(ConfigInvalidException.class); - expectedEx.expectMessage(notFound); - parse("[include]\npath=" + notFound + "\n"); + Config parsed = parse("[include]\npath=" + notFound + "\n"); + assertEquals(1, parsed.getSections().size()); + assertEquals(notFound, parsed.getString("include", null, "path")); + } + + @Test + public void testIncludeValuePathWithTilde() throws ConfigInvalidException { + // we do not expect an exception, included path not supported are + // ignored + String notSupported = "~/someFile"; + Config parsed = parse("[include]\npath=" + notSupported + "\n"); + assertEquals(1, parsed.getSections().size()); + assertEquals(notSupported, parsed.getString("include", null, "path")); + } + + @Test + public void testIncludeValuePathRelative() throws ConfigInvalidException { + // we do not expect an exception, included path not supported are + // ignored + String notSupported = "someRelativeFile"; + Config parsed = parse("[include]\npath=" + notSupported + "\n"); + assertEquals(1, parsed.getSections().size()); + assertEquals(notSupported, parsed.getString("include", null, "path")); } @Test diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index b71e48b52..ead835986 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -339,7 +339,6 @@ invalidId0=Invalid id invalidIdLength=Invalid id length {0}; should be {1} invalidIgnoreParamSubmodule=Found invalid ignore param for submodule {0}. invalidIgnoreRule=Exception caught while parsing ignore rule ''{0}''. -invalidIncludedPathInConfigFile=Invalid included path in config file: {0} invalidIntegerValue=Invalid integer value: {0}.{1}={2} invalidKey=Invalid key: {0} invalidLineInConfigFile=Invalid line in config file diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 7c101780a..a113cd28f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -398,7 +398,6 @@ public static JGitText get() { /***/ public String invalidIdLength; /***/ public String invalidIgnoreParamSubmodule; /***/ public String invalidIgnoreRule; - /***/ public String invalidIncludedPathInConfigFile; /***/ public String invalidIntegerValue; /***/ public String invalidKey; /***/ public String invalidLineInConfigFile; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java index b8eba3acc..6592823da 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java @@ -52,6 +52,7 @@ package org.eclipse.jgit.lib; import java.io.File; +import java.io.FileNotFoundException; import java.io.IOException; import java.text.MessageFormat; import java.util.ArrayList; @@ -1127,9 +1128,15 @@ private void addIncludedConfig(final List newEntries, decoded = RawParseUtils.decode(bytes); } newEntries.addAll(fromTextRecurse(decoded, depth + 1)); + } catch (FileNotFoundException fnfe) { + if (path.exists()) { + throw new ConfigInvalidException(MessageFormat + .format(JGitText.get().cannotReadFile, path), fnfe); + } } catch (IOException ioe) { - throw new ConfigInvalidException(MessageFormat.format( - JGitText.get().invalidIncludedPathInConfigFile, path)); + throw new ConfigInvalidException( + MessageFormat.format(JGitText.get().cannotReadFile, path), + ioe); } } From 7a6582c09c4ded97444e31c690293fc5df69114b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Ar=C3=A8s?= Date: Wed, 15 Jun 2016 15:14:06 -0400 Subject: [PATCH 4/7] Align include.path max depth with native git MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I52f059816e1d94b2d60d096e3013bf4095cd0fc4 Signed-off-by: Hugo Arès --- org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java index 6592823da..b32e5d166 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java @@ -80,7 +80,7 @@ public class Config { private static final long KiB = 1024; private static final long MiB = 1024 * KiB; private static final long GiB = 1024 * MiB; - private static final int MAX_DEPTH = 999; + private static final int MAX_DEPTH = 10; /** the change listeners */ private final ListenerList listeners = new ListenerList(); From c1ca9cc80004b4eaa90f18a8fea251d565304e56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Ar=C3=A8s?= Date: Fri, 3 Jun 2016 09:11:58 -0400 Subject: [PATCH 5/7] Add method to read time unit from config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Time units supported: -milliseconds (1 ms, 2 milliseconds) -seconds (1 s, 1 sec, 1 second, 2 seconds) -minutes (1 m, 1 min, 1 minute, 2 minutes) -hours (1 h, 1 hr, 1 hour, 2 hours) -days (1 d, 1 day, 2 days) -weeks (1 w, 1 week, 2 weeks) -months (1 mon, 1 month, 2 months) -years (1 y, 1 year, 2 years) This functionality is implemented in Gerrit ConfigUtil class. Add it to JGit so it can eventually be remove from Gerrit. Change-Id: I2d6564ff656b6ab9424a9360624061c94fd5f413 Signed-off-by: Hugo Arès Signed-off-by: Matthias Sohn --- .../tst/org/eclipse/jgit/lib/ConfigTest.java | 95 ++++++++++++++ org.eclipse.jgit/.settings/.api_filters | 8 ++ .../eclipse/jgit/internal/JGitText.properties | 2 + .../org/eclipse/jgit/internal/JGitText.java | 2 + .../src/org/eclipse/jgit/lib/Config.java | 120 ++++++++++++++++++ 5 files changed, 227 insertions(+) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java index c14c6d7f2..aebbafeff 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java @@ -48,6 +48,11 @@ package org.eclipse.jgit.lib; +import static java.util.concurrent.TimeUnit.DAYS; +import static java.util.concurrent.TimeUnit.HOURS; +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.concurrent.TimeUnit.MINUTES; +import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -64,6 +69,7 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.Set; +import java.util.concurrent.TimeUnit; import org.eclipse.jgit.api.MergeCommand.FastForwardMode; import org.eclipse.jgit.errors.ConfigInvalidException; @@ -870,4 +876,93 @@ private static Config parse(final String content, Config baseConfig) c.fromText(content); return c; } + + @Test + public void testTimeUnit() throws ConfigInvalidException { + assertEquals(0, parseTime("0", MILLISECONDS)); + assertEquals(2, parseTime("2ms", MILLISECONDS)); + assertEquals(200, parseTime("200 milliseconds", MILLISECONDS)); + + assertEquals(0, parseTime("0s", SECONDS)); + assertEquals(2, parseTime("2s", SECONDS)); + assertEquals(231, parseTime("231sec", SECONDS)); + assertEquals(1, parseTime("1second", SECONDS)); + assertEquals(300, parseTime("300 seconds", SECONDS)); + + assertEquals(2, parseTime("2m", MINUTES)); + assertEquals(2, parseTime("2min", MINUTES)); + assertEquals(1, parseTime("1 minute", MINUTES)); + assertEquals(10, parseTime("10 minutes", MINUTES)); + + assertEquals(5, parseTime("5h", HOURS)); + assertEquals(5, parseTime("5hr", HOURS)); + assertEquals(1, parseTime("1hour", HOURS)); + assertEquals(48, parseTime("48hours", HOURS)); + + assertEquals(5, parseTime("5 h", HOURS)); + assertEquals(5, parseTime("5 hr", HOURS)); + assertEquals(1, parseTime("1 hour", HOURS)); + assertEquals(48, parseTime("48 hours", HOURS)); + assertEquals(48, parseTime("48 \t \r hours", HOURS)); + + assertEquals(4, parseTime("4d", DAYS)); + assertEquals(1, parseTime("1day", DAYS)); + assertEquals(14, parseTime("14days", DAYS)); + + assertEquals(7, parseTime("1w", DAYS)); + assertEquals(7, parseTime("1week", DAYS)); + assertEquals(14, parseTime("2w", DAYS)); + assertEquals(14, parseTime("2weeks", DAYS)); + + assertEquals(30, parseTime("1mon", DAYS)); + assertEquals(30, parseTime("1month", DAYS)); + assertEquals(60, parseTime("2mon", DAYS)); + assertEquals(60, parseTime("2months", DAYS)); + + assertEquals(365, parseTime("1y", DAYS)); + assertEquals(365, parseTime("1year", DAYS)); + assertEquals(365 * 2, parseTime("2years", DAYS)); + } + + private long parseTime(String value, TimeUnit unit) + throws ConfigInvalidException { + Config c = parse("[a]\na=" + value + "\n"); + return c.getTimeUnit("a", null, "a", 0, unit); + } + + @Test + public void testTimeUnitDefaultValue() throws ConfigInvalidException { + // value not present + assertEquals(20, parse("[a]\na=0\n").getTimeUnit("a", null, "b", 20, + MILLISECONDS)); + // value is empty + assertEquals(20, parse("[a]\na=\" \"\n").getTimeUnit("a", null, "a", 20, + MILLISECONDS)); + + // value is not numeric + assertEquals(20, parse("[a]\na=test\n").getTimeUnit("a", null, "a", 20, + MILLISECONDS)); + } + + @Test + public void testTimeUnitInvalid() throws ConfigInvalidException { + expectedEx.expect(IllegalArgumentException.class); + expectedEx + .expectMessage("Invalid time unit value: a.a=1 monttthhh"); + parseTime("1 monttthhh", DAYS); + } + + @Test + public void testTimeUnitInvalidWithSection() throws ConfigInvalidException { + Config c = parse("[a \"b\"]\na=1 monttthhh\n"); + expectedEx.expect(IllegalArgumentException.class); + expectedEx.expectMessage("Invalid time unit value: a.b.a=1 monttthhh"); + c.getTimeUnit("a", "b", "a", 0, DAYS); + } + + @Test + public void testTimeUnitNegative() throws ConfigInvalidException { + expectedEx.expect(IllegalArgumentException.class); + parseTime("-1", MILLISECONDS); + } } diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index c0dbc779f..bacfb336a 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -1,5 +1,13 @@ + + + + + + + + diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index ead835986..83a72f0d5 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -360,6 +360,8 @@ invalidShallowObject=invalid shallow object {0}, expected commit invalidStageForPath=Invalid stage {0} for path {1} invalidTagOption=Invalid tag option: {0} invalidTimeout=Invalid timeout: {0} +invalidTimeUnitValue2=Invalid time unit value: {0}.{1}={2} +invalidTimeUnitValue3=Invalid time unit value: {0}.{1}.{2}={3} invalidURL=Invalid URL {0} invalidWildcards=Invalid wildcards {0} invalidRefSpec=Invalid refspec {0} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index a113cd28f..99b18b72c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -418,6 +418,8 @@ public static JGitText get() { /***/ public String invalidStageForPath; /***/ public String invalidTagOption; /***/ public String invalidTimeout; + /***/ public String invalidTimeUnitValue2; + /***/ public String invalidTimeUnitValue3; /***/ public String invalidURL; /***/ public String invalidWildcards; /***/ public String invalidRefSpec; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java index b32e5d166..1e1d14737 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java @@ -59,7 +59,10 @@ import java.util.Collections; import java.util.List; import java.util.Set; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.events.ConfigChangedEvent; @@ -490,6 +493,123 @@ public String[] getStringList(final String section, String subsection, return res; } + /** + * Parse a numerical time unit, such as "1 minute", from the configuration. + * + * @param section + * section the key is in. + * @param subsection + * subsection the key is in, or null if not in a subsection. + * @param name + * the key name. + * @param defaultValue + * default value to return if no value was present. + * @param wantUnit + * the units of {@code defaultValue} and the return value, as + * well as the units to assume if the value does not contain an + * indication of the units. + * @return the value, or {@code defaultValue} if not set, expressed in + * {@code units}. + * @since 4.4 + */ + public long getTimeUnit(String section, String subsection, String name, + long defaultValue, TimeUnit wantUnit) { + String valueString = getString(section, subsection, name); + + if (valueString == null) { + return defaultValue; + } + + String s = valueString.trim(); + if (s.length() == 0) { + return defaultValue; + } + + if (s.startsWith("-")/* negative */) { //$NON-NLS-1$ + throw notTimeUnit(section, subsection, name, valueString); + } + + Matcher m = Pattern.compile("^(0|[1-9][0-9]*)\\s*(.*)$") //$NON-NLS-1$ + .matcher(valueString); + if (!m.matches()) { + return defaultValue; + } + + String digits = m.group(1); + String unitName = m.group(2).trim(); + + TimeUnit inputUnit; + int inputMul; + + if (unitName.isEmpty()) { + inputUnit = wantUnit; + inputMul = 1; + + } else if (match(unitName, "ms", "milliseconds")) { //$NON-NLS-1$ //$NON-NLS-2$ + inputUnit = TimeUnit.MILLISECONDS; + inputMul = 1; + + } else if (match(unitName, "s", "sec", "second", "seconds")) { //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ + inputUnit = TimeUnit.SECONDS; + inputMul = 1; + + } else if (match(unitName, "m", "min", "minute", "minutes")) { //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ + inputUnit = TimeUnit.MINUTES; + inputMul = 1; + + } else if (match(unitName, "h", "hr", "hour", "hours")) { //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ + inputUnit = TimeUnit.HOURS; + inputMul = 1; + + } else if (match(unitName, "d", "day", "days")) { //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + inputUnit = TimeUnit.DAYS; + inputMul = 1; + + } else if (match(unitName, "w", "week", "weeks")) { //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + inputUnit = TimeUnit.DAYS; + inputMul = 7; + + } else if (match(unitName, "mon", "month", "months")) { //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + inputUnit = TimeUnit.DAYS; + inputMul = 30; + + } else if (match(unitName, "y", "year", "years")) { //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + inputUnit = TimeUnit.DAYS; + inputMul = 365; + + } else { + throw notTimeUnit(section, subsection, name, valueString); + } + + try { + return wantUnit.convert(Long.parseLong(digits) * inputMul, + inputUnit); + } catch (NumberFormatException nfe) { + throw notTimeUnit(section, subsection, unitName, valueString); + } + } + + private static boolean match(final String a, final String... cases) { + for (final String b : cases) { + if (b != null && b.equalsIgnoreCase(a)) { + return true; + } + } + return false; + } + + private IllegalArgumentException notTimeUnit(String section, + String subsection, String name, String valueString) { + if (subsection != null) { + return new IllegalArgumentException( + MessageFormat.format(JGitText.get().invalidTimeUnitValue3, + section, subsection, name, valueString)); + } + return new IllegalArgumentException( + MessageFormat.format(JGitText.get().invalidTimeUnitValue2, + section, name, valueString)); + } + /** * @param section * section to search for. From 7ffe547da79bf26301c5e5a0665d19a233b44818 Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Mon, 30 May 2016 16:09:58 +0200 Subject: [PATCH 6/7] Time based eviction strategy for repository cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Repository.close() decrements the useCount to 0 currently the cache immediately evicts the repository from WindowCache and RepositoryCache. This leads to I/O overhead on busy repositories because pack files and references are inserted and deleted from the cache frequently. This commit defers the eviction of a repository from the caches until last use of the repository is older than time to live. The eviction is handled by a background task running periodically. Add two new configuration parameters: * core.repositoryCacheExpireAfter: cache entries are evicted if the cache entry wasn't accessed longer than this time in milliseconds * core.repositoryCacheCleanupDelay: defines the interval in milliseconds for running a background task evicting expired cache entries. If set to -1 the delay is set to min(repositoryCacheExpireAfter, 10 minutes). If set to 0 the time based eviction is switched off and no background task is started. If time based eviction is switched off the JVM can still evict cache entries if heap memory is running low. Change-Id: I4a0214ad8b4a193985dda6a0ade63b70bdb948d7 Also-by: Matthias Sohn Also-by: Hugo Arès Also-by: Sasa Zivkov --- .../jgit/lib/RepositoryCacheConfigTest.java | 107 ++++++++++++ .../eclipse/jgit/lib/RepositoryCacheTest.java | 72 ++++++++- .../src/org/eclipse/jgit/lib/Repository.java | 10 +- .../org/eclipse/jgit/lib/RepositoryCache.java | 91 ++++++++++- .../jgit/lib/RepositoryCacheConfig.java | 152 ++++++++++++++++++ 5 files changed, 419 insertions(+), 13 deletions(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheConfigTest.java create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCacheConfig.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheConfigTest.java new file mode 100644 index 000000000..52cc9fb03 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheConfigTest.java @@ -0,0 +1,107 @@ +/* + * Copyright (C) 2016 Ericsson + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.lib; + +import static org.eclipse.jgit.lib.RepositoryCacheConfig.AUTO_CLEANUP_DELAY; +import static org.eclipse.jgit.lib.RepositoryCacheConfig.NO_CLEANUP; +import static org.junit.Assert.assertEquals; + +import java.util.concurrent.TimeUnit; + +import org.eclipse.jgit.errors.ConfigInvalidException; +import org.junit.Before; +import org.junit.Test; + +public class RepositoryCacheConfigTest { + + private RepositoryCacheConfig config; + + @Before + public void setUp() { + config = new RepositoryCacheConfig(); + } + + @Test + public void testDefaultValues() { + assertEquals(TimeUnit.HOURS.toMillis(1), config.getExpireAfter()); + assertEquals(config.getExpireAfter() / 10, config.getCleanupDelay()); + } + + @Test + public void testCleanupDelay() { + config.setCleanupDelay(TimeUnit.HOURS.toMillis(1)); + assertEquals(TimeUnit.HOURS.toMillis(1), config.getCleanupDelay()); + } + + @Test + public void testAutoCleanupDelay() { + config.setExpireAfter(TimeUnit.MINUTES.toMillis(20)); + config.setCleanupDelay(AUTO_CLEANUP_DELAY); + assertEquals(TimeUnit.MINUTES.toMillis(20), config.getExpireAfter()); + assertEquals(config.getExpireAfter() / 10, config.getCleanupDelay()); + } + + @Test + public void testAutoCleanupDelayShouldBeMax10minutes() { + config.setExpireAfter(TimeUnit.HOURS.toMillis(10)); + assertEquals(TimeUnit.HOURS.toMillis(10), config.getExpireAfter()); + assertEquals(TimeUnit.MINUTES.toMillis(10), config.getCleanupDelay()); + } + + @Test + public void testDisabledCleanupDelay() { + config.setCleanupDelay(NO_CLEANUP); + assertEquals(NO_CLEANUP, config.getCleanupDelay()); + } + + @Test + public void testFromConfig() throws ConfigInvalidException { + Config otherConfig = new Config(); + otherConfig.fromText("[core]\nrepositoryCacheExpireAfter=1000\n" + + "repositoryCacheCleanupDelay=500"); + config.fromConfig(otherConfig); + assertEquals(1000, config.getExpireAfter()); + assertEquals(500, config.getCleanupDelay()); + } +} diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheTest.java index a1cec2d91..6bea32012 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheTest.java @@ -195,17 +195,81 @@ public void testRepositoryUsageCountWithRegisteredRepository() { assertEquals(0, ((Repository) db).useCnt.get()); } - public void testRepositoryUnregisteringWhenClosing() throws Exception { + @Test + public void testRepositoryNotUnregisteringWhenClosing() throws Exception { FileKey loc = FileKey.exact(db.getDirectory(), db.getFS()); Repository d2 = RepositoryCache.open(loc); assertEquals(1, d2.useCnt.get()); assertThat(RepositoryCache.getRegisteredKeys(), hasItem(FileKey.exact(db.getDirectory(), db.getFS()))); assertEquals(1, RepositoryCache.getRegisteredKeys().size()); - d2.close(); - assertEquals(0, d2.useCnt.get()); - assertEquals(0, RepositoryCache.getRegisteredKeys().size()); + assertEquals(1, RepositoryCache.getRegisteredKeys().size()); + assertTrue(RepositoryCache.isCached(d2)); + } + + @Test + public void testRepositoryUnregisteringWhenExpired() throws Exception { + Repository repoA = createBareRepository(); + Repository repoB = createBareRepository(); + Repository repoC = createBareRepository(); + RepositoryCache.register(repoA); + RepositoryCache.register(repoB); + RepositoryCache.register(repoC); + + assertEquals(3, RepositoryCache.getRegisteredKeys().size()); + assertTrue(RepositoryCache.isCached(repoA)); + assertTrue(RepositoryCache.isCached(repoB)); + assertTrue(RepositoryCache.isCached(repoC)); + + // fake that repoA was closed more than 1 hour ago (default expiration + // time) + repoA.close(); + repoA.closedAt.set(System.currentTimeMillis() - 65 * 60 * 1000); + // close repoB but this one will not be expired + repoB.close(); + + assertEquals(3, RepositoryCache.getRegisteredKeys().size()); + assertTrue(RepositoryCache.isCached(repoA)); + assertTrue(RepositoryCache.isCached(repoB)); + assertTrue(RepositoryCache.isCached(repoC)); + + RepositoryCache.clearExpired(); + + assertEquals(2, RepositoryCache.getRegisteredKeys().size()); + assertFalse(RepositoryCache.isCached(repoA)); + assertTrue(RepositoryCache.isCached(repoB)); + assertTrue(RepositoryCache.isCached(repoC)); + } + + @Test + public void testReconfigure() throws InterruptedException { + RepositoryCache.register(db); + assertTrue(RepositoryCache.isCached(db)); + db.close(); + assertTrue(RepositoryCache.isCached(db)); + + // Actually, we would only need to validate that + // WorkQueue.getExecutor().scheduleWithFixedDelay is called with proper + // values but since we do not have a mock library, we test + // reconfiguration from a black box perspective. I.e. reconfigure + // expireAfter and cleanupDelay to 1 ms and wait until the Repository + // is evicted to prove that reconfiguration worked. + RepositoryCacheConfig config = new RepositoryCacheConfig(); + config.setExpireAfter(1); + config.setCleanupDelay(1); + config.install(); + + // Instead of using a fixed waiting time, start with small and increase: + // sleep 1, 2, 4, 8, 16, ..., 1024 ms + // This wait will time out after 2048 ms + for (int i = 0; i <= 10; i++) { + Thread.sleep(1 << i); + if (!RepositoryCache.isCached(db)) { + return; + } + } + fail("Repository should have been evicted from cache"); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java index 5703dddf1..9711fda5c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java @@ -63,6 +63,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.annotations.Nullable; @@ -113,6 +114,8 @@ public static ListenerList getGlobalListenerList() { /** Use counter */ final AtomicInteger useCnt = new AtomicInteger(1); + final AtomicLong closedAt = new AtomicLong(); + /** Metadata directory holding the repository's critical files. */ private final File gitDir; @@ -864,8 +867,11 @@ public void incrementOpen() { /** Decrement the use count, and maybe close resources. */ public void close() { if (useCnt.decrementAndGet() == 0) { - doClose(); - RepositoryCache.unregister(this); + if (RepositoryCache.isCached(this)) { + closedAt.set(System.currentTimeMillis()); + } else { + doClose(); + } } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCache.java index 22b5fcd11..29ef08483 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCache.java @@ -52,17 +52,26 @@ import java.util.Iterator; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.internal.storage.file.FileRepository; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.RawParseUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** Cache of active {@link Repository} instances. */ public class RepositoryCache { private static final RepositoryCache cache = new RepositoryCache(); + private final static Logger LOG = LoggerFactory + .getLogger(RepositoryCache.class); + /** * Open an existing repository, reusing a cached instance if possible. *

@@ -138,10 +147,10 @@ public static void register(final Repository db) { * @param db * repository to unregister. */ - public static void close(final Repository db) { + public static void close(@NonNull final Repository db) { if (db.getDirectory() != null) { FileKey key = FileKey.exact(db.getDirectory(), db.getFS()); - cache.unregisterAndCloseRepository(key); + cache.unregisterAndCloseRepository(key, db); } } @@ -187,20 +196,69 @@ public static Collection getRegisteredKeys() { return cache.getKeys(); } + static boolean isCached(@NonNull Repository repo) { + File gitDir = repo.getDirectory(); + if (gitDir == null) { + return false; + } + FileKey key = new FileKey(gitDir, repo.getFS()); + Reference repoRef = cache.cacheMap.get(key); + return repoRef != null && repoRef.get() == repo; + } + /** Unregister all repositories from the cache. */ public static void clear() { cache.clearAll(); } + static void clearExpired() { + cache.clearAllExpired(); + } + + static void reconfigure(RepositoryCacheConfig repositoryCacheConfig) { + cache.configureEviction(repositoryCacheConfig); + } + private final ConcurrentHashMap> cacheMap; private final Lock[] openLocks; + private ScheduledFuture cleanupTask; + + private volatile long expireAfter; + private RepositoryCache() { cacheMap = new ConcurrentHashMap>(); openLocks = new Lock[4]; - for (int i = 0; i < openLocks.length; i++) + for (int i = 0; i < openLocks.length; i++) { openLocks[i] = new Lock(); + } + configureEviction(new RepositoryCacheConfig()); + } + + private void configureEviction( + RepositoryCacheConfig repositoryCacheConfig) { + expireAfter = repositoryCacheConfig.getExpireAfter(); + ScheduledThreadPoolExecutor scheduler = WorkQueue.getExecutor(); + synchronized (scheduler) { + if (cleanupTask != null) { + cleanupTask.cancel(false); + } + long delay = repositoryCacheConfig.getCleanupDelay(); + if (delay == RepositoryCacheConfig.NO_CLEANUP) { + return; + } + cleanupTask = scheduler.scheduleWithFixedDelay(new Runnable() { + @Override + public void run() { + try { + cache.clearAllExpired(); + } catch (Throwable e) { + LOG.error(e.getMessage(), e); + } + } + }, delay, delay, TimeUnit.MILLISECONDS); + } } @SuppressWarnings("resource") @@ -239,10 +297,20 @@ private Repository unregisterRepository(final Key location) { return oldRef != null ? oldRef.get() : null; } - private void unregisterAndCloseRepository(final Key location) { - Repository oldDb = unregisterRepository(location); - if (oldDb != null) { - oldDb.close(); + private boolean isExpired(Repository db) { + return db != null && db.useCnt.get() == 0 + && (System.currentTimeMillis() - db.closedAt.get() > expireAfter); + } + + private void unregisterAndCloseRepository(final Key location, + Repository db) { + synchronized (lockFor(location)) { + if (isExpired(db)) { + Repository oldDb = unregisterRepository(location); + if (oldDb != null) { + oldDb.close(); + } + } } } @@ -250,6 +318,15 @@ private Collection getKeys() { return new ArrayList(cacheMap.keySet()); } + private void clearAllExpired() { + for (Reference ref : cacheMap.values()) { + Repository db = ref.get(); + if (isExpired(db)) { + RepositoryCache.close(db); + } + } + } + private void clearAll() { for (int stage = 0; stage < 2; stage++) { for (Iterator>> i = cacheMap diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCacheConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCacheConfig.java new file mode 100644 index 000000000..428dea3e6 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCacheConfig.java @@ -0,0 +1,152 @@ +/* + * Copyright (C) 2016 Ericsson + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package org.eclipse.jgit.lib; + +import java.util.concurrent.TimeUnit; + +/** + * Configuration parameters for JVM-wide repository cache used by JGit. + * + * @since 4.4 + */ +public class RepositoryCacheConfig { + + /** + * Set cleanupDelayMillis to this value in order to switch off time-based + * cache eviction. The JVM can still expire cache entries when heap memory + * runs low. + */ + public static final long NO_CLEANUP = 0; + + /** + * Set cleanupDelayMillis to this value in order to auto-set it to minimum + * of 1/10 of expireAfterMillis and 10 minutes + */ + public static final long AUTO_CLEANUP_DELAY = -1; + + private long expireAfterMillis; + + private long cleanupDelayMillis; + + /** Create a default configuration. */ + public RepositoryCacheConfig() { + expireAfterMillis = TimeUnit.HOURS.toMillis(1); + cleanupDelayMillis = AUTO_CLEANUP_DELAY; + } + + /** + * @return the time an unused repository should expired and be evicted from + * the RepositoryCache in milliseconds. Default is 1 hour. + */ + public long getExpireAfter() { + return expireAfterMillis; + } + + /** + * @param expireAfterMillis + * the time an unused repository should expired and be evicted + * from the RepositoryCache in milliseconds. + */ + public void setExpireAfter(long expireAfterMillis) { + this.expireAfterMillis = expireAfterMillis; + } + + /** + * @return the delay between the periodic cleanup of expired repository in + * milliseconds. Default is minimum of 1/10 of expireAfterMillis + * and 10 minutes + */ + public long getCleanupDelay() { + if (cleanupDelayMillis < 0) { + return Math.min(expireAfterMillis / 10, + TimeUnit.MINUTES.toMillis(10)); + } + return cleanupDelayMillis; + } + + /** + * @param cleanupDelayMillis + * the delay between the periodic cleanup of expired repository + * in milliseconds. Set it to {@link #AUTO_CLEANUP_DELAY} to + * automatically derive cleanup delay from expireAfterMillis. + *

+ * Set it to {@link #NO_CLEANUP} in order to switch off cache + * expiration. + *

+ * If cache expiration is switched off the JVM still can evict + * cache entries when the JVM is running low on available heap + * memory. + */ + public void setCleanupDelay(long cleanupDelayMillis) { + this.cleanupDelayMillis = cleanupDelayMillis; + } + + /** + * Update properties by setting fields from the configuration. + *

+ * If a property is not defined in the configuration, then it is left + * unmodified. + * + * @param config + * configuration to read properties from. + * @return {@code this}. + */ + public RepositoryCacheConfig fromConfig(Config config) { + setExpireAfter( + config.getTimeUnit("core", null, "repositoryCacheExpireAfter", //$NON-NLS-1$//$NON-NLS-2$ + getExpireAfter(), TimeUnit.MILLISECONDS)); + setCleanupDelay( + config.getTimeUnit("core", null, "repositoryCacheCleanupDelay", //$NON-NLS-1$ //$NON-NLS-2$ + AUTO_CLEANUP_DELAY, TimeUnit.MILLISECONDS)); + return this; + } + + /** + * Install this configuration as the live settings. + *

+ * The new configuration is applied immediately. + */ + public void install() { + RepositoryCache.reconfigure(this); + } +} From ceaadf8f9835e01ca8b361885ea357d7b00536b6 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Thu, 7 Jul 2016 16:57:49 +0200 Subject: [PATCH 7/7] Log if Repository.useCnt becomes negative We observe in Gerrit 2.12 that useCnt can become negative in rare cases. Log this to help finding the bug. Change-Id: Ie91c7f9d190a5d7cf4733d4bf84124d119ca20f7 Signed-off-by: Matthias Sohn --- .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../src/org/eclipse/jgit/lib/Repository.java | 18 +++++++++++++++++- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 83a72f0d5..21fbaa491 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -185,6 +185,7 @@ corruptObjectTruncatedInMode=truncated in mode corruptObjectTruncatedInName=truncated in name corruptObjectTruncatedInObjectId=truncated in object id corruptObjectZeroId=entry points to null SHA-1 +corruptUseCnt=close() called when useCnt is already zero couldNotCheckOutBecauseOfConflicts=Could not check out because of conflicts couldNotDeleteLockFileShouldNotHappen=Could not delete lock file. Should not happen couldNotDeleteTemporaryIndexFileShouldNotHappen=Could not delete temporary index file. Should not happen diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 99b18b72c..b7ef0854c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -245,6 +245,7 @@ public static JGitText get() { /***/ public String corruptObjectTruncatedInObjectId; /***/ public String corruptObjectZeroId; /***/ public String corruptPack; + /***/ public String corruptUseCnt; /***/ public String couldNotCheckOutBecauseOfConflicts; /***/ public String couldNotDeleteLockFileShouldNotHappen; /***/ public String couldNotDeleteTemporaryIndexFileShouldNotHappen; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java index 9711fda5c..7ec24998b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java @@ -94,6 +94,8 @@ import org.eclipse.jgit.util.RawParseUtils; import org.eclipse.jgit.util.SystemReader; import org.eclipse.jgit.util.io.SafeBufferedOutputStream; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Represents a Git repository. @@ -104,6 +106,8 @@ * This class is thread-safe. */ public abstract class Repository implements AutoCloseable { + private static Logger LOG = LoggerFactory.getLogger(Repository.class); + private static final ListenerList globalListeners = new ListenerList(); /** @return the global listener list observing all events in this JVM. */ @@ -866,12 +870,24 @@ public void incrementOpen() { /** Decrement the use count, and maybe close resources. */ public void close() { - if (useCnt.decrementAndGet() == 0) { + int newCount = useCnt.decrementAndGet(); + if (newCount == 0) { if (RepositoryCache.isCached(this)) { closedAt.set(System.currentTimeMillis()); } else { doClose(); } + } else if (newCount == -1) { + // should not happen, only log when useCnt became negative to + // minimize number of log entries + LOG.warn(JGitText.get().corruptUseCnt); + if (LOG.isDebugEnabled()) { + IllegalStateException e = new IllegalStateException(); + LOG.debug("", e); //$NON-NLS-1$ + } + if (RepositoryCache.isCached(this)) { + closedAt.set(System.currentTimeMillis()); + } } }