From 42f0c7c9cb1c516603da6f89ae7072989bf4b984 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 30 Nov 2019 03:38:13 +0100 Subject: [PATCH 01/12] Enhance WindowCache statistics Add the following statistics - cache hit count and hit ratio - cache miss count and miss ratio - count of successful and failed loads - rate of failed loads - load, eviction and request count - average and total load time Use LongAdder instead of AtomicLong to implement counters in order to improve scalability. Optionally expose these metrics via JMX, they are registered with the platform MBean server if the config option jmx.WindowCacheStats = true in the user or system level git config. Bug: 553573 Change-Id: Ia2d5246ef69b9c2bd594a23934424bc5800774aa Signed-off-by: Matthias Sohn --- .../storage/file/WindowCacheGetTest.java | 43 ++- org.eclipse.jgit/.settings/.api_filters | 23 ++ .../internal/storage/file/WindowCache.java | 258 ++++++++++++++++-- .../org/eclipse/jgit/lib/ConfigConstants.java | 6 + .../jgit/storage/file/WindowCacheStats.java | 176 +++++++++++- .../src/org/eclipse/jgit/util/Monitoring.java | 82 ++++++ 6 files changed, 547 insertions(+), 41 deletions(-) create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/util/Monitoring.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/WindowCacheGetTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/WindowCacheGetTest.java index 9063b6518..f1a18b096 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/WindowCacheGetTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/WindowCacheGetTest.java @@ -61,6 +61,7 @@ import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.storage.file.WindowCacheConfig; +import org.eclipse.jgit.storage.file.WindowCacheStats; import org.eclipse.jgit.test.resources.SampleDataRepositoryTestCase; import org.eclipse.jgit.util.MutableInteger; import org.junit.Before; @@ -102,8 +103,21 @@ public class WindowCacheGetTest extends SampleDataRepositoryTestCase { checkLimits(cfg); final WindowCache cache = WindowCache.getInstance(); - assertEquals(6, cache.getOpenFiles()); - assertEquals(17346, cache.getOpenBytes()); + WindowCacheStats s = cache.getStats(); + assertEquals(6, s.getOpenFileCount()); + assertEquals(17346, s.getOpenByteCount()); + assertEquals(0, s.getEvictionCount()); + assertEquals(90, s.getHitCount()); + assertTrue(s.getHitRatio() > 0.0 && s.getHitRatio() < 1.0); + assertEquals(6, s.getLoadCount()); + assertEquals(0, s.getLoadFailureCount()); + assertEquals(0, s.getLoadFailureRatio(), 0.001); + assertEquals(6, s.getLoadSuccessCount()); + assertEquals(6, s.getMissCount()); + assertTrue(s.getMissRatio() > 0.0 && s.getMissRatio() < 1.0); + assertEquals(96, s.getRequestCount()); + assertTrue(s.getAverageLoadTime() > 0.0); + assertTrue(s.getTotalLoadTime() > 0.0); } @Test @@ -127,10 +141,27 @@ public class WindowCacheGetTest extends SampleDataRepositoryTestCase { private static void checkLimits(WindowCacheConfig cfg) { final WindowCache cache = WindowCache.getInstance(); - assertTrue(cache.getOpenFiles() <= cfg.getPackedGitOpenFiles()); - assertTrue(cache.getOpenBytes() <= cfg.getPackedGitLimit()); - assertTrue(0 < cache.getOpenFiles()); - assertTrue(0 < cache.getOpenBytes()); + WindowCacheStats s = cache.getStats(); + assertTrue(0 < s.getAverageLoadTime()); + assertTrue(0 < s.getOpenByteCount()); + assertTrue(0 < s.getOpenByteCount()); + assertTrue(0.0 < s.getAverageLoadTime()); + assertTrue(0 <= s.getEvictionCount()); + assertTrue(0 < s.getHitCount()); + assertTrue(0 < s.getHitRatio()); + assertTrue(1 > s.getHitRatio()); + assertTrue(0 < s.getLoadCount()); + assertTrue(0 <= s.getLoadFailureCount()); + assertTrue(0.0 <= s.getLoadFailureRatio()); + assertTrue(1 > s.getLoadFailureRatio()); + assertTrue(0 < s.getLoadSuccessCount()); + assertTrue(s.getOpenByteCount() <= cfg.getPackedGitLimit()); + assertTrue(s.getOpenFileCount() <= cfg.getPackedGitOpenFiles()); + assertTrue(0 <= s.getMissCount()); + assertTrue(0 <= s.getMissRatio()); + assertTrue(1 > s.getMissRatio()); + assertTrue(0 < s.getRequestCount()); + assertTrue(0 < s.getTotalLoadTime()); } private void doCacheTests() throws IOException { diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 56edd61d7..9ee9ff7b6 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -43,6 +43,12 @@ + + + + + + @@ -78,6 +84,15 @@ + + + + + + + + + @@ -232,6 +247,14 @@ + + + + + + + + diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java index 8cf1d4e21..6cf5bd948 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java @@ -48,13 +48,16 @@ import java.io.IOException; import java.lang.ref.ReferenceQueue; import java.lang.ref.SoftReference; import java.util.Random; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReferenceArray; +import java.util.concurrent.atomic.LongAdder; import java.util.concurrent.locks.ReentrantLock; +import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.storage.file.WindowCacheConfig; +import org.eclipse.jgit.storage.file.WindowCacheStats; +import org.eclipse.jgit.util.Monitoring; /** * Caches slices of a {@link org.eclipse.jgit.internal.storage.file.PackFile} in @@ -124,6 +127,196 @@ import org.eclipse.jgit.storage.file.WindowCacheConfig; * other threads. */ public class WindowCache { + + /** + * Record statistics for a cache + */ + static interface StatsRecorder { + /** + * Record cache hits. Called when cache returns a cached entry. + * + * @param count + * number of cache hits to record + */ + void recordHits(int count); + + /** + * Record cache misses. Called when the cache returns an entry which had + * to be loaded. + * + * @param count + * number of cache misses to record + */ + void recordMisses(int count); + + /** + * Record a successful load of a cache entry + * + * @param loadTimeNanos + * time to load a cache entry + */ + void recordLoadSuccess(long loadTimeNanos); + + /** + * Record a failed load of a cache entry + * + * @param loadTimeNanos + * time used trying to load a cache entry + */ + void recordLoadFailure(long loadTimeNanos); + + /** + * Record cache evictions due to the cache evictions strategy + * + * @param count + * number of evictions to record + */ + void recordEvictions(int count); + + /** + * Record files opened by cache + * + * @param count + * delta of number of files opened by cache + */ + void recordOpenFiles(int count); + + /** + * Record cached bytes + * + * @param count + * delta of cached bytes + */ + void recordOpenBytes(int count); + + /** + * Returns a snapshot of this recorder's stats. Note that this may be an + * inconsistent view, as it may be interleaved with update operations. + * + * @return a snapshot of this recorder's stats + */ + @NonNull + WindowCacheStats getStats(); + } + + static class StatsRecorderImpl + implements StatsRecorder, WindowCacheStats { + private final LongAdder hitCount; + private final LongAdder missCount; + private final LongAdder loadSuccessCount; + private final LongAdder loadFailureCount; + private final LongAdder totalLoadTime; + private final LongAdder evictionCount; + private final LongAdder openFileCount; + private final LongAdder openByteCount; + + /** + * Constructs an instance with all counts initialized to zero. + */ + public StatsRecorderImpl() { + hitCount = new LongAdder(); + missCount = new LongAdder(); + loadSuccessCount = new LongAdder(); + loadFailureCount = new LongAdder(); + totalLoadTime = new LongAdder(); + evictionCount = new LongAdder(); + openFileCount = new LongAdder(); + openByteCount = new LongAdder(); + } + + @Override + public void recordHits(int count) { + hitCount.add(count); + } + + @Override + public void recordMisses(int count) { + missCount.add(count); + } + + @Override + public void recordLoadSuccess(long loadTimeNanos) { + loadSuccessCount.increment(); + totalLoadTime.add(loadTimeNanos); + } + + @Override + public void recordLoadFailure(long loadTimeNanos) { + loadFailureCount.increment(); + totalLoadTime.add(loadTimeNanos); + } + + @Override + public void recordEvictions(int count) { + evictionCount.add(count); + } + + @Override + public void recordOpenFiles(int count) { + openFileCount.add(count); + } + + @Override + public void recordOpenBytes(int count) { + openByteCount.add(count); + } + + @Override + public WindowCacheStats getStats() { + return this; + } + + @Override + public long getHitCount() { + return hitCount.sum(); + } + + @Override + public long getMissCount() { + return missCount.sum(); + } + + @Override + public long getLoadSuccessCount() { + return loadSuccessCount.sum(); + } + + @Override + public long getLoadFailureCount() { + return loadFailureCount.sum(); + } + + @Override + public long getEvictionCount() { + return evictionCount.sum(); + } + + @Override + public long getTotalLoadTime() { + return totalLoadTime.sum(); + } + + @Override + public long getOpenFileCount() { + return openFileCount.sum(); + } + + @Override + public long getOpenByteCount() { + return openByteCount.sum(); + } + + @Override + public void resetCounters() { + hitCount.reset(); + missCount.reset(); + loadSuccessCount.reset(); + loadFailureCount.reset(); + totalLoadTime.reset(); + evictionCount.reset(); + } + } + private static final int bits(int newSize) { if (newSize < 4096) throw new IllegalArgumentException(JGitText.get().invalidWindowSize); @@ -228,9 +421,9 @@ public class WindowCache { private final int windowSize; - private final AtomicInteger openFiles; + private final StatsRecorder statsRecorder; - private final AtomicLong openBytes; + private final StatsRecorderImpl mbean; private WindowCache(WindowCacheConfig cfg) { tableSize = tableSize(cfg); @@ -263,8 +456,9 @@ public class WindowCache { windowSizeShift = bits(cfg.getPackedGitWindowSize()); windowSize = 1 << windowSizeShift; - openFiles = new AtomicInteger(); - openBytes = new AtomicLong(); + mbean = new StatsRecorderImpl(); + statsRecorder = mbean; + Monitoring.registerMBean(WindowCacheStats.class, "block_cache"); //$NON-NLS-1$ if (maxFiles < 1) throw new IllegalArgumentException(JGitText.get().openFilesMustBeAtLeast1); @@ -273,61 +467,63 @@ public class WindowCache { } /** - * @return the number of open files. + * @return cache statistics for the WindowCache */ - public int getOpenFiles() { - return openFiles.get(); + public WindowCacheStats getStats() { + return statsRecorder.getStats(); } /** - * @return the number of open bytes. + * Reset stats. Does not reset open bytes and open files stats. */ - public long getOpenBytes() { - return openBytes.get(); + public void resetStats() { + mbean.resetCounters(); } private int hash(int packHash, long off) { return packHash + (int) (off >>> windowSizeShift); } - private ByteWindow load(PackFile pack, long offset) - throws IOException { + private ByteWindow load(PackFile pack, long offset) throws IOException { + long startTime = System.nanoTime(); if (pack.beginWindowCache()) - openFiles.incrementAndGet(); + statsRecorder.recordOpenFiles(1); try { if (mmap) return pack.mmap(offset, windowSize); - return pack.read(offset, windowSize); - } catch (IOException e) { - close(pack); - throw e; - } catch (RuntimeException e) { - close(pack); - throw e; - } catch (Error e) { + ByteArrayWindow w = pack.read(offset, windowSize); + statsRecorder.recordLoadSuccess(System.nanoTime() - startTime); + return w; + } catch (IOException | RuntimeException | Error e) { close(pack); + statsRecorder.recordLoadFailure(System.nanoTime() - startTime); throw e; + } finally { + statsRecorder.recordMisses(1); } } private Ref createRef(PackFile p, long o, ByteWindow v) { final Ref ref = new Ref(p, o, v, queue); - openBytes.addAndGet(ref.size); + statsRecorder.recordOpenBytes(ref.size); return ref; } private void clear(Ref ref) { - openBytes.addAndGet(-ref.size); + statsRecorder.recordOpenBytes(-ref.size); + statsRecorder.recordEvictions(1); close(ref.pack); } private void close(PackFile pack) { - if (pack.endWindowCache()) - openFiles.decrementAndGet(); + if (pack.endWindowCache()) { + statsRecorder.recordOpenFiles(-1); + } } private boolean isFull() { - return maxFiles < openFiles.get() || maxBytes < openBytes.get(); + return maxFiles < mbean.getOpenFileCount() + || maxBytes < mbean.getOpenByteCount(); } private long toStart(long offset) { @@ -365,15 +561,19 @@ public class WindowCache { final int slot = slot(pack, position); final Entry e1 = table.get(slot); ByteWindow v = scan(e1, pack, position); - if (v != null) + if (v != null) { + statsRecorder.recordHits(1); return v; + } synchronized (lock(pack, position)) { Entry e2 = table.get(slot); if (e2 != e1) { v = scan(e2, pack, position); - if (v != null) + if (v != null) { + statsRecorder.recordHits(1); return v; + } } v = load(pack, position); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java index 82ccd7b03..43776e081 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java @@ -451,4 +451,10 @@ public final class ConfigConstants { * @since 5.1.9 */ public static final String CONFIG_KEY_MIN_RACY_THRESHOLD = "minRacyThreshold"; + + /** + * The "jmx" section + * @since 5.1.13 + */ + public static final String CONFIG_JMX_SECTION = "jmx"; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheStats.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheStats.java index 3570733e4..b7f6394df 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheStats.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/WindowCacheStats.java @@ -40,29 +40,193 @@ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ - package org.eclipse.jgit.storage.file; +import javax.management.MXBean; + import org.eclipse.jgit.internal.storage.file.WindowCache; /** - * Accessor for stats about {@link WindowCache}. + * Cache statistics for {@link WindowCache}. * * @since 4.11 - * */ -public class WindowCacheStats { +@MXBean +public interface WindowCacheStats { /** * @return the number of open files. + * @deprecated use {@link #getOpenFileCount()} instead */ + @Deprecated public static int getOpenFiles() { - return WindowCache.getInstance().getOpenFiles(); + return (int) WindowCache.getInstance().getStats().getOpenFileCount(); } /** * @return the number of open bytes. + * @deprecated use {@link #getOpenByteCount()} instead */ + @Deprecated public static long getOpenBytes() { - return WindowCache.getInstance().getOpenBytes(); + return WindowCache.getInstance().getStats().getOpenByteCount(); + } + + /** + * @return cache statistics for the WindowCache + * @since 5.1.13 + */ + public static WindowCacheStats getStats() { + return WindowCache.getInstance().getStats(); + } + + /** + * Number of cache hits + * + * @return number of cache hits + */ + long getHitCount(); + + /** + * Ratio of cache requests which were hits defined as + * {@code hitCount / requestCount}, or {@code 1.0} when + * {@code requestCount == 0}. Note that {@code hitRate + missRate =~ 1.0}. + * + * @return the ratio of cache requests which were hits + */ + default double getHitRatio() { + long requestCount = getRequestCount(); + return (requestCount == 0) ? 1.0 + : (double) getHitCount() / requestCount; + } + + /** + * Number of cache misses. + * + * @return number of cash misses + */ + long getMissCount(); + + /** + * Ratio of cache requests which were misses defined as + * {@code missCount / requestCount}, or {@code 0.0} when + * {@code requestCount == 0}. Note that {@code hitRate + missRate =~ 1.0}. + * Cache misses include all requests which weren't cache hits, including + * requests which resulted in either successful or failed loading attempts. + * + * @return the ratio of cache requests which were misses + */ + default double getMissRatio() { + long requestCount = getRequestCount(); + return (requestCount == 0) ? 0.0 + : (double) getMissCount() / requestCount; + } + + /** + * Number of successful loads + * + * @return number of successful loads + */ + long getLoadSuccessCount(); + + /** + * Number of failed loads + * + * @return number of failed loads + */ + long getLoadFailureCount(); + + /** + * Ratio of cache load attempts which threw exceptions. This is defined as + * {@code loadFailureCount / (loadSuccessCount + loadFailureCount)}, or + * {@code 0.0} when {@code loadSuccessCount + loadFailureCount == 0}. + * + * @return the ratio of cache loading attempts which threw exceptions + */ + default double getLoadFailureRatio() { + long loadFailureCount = getLoadFailureCount(); + long totalLoadCount = getLoadSuccessCount() + loadFailureCount; + return (totalLoadCount == 0) ? 0.0 + : (double) loadFailureCount / totalLoadCount; } + + /** + * Total number of times that the cache attempted to load new values. This + * includes both successful load operations, as well as failed loads. This + * is defined as {@code loadSuccessCount + loadFailureCount}. + * + * @return the {@code loadSuccessCount + loadFailureCount} + */ + default long getLoadCount() { + return getLoadSuccessCount() + getLoadFailureCount(); + } + + /** + * Number of cache evictions + * + * @return number of evictions + */ + long getEvictionCount(); + + /** + * Ratio of cache evictions. This is defined as + * {@code evictionCount / requestCount}, or {@code 0.0} when + * {@code requestCount == 0}. + * + * @return the ratio of cache loading attempts which threw exceptions + */ + default double getEvictionRatio() { + long evictionCount = getEvictionCount(); + long requestCount = getRequestCount(); + return (requestCount == 0) ? 0.0 + : (double) evictionCount / requestCount; + } + + /** + * Number of times the cache returned either a cached or uncached value. + * This is defined as {@code hitCount + missCount}. + * + * @return the {@code hitCount + missCount} + */ + default long getRequestCount() { + return getHitCount() + getMissCount(); + } + + /** + * Average time in nanoseconds for loading new values. This is + * {@code totalLoadTime / (loadSuccessCount + loadFailureCount)}. + * + * @return the average time spent loading new values + */ + default double getAverageLoadTime() { + long totalLoadCount = getLoadSuccessCount() + getLoadFailureCount(); + return (totalLoadCount == 0) ? 0.0 + : (double) getTotalLoadTime() / totalLoadCount; + } + + /** + * Total time in nanoseconds the cache spent loading new values. + * + * @return the total number of nanoseconds the cache has spent loading new + * values + */ + long getTotalLoadTime(); + + /** + * Number of pack files kept open by the cache + * + * @return number of files kept open by cache + */ + long getOpenFileCount(); + + /** + * Number of bytes cached + * + * @return number of bytes cached + */ + long getOpenByteCount(); + + /** + * Reset counters. Does not reset open bytes and open files counters. + */ + void resetCounters(); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/Monitoring.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/Monitoring.java new file mode 100644 index 000000000..a3c8f3e04 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/Monitoring.java @@ -0,0 +1,82 @@ +/* + * Copyright (c) 2019 Matthias Sohn + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +package org.eclipse.jgit.util; + +import java.io.IOException; +import java.lang.management.ManagementFactory; + +import javax.management.InstanceAlreadyExistsException; +import javax.management.InstanceNotFoundException; +import javax.management.MBeanRegistrationException; +import javax.management.MBeanServer; +import javax.management.MalformedObjectNameException; +import javax.management.NotCompliantMBeanException; +import javax.management.ObjectInstance; +import javax.management.ObjectName; + +import org.eclipse.jgit.annotations.Nullable; +import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.lib.ConfigConstants; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Enables monitoring JGit via JMX + * + * @since 5.1.13 + */ +public class Monitoring { + private final static Logger LOG = LoggerFactory.getLogger(Monitoring.class); + + /** + * Register a MBean with the platform MBean server + * + * @param mbean + * the mbean interface to register + * @param metricName + * name of the JGit metric, will be prefixed with + * "org.eclipse.jgit/" + * @return the registered mbean's object instance + */ + public static @Nullable ObjectInstance registerMBean(Class mbean, + String metricName) { + boolean register; + try { + register = SystemReader.getInstance().getUserConfig().getBoolean( + ConfigConstants.CONFIG_JMX_SECTION, + mbean.getSimpleName(), false); + } catch (IOException | ConfigInvalidException e) { + LOG.error(e.getMessage(), e); + return null; + } + if (!register) { + return null; + } + MBeanServer server = ManagementFactory.getPlatformMBeanServer(); + try { + ObjectName mbeanName = objectName(mbean, metricName); + if (server.isRegistered(mbeanName)) { + server.unregisterMBean(mbeanName); + } + return server.registerMBean(mbean, mbeanName); + } catch (MalformedObjectNameException | InstanceAlreadyExistsException + | MBeanRegistrationException | NotCompliantMBeanException + | InstanceNotFoundException e) { + LOG.error(e.getMessage(), e); + return null; + } + } + + private static ObjectName objectName(Class mbean, String metricName) + throws MalformedObjectNameException { + return new ObjectName(String.format("org.eclipse.jgit/%s:type=%s", //$NON-NLS-1$ + metricName, mbean.getSimpleName())); + } +} From 549c3acc5f5ced76b8649630850e00f68798f311 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 13 Dec 2019 17:32:51 +0100 Subject: [PATCH 02/12] Fix MBean registration Change-Id: I6f6b8641f6c3e8ab9f625594085014272305656a Signed-off-by: Matthias Sohn --- .../internal/storage/file/WindowCache.java | 2 +- .../src/org/eclipse/jgit/util/Monitoring.java | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java index 6cf5bd948..797507dd1 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java @@ -458,7 +458,7 @@ public class WindowCache { mbean = new StatsRecorderImpl(); statsRecorder = mbean; - Monitoring.registerMBean(WindowCacheStats.class, "block_cache"); //$NON-NLS-1$ + Monitoring.registerMBean(mbean, "block_cache"); //$NON-NLS-1$ if (maxFiles < 1) throw new IllegalArgumentException(JGitText.get().openFilesMustBeAtLeast1); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/Monitoring.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/Monitoring.java index a3c8f3e04..83bf695f7 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/Monitoring.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/Monitoring.java @@ -39,19 +39,26 @@ public class Monitoring { * Register a MBean with the platform MBean server * * @param mbean - * the mbean interface to register + * the mbean object to register * @param metricName * name of the JGit metric, will be prefixed with * "org.eclipse.jgit/" * @return the registered mbean's object instance */ - public static @Nullable ObjectInstance registerMBean(Class mbean, + public static @Nullable ObjectInstance registerMBean(Object mbean, String metricName) { - boolean register; + boolean register = false; try { - register = SystemReader.getInstance().getUserConfig().getBoolean( + Class interfaces[] = mbean.getClass().getInterfaces(); + for (Class i : interfaces) { + register = SystemReader.getInstance().getUserConfig() + .getBoolean( ConfigConstants.CONFIG_JMX_SECTION, - mbean.getSimpleName(), false); + i.getSimpleName(), false); + if (register) { + break; + } + } } catch (IOException | ConfigInvalidException e) { LOG.error(e.getMessage(), e); return null; @@ -61,7 +68,7 @@ public class Monitoring { } MBeanServer server = ManagementFactory.getPlatformMBeanServer(); try { - ObjectName mbeanName = objectName(mbean, metricName); + ObjectName mbeanName = objectName(mbean.getClass(), metricName); if (server.isRegistered(mbeanName)) { server.unregisterMBean(mbeanName); } From e3922b590bef45c72cd43e9629fda5f173be74ca Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 16 Dec 2019 15:22:08 +0100 Subject: [PATCH 03/12] Ignore warnings for generated source code in org.eclipse.jgit.benchmark The source code in the folder .apt_generated is generated by the JMH code generator, so there's no point in raising any warnings as this could only be fixed in the upstream code generator. Change-Id: I882888e7bf924f9ae74182598fcb91671a5c9818 Signed-off-by: Matthias Sohn --- org.eclipse.jgit.benchmarks/.classpath | 1 + 1 file changed, 1 insertion(+) diff --git a/org.eclipse.jgit.benchmarks/.classpath b/org.eclipse.jgit.benchmarks/.classpath index 438d155e3..1285bd9e3 100644 --- a/org.eclipse.jgit.benchmarks/.classpath +++ b/org.eclipse.jgit.benchmarks/.classpath @@ -18,6 +18,7 @@ + From 3c0c38119427089778c4bf0124821345fa136205 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 18 Dec 2019 15:49:18 +0100 Subject: [PATCH 04/12] Add dependency to enable site generation for benchmark module Change-Id: Iae4524ddc730d57993e9c6d6807282e4b07d1336 Signed-off-by: Matthias Sohn --- org.eclipse.jgit.benchmarks/pom.xml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/org.eclipse.jgit.benchmarks/pom.xml b/org.eclipse.jgit.benchmarks/pom.xml index 4e60ab9b7..82c0a080a 100644 --- a/org.eclipse.jgit.benchmarks/pom.xml +++ b/org.eclipse.jgit.benchmarks/pom.xml @@ -189,6 +189,33 @@ + + org.apache.maven.plugins + maven-site-plugin + 3.8.2 + + + org.apache.maven.wagon + wagon-ssh + 3.3.4 + + + + + org.apache.maven.plugins + maven-surefire-report-plugin + 3.0.0-M3 + + + org.apache.maven.plugins + maven-jxr-plugin + 3.0.0 + + + org.apache.maven.plugins + maven-project-info-reports-plugin + 3.0.0 + From 8669d6df188bf0def1e4afe729aac63f36adc851 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 18 Dec 2019 15:50:35 +0100 Subject: [PATCH 05/12] Update maven-site-plugin used by benchmark module to 3.8.2 The benchmark module currently has no parent, adjust the version used here to the one used by all the other jgit Maven modules. Change-Id: I8807a694fe23f8f131d1d22a58a3e18874d756cc Signed-off-by: Matthias Sohn --- org.eclipse.jgit.packaging/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jgit.packaging/pom.xml b/org.eclipse.jgit.packaging/pom.xml index 5cbd2d485..cf43ab199 100644 --- a/org.eclipse.jgit.packaging/pom.xml +++ b/org.eclipse.jgit.packaging/pom.xml @@ -271,7 +271,7 @@ org.apache.maven.plugins maven-site-plugin - 3.7.1 + 3.8.2 From d422a820860447905027d3f88ee72880036ecfb0 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 4 Jan 2020 13:10:00 +0100 Subject: [PATCH 06/12] Remove duplicate ignore_optional_problems entry in .classpath Change-Id: I326377c90af59ecaada7f5185a638726a8e909b7 Signed-off-by: Matthias Sohn --- org.eclipse.jgit.benchmarks/.classpath | 1 - 1 file changed, 1 deletion(-) diff --git a/org.eclipse.jgit.benchmarks/.classpath b/org.eclipse.jgit.benchmarks/.classpath index 1285bd9e3..438d155e3 100644 --- a/org.eclipse.jgit.benchmarks/.classpath +++ b/org.eclipse.jgit.benchmarks/.classpath @@ -18,7 +18,6 @@ - From f4e653328a9ba14202c7b655bc54cf1f9dcef932 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 4 Jan 2020 13:11:06 +0100 Subject: [PATCH 07/12] Remove unused import from CreateFileSnapshotBenchmark Change-Id: Iad0bcc01ada4252e9ab4f60d4375f98f084f6a5f Signed-off-by: Matthias Sohn --- .../org/eclipse/jgit/benchmarks/CreateFileSnapshotBenchmark.java | 1 - 1 file changed, 1 deletion(-) diff --git a/org.eclipse.jgit.benchmarks/src/org/eclipse/jgit/benchmarks/CreateFileSnapshotBenchmark.java b/org.eclipse.jgit.benchmarks/src/org/eclipse/jgit/benchmarks/CreateFileSnapshotBenchmark.java index ffe4a26d8..21c54c563 100644 --- a/org.eclipse.jgit.benchmarks/src/org/eclipse/jgit/benchmarks/CreateFileSnapshotBenchmark.java +++ b/org.eclipse.jgit.benchmarks/src/org/eclipse/jgit/benchmarks/CreateFileSnapshotBenchmark.java @@ -45,7 +45,6 @@ package org.eclipse.jgit.benchmarks; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.UUID; import java.util.concurrent.TimeUnit; import org.eclipse.jgit.internal.storage.file.FileSnapshot; From ebea36aa07520f7f4b873930ca9b4dbca8db52ef Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 4 Jan 2020 13:15:37 +0100 Subject: [PATCH 08/12] Don't override already managed maven-compiler-plugin version Change-Id: Ie2cb178cf8d805aadc76a2096bcdde95a146d07c Signed-off-by: Matthias Sohn --- pom.xml | 2 -- 1 file changed, 2 deletions(-) diff --git a/pom.xml b/pom.xml index 28f180e68..3896c5f7b 100644 --- a/pom.xml +++ b/pom.xml @@ -766,7 +766,6 @@ maven-compiler-plugin - ${maven-compiler-plugin-version} UTF-8 1.8 @@ -835,7 +834,6 @@ maven-compiler-plugin - ${maven-compiler-plugin-version} eclipse UTF-8 From d9e957dc2485d4a0737e537298e575365ec5eb2c Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 4 Jan 2020 15:44:19 +0100 Subject: [PATCH 09/12] SshSupport#runSshCommand: don't throw exception in finally block The CommandFailedException which was thrown in finally block is silently discarded [1]. Refactor this method to throw the exception after the finally block. This fixes the warning "Null comparison always yields false: The variable failure can only be null at this location". [1] https://wiki.sei.cmu.edu/confluence/display/java/ERR04-J.+Do+not+complete+abruptly+from+a+finally+block https://docs.oracle.com/javase/specs/jls/se8/html/jls-14.html#jls-14.20.2 Change-Id: Idbfc303d9c9046ab9a43e0d4c6d65d325bdaf0ed Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/util/SshSupport.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java index 913aa7286..ac5be1bf2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java @@ -94,6 +94,7 @@ public class SshSupport { CommandFailedException failure = null; @SuppressWarnings("resource") MessageWriter stderr = new MessageWriter(); + String out; try (MessageWriter stdout = new MessageWriter()) { session = SshSessionFactory.getInstance().getSession(sshUri, provider, fs, 1000 * timeout); @@ -108,12 +109,12 @@ public class SshSupport { // waitFor with timeout has a bug - JSch' exitValue() throws the // wrong exception type :( if (process.waitFor() == 0) { - return stdout.toString(); + out = stdout.toString(); } else { - return null; // still running after timeout + out = null; // still running after timeout } } catch (InterruptedException e) { - return null; // error + out = null; // error } } finally { if (errorThread != null) { @@ -147,10 +148,11 @@ public class SshSupport { if (session != null) { SshSessionFactory.getInstance().releaseSession(session); } - if (failure != null) { - throw failure; - } } + if (failure != null) { + throw failure; + } + return out; } } From 14bfe08757fa6267df2844c7365af9fb9ebc1578 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 4 Jan 2020 16:06:54 +0100 Subject: [PATCH 10/12] JschConfigSessionFactory: fix boxing warning Change-Id: I1735033c56b444a9a7160cb7df89292a228d5b34 Signed-off-by: Matthias Sohn --- .../org/eclipse/jgit/transport/JschConfigSessionFactory.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java index 4e712a556..7924ec8c2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java @@ -278,7 +278,8 @@ public abstract class JschConfigSessionFactory extends SshSessionFactory { if (s.getPort() == SSH_PORT) { return s.getHost(); } - return String.format("[%s]:%d", s.getHost(), s.getPort()); //$NON-NLS-1$ + return String.format("[%s]:%d", s.getHost(), //$NON-NLS-1$ + Integer.valueOf(s.getPort())); } private void copyConfigValueToSession(Session session, Config cfg, From 4209a0f84b1917c0640d781ac54e8c5ffbadd574 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 4 Jan 2020 17:03:52 +0100 Subject: [PATCH 11/12] Fix unclosed resource warning in SmartOutputStream Change-Id: Ia4b96ae1c2cc9357802487384ee32a80ed40334b Signed-off-by: Matthias Sohn --- .../jgit/http/server/SmartOutputStream.java | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/SmartOutputStream.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/SmartOutputStream.java index ad5e8d479..0dfaec256 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/SmartOutputStream.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/SmartOutputStream.java @@ -105,17 +105,16 @@ class SmartOutputStream extends TemporaryBuffer { // If output hasn't started yet, the entire thing fit into our // buffer. Try to use a proper Content-Length header, and also // deflate the response with gzip if it will be smaller. - TemporaryBuffer out = this; - - if (256 < out.length() && acceptsGzipEncoding(req)) { + if (256 < this.length() && acceptsGzipEncoding(req)) { TemporaryBuffer gzbuf = new TemporaryBuffer.Heap(LIMIT); try { try (GZIPOutputStream gzip = new GZIPOutputStream(gzbuf)) { - out.writeTo(gzip, null); + this.writeTo(gzip, null); } - if (gzbuf.length() < out.length()) { - out = gzbuf; + if (gzbuf.length() < this.length()) { rsp.setHeader(HDR_CONTENT_ENCODING, ENCODING_GZIP); + writeResponse(gzbuf); + return; } } catch (IOException err) { // Most likely caused by overflowing the buffer, meaning @@ -123,15 +122,18 @@ class SmartOutputStream extends TemporaryBuffer { // copy and use the original. } } + writeResponse(this); + } + } - // The Content-Length cannot overflow when cast to an int, our - // hardcoded LIMIT constant above assures us we wouldn't store - // more than 2 GiB of content in memory. - rsp.setContentLength((int) out.length()); - try (OutputStream os = rsp.getOutputStream()) { - out.writeTo(os, null); - os.flush(); - } + private void writeResponse(TemporaryBuffer out) throws IOException { + // The Content-Length cannot overflow when cast to an int, our + // hardcoded LIMIT constant above assures us we wouldn't store + // more than 2 GiB of content in memory. + rsp.setContentLength((int) out.length()); + try (OutputStream os = rsp.getOutputStream()) { + out.writeTo(os, null); + os.flush(); } } } From fefddb28b23d9b49215cfc813acd8612a9cda8bb Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 4 Jan 2020 13:01:39 +0100 Subject: [PATCH 12/12] Fix API problem filters Change-Id: Icc78570f949ad64beb58caa192c829e536aa8dad Signed-off-by: Matthias Sohn --- .../.settings/.api_filters | 8 -- org.eclipse.jgit.lfs/.settings/.api_filters | 11 --- org.eclipse.jgit/.settings/.api_filters | 82 +++++++++++++++++-- 3 files changed, 74 insertions(+), 27 deletions(-) delete mode 100644 org.eclipse.jgit.lfs/.settings/.api_filters diff --git a/org.eclipse.jgit.lfs.server/.settings/.api_filters b/org.eclipse.jgit.lfs.server/.settings/.api_filters index 3e60680cf..6609c3d40 100644 --- a/org.eclipse.jgit.lfs.server/.settings/.api_filters +++ b/org.eclipse.jgit.lfs.server/.settings/.api_filters @@ -1,13 +1,5 @@ - - - - - - - - diff --git a/org.eclipse.jgit.lfs/.settings/.api_filters b/org.eclipse.jgit.lfs/.settings/.api_filters deleted file mode 100644 index 95a45574e..000000000 --- a/org.eclipse.jgit.lfs/.settings/.api_filters +++ /dev/null @@ -1,11 +0,0 @@ - - - - - - - - - - - diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index cbb9de113..857fcac17 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -1,13 +1,5 @@ - - - - - - - - @@ -42,6 +34,28 @@ + + + + + + + + + + + + + + + + + + + + + + @@ -62,6 +76,42 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -118,6 +168,14 @@ + + + + + + + + @@ -132,6 +190,14 @@ + + + + + + + +