Re: [JGIT PATCH 2/2] Allow core.packedGitLimit to exceed "2 g"
Show 72 quoted lines
> A 64 bit JVM might actually be able to dedicate more than 2 GiB of
> memory to the window cache, and on a busy server, this may be an
> ideal configuration since JGit can't always reliably use mmap. By
> treating the limit as a long we increase our range to 2^63, which
> is far beyond what any JVM heap would be able to actually support.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
> .../src/org/spearce/jgit/lib/WindowCache.java | 13 +++++++------
> .../org/spearce/jgit/lib/WindowCacheConfig.java | 8 ++++----
> 2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
> index 0c60853..b6a35ad 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
> @@ -41,6 +41,7 @@
> import java.io.IOException;
> import java.lang.ref.ReferenceQueue;
> import java.util.concurrent.atomic.AtomicInteger;
> +import java.util.concurrent.atomic.AtomicLong;
>
> /**
> * Caches slices of a {@link PackFile} in memory for faster read access.
> @@ -140,7 +141,7 @@ static final void purge(final PackFile pack) {
>
> private final int maxFiles;
>
> - private final int maxBytes;
> + private final long maxBytes;
>
> private final boolean mmap;
>
> @@ -150,7 +151,7 @@ static final void purge(final PackFile pack) {
>
> private final AtomicInteger openFiles;
>
> - private final AtomicInteger openBytes;
> + private final AtomicLong openBytes;
>
> private WindowCache(final WindowCacheConfig cfg) {
> super(tableSize(cfg), lockCount(cfg));
> @@ -161,7 +162,7 @@ private WindowCache(final WindowCacheConfig cfg) {
> windowSize = 1 << windowSizeShift;
>
> openFiles = new AtomicInteger();
> - openBytes = new AtomicInteger();
> + openBytes = new AtomicLong();
>
> if (maxFiles < 1)
> throw new IllegalArgumentException("Open files must be >= 1");
> @@ -173,7 +174,7 @@ int getOpenFiles() {
> return openFiles.get();
> }
>
> - int getOpenBytes() {
> + long getOpenBytes() {
> return openBytes.get();
> }
>
> @@ -233,12 +234,12 @@ private long toStart(final long offset) {
>
> private static int tableSize(final WindowCacheConfig cfg) {
> final int wsz = cfg.getPackedGitWindowSize();
> - final int limit = cfg.getPackedGitLimit();
> + final long limit = cfg.getPackedGitLimit();
> if (wsz <= 0)
> throw new IllegalArgumentException("Invalid window size");
> if (limit < wsz)
> throw new IllegalArgumentException("Window size must be < limit");
> - return 5 * (limit / wsz) / 2;
> + return (int) Math.min(5 * (limit / wsz) / 2, 2000000000);Math.min returns a long because the prototype Math.min(long,long) will
be chosen. The cast can then overflow and fail. Better change the return
type to a long:
+ return Math.min(5 * (limit / wsz) / 2, 2000000000L);
Show 43 quoted lines
> }
>
> private static int lockCount(final WindowCacheConfig cfg) {
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
> index ea28164..97edd3a 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCacheConfig.java
> @@ -47,7 +47,7 @@
>
> private int packedGitOpenFiles;
>
> - private int packedGitLimit;
> + private long packedGitLimit;
>
> private int packedGitWindowSize;
>
> @@ -85,7 +85,7 @@ public void setPackedGitOpenFiles(final int fdLimit) {
> * @return maximum number bytes of heap memory to dedicate to caching pack
> * file data. <b>Default is 10 MB.</b>
> */
> - public int getPackedGitLimit() {
> + public long getPackedGitLimit() {
> return packedGitLimit;
> }
>
> @@ -94,7 +94,7 @@ public int getPackedGitLimit() {
> * maximum number bytes of heap memory to dedicate to caching
> * pack file data.
> */
> - public void setPackedGitLimit(final int newLimit) {
> + public void setPackedGitLimit(final long newLimit) {
> packedGitLimit = newLimit;
> }
>
> @@ -162,7 +162,7 @@ public void setDeltaBaseCacheLimit(final int newLimit) {
> */
> public void fromConfig(final RepositoryConfig rc) {
> setPackedGitOpenFiles(rc.getInt("core", null, "packedgitopenfiles", getPackedGitOpenFiles()));
> - setPackedGitLimit(rc.getInt("core", null, "packedgitlimit", getPackedGitLimit()));
> + setPackedGitLimit(rc.getLong("core", null, "packedgitlimit", getPackedGitLimit()));
> setPackedGitWindowSize(rc.getInt("core", null, "packedgitwindowsize", getPackedGitWindowSize()));
> setPackedGitMMAP(rc.getBoolean("core", null, "packedgitmmap", isPackedGitMMAP()));
> setDeltaBaseCacheLimit(rc.getInt("core", null, "deltabasecachelimit", getDeltaBaseCacheLimit()));