From f73b13c14277bb0ffb4387f22f3472920549b461 Mon Sep 17 00:00:00 2001
From: vruano <valentin.ruano@gmail.com>
Date: Tue, 10 Jan 2023 23:01:28 -0500
Subject: [PATCH] Removed 'backed' last cache hit hard references; it seems
 that that it might be trying to solve an non-existent issue. Chaned
 WeakReference to SoftReference to make cache more persistent in memory.
 Addresses issue #1643.

---
 .../samtools/cram/ref/ReferenceSource.java    | 28 ++++---------------
 1 file changed, 5 insertions(+), 23 deletions(-)

diff --git a/src/main/java/htsjdk/samtools/cram/ref/ReferenceSource.java b/src/main/java/htsjdk/samtools/cram/ref/ReferenceSource.java
index 32810f8678..4f85c07402 100644
--- a/src/main/java/htsjdk/samtools/cram/ref/ReferenceSource.java
+++ b/src/main/java/htsjdk/samtools/cram/ref/ReferenceSource.java
@@ -33,7 +33,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.lang.ref.WeakReference;
+import java.lang.ref.SoftReference;
 import java.net.URL;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -58,13 +58,7 @@ public class ReferenceSource implements CRAMReferenceSource {
     private final ReferenceSequenceFile rsFile;
     private int downloadTriesBeforeFailing = 2;
 
-    private final Map<String, WeakReference<byte[]>> cacheW = new HashMap<>();
-
-    // Optimize for locality of reference by maintaining the backing reference bases for the most recent request.
-    // Failure to do this will result in the bases being aggressively GC'd when the only other reference to them
-    // is the weak reference hash map above, resulting in much thrashing.
-    private byte[] backingReferenceBases;
-    private int backingContigIndex;
+    private final Map<String, SoftReference<byte[]>> cache = new HashMap<>();
 
     public ReferenceSource(final File file) {
         this(IOUtil.toPath(file));
@@ -116,7 +110,7 @@ else if (Defaults.USE_CRAM_REF_DOWNLOAD) {
     }
 
     private byte[] findInCache(final String name) {
-        final WeakReference<byte[]> weakReference = cacheW.get(name);
+        final SoftReference<byte[]> weakReference = cache.get(name);
         if (weakReference != null) {
             final byte[] bytes = weakReference.get();
             if (bytes != null)
@@ -133,7 +127,7 @@ private byte[] addToCache(final String sequenceName, final byte[] bases) {
         for (int i = 0; i < bases.length; i++) {
             bases[i] = StringUtil.toUpperCase(bases[i]);
         }
-        cacheW.put(sequenceName, new WeakReference<>(bases));
+        cache.put(sequenceName, new SoftReference<>(bases));
         return bases;
     }
 
@@ -196,13 +190,9 @@ public byte[] getReferenceBasesByRegion(
 
         // this implementation maintains the entire reference sequence, and hands out whatever region
         // of it is requested
-        final byte[] bases = getBackingBases(sequenceRecord);
+        final byte[] bases = getReferenceBases(sequenceRecord, false);
 
         if (bases != null) {
-            // cache the backing bases to prevent thrashing due to aggressive GC
-            backingReferenceBases = bases;
-            backingContigIndex = sequenceRecord.getSequenceIndex();
-
             if (zeroBasedStart >= bases.length) {
                 throw new IllegalArgumentException(String.format("Requested start %d is beyond the sequence length %s",
                         zeroBasedStart,
@@ -213,14 +203,6 @@ public byte[] getReferenceBasesByRegion(
         return bases;
     }
 
-    private byte[] getBackingBases(final SAMSequenceRecord sequenceRecord) {
-        if (backingReferenceBases != null && backingContigIndex == sequenceRecord.getSequenceIndex()) {
-            return backingReferenceBases;
-        } else {
-            return getReferenceBases(sequenceRecord, false);
-        }
-    }
-
     private byte[] findBasesByName(final String name, final boolean tryVariants) {
         if (rsFile == null || !rsFile.isIndexed())
             return null;