From 0895474d415f696153a3ac4001554968fa684bb9 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Mon, 18 Dec 2023 17:31:05 -0500 Subject: [PATCH] Update for to use Java 21 * update tests to run on 21 * remove deprecated uses of finalize, make DiskBackQueue and associated Autocloseable so that it's easier for downstream code to handle them without finalize Note: if this proves to be insufficient we should look into implementing a Cleaner * remove deprecated boxing constructors new Byte() -> Byte.valueof() etc --- .github/workflows/tests.yml | 14 ++--- build.gradle | 2 +- .../htsjdk/samtools/util/DiskBackedQueue.java | 25 ++++---- .../util/FileAppendStreamLRUCache.java | 1 - .../util/SamRecordTrackingBuffer.java | 22 +++++-- .../SAMBinaryTagAndValueUnitTest.java | 62 +++++++++---------- 6 files changed, 66 insertions(+), 60 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0975777cc8..e90f8e5253 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,7 +14,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - java: [ 17 ] + java: [ 21 ] experimental: [false] fail-fast: false continue-on-error: ${{ matrix.experimental }} @@ -49,10 +49,10 @@ jobs: name: Tests that require external APIs steps: - uses: actions/checkout@v3 - - name: Set up java 17 + - name: Set up java 21 uses: actions/setup-java@v3 with: - java-version: '17' + java-version: '21' distribution: 'adopt' cache: gradle - name: Grant execute permission for gradlew @@ -75,10 +75,10 @@ jobs: name: FTP tests steps: - uses: actions/checkout@v3 - - name: Set up java 17 + - name: Set up java 21 uses: actions/setup-java@v3 with: - java-version: '17' + java-version: '21' distribution: 'adopt' cache: gradle - name: Grant execute permission for gradlew @@ -100,10 +100,10 @@ jobs: name: SpotBugs steps: - uses: actions/checkout@v3 - - name: Set up java 17 + - name: Set up java 21 uses: actions/setup-java@v3 with: - java-version: '17' + java-version: '21' distribution: 'adopt' cache: gradle - name: Grant execute permission for gradlew diff --git a/build.gradle b/build.gradle index fb4145dcda..9a1b5901aa 100644 --- a/build.gradle +++ b/build.gradle @@ -47,7 +47,7 @@ dependencies { java { toolchain { - languageVersion = JavaLanguageVersion.of(17) + languageVersion = JavaLanguageVersion.of(21) } withJavadocJar() withSourcesJar() diff --git a/src/main/java/htsjdk/samtools/util/DiskBackedQueue.java b/src/main/java/htsjdk/samtools/util/DiskBackedQueue.java index b353fb132f..4cc0924311 100644 --- a/src/main/java/htsjdk/samtools/util/DiskBackedQueue.java +++ b/src/main/java/htsjdk/samtools/util/DiskBackedQueue.java @@ -54,7 +54,7 @@ * * Created by bradt on 4/28/14. */ -public class DiskBackedQueue implements Queue { +public class DiskBackedQueue implements Queue, AutoCloseable { private final int maxRecordsInRamQueue; private final Queue ramRecords; private Path diskRecords = null; @@ -95,7 +95,7 @@ private DiskBackedQueue(final SortingCollection.Codec codec, this.tmpDirs = tmpDirs; this.codec = codec; this.maxRecordsInRamQueue = (maxRecordsInRam == 0) ? 0 : maxRecordsInRam - 1; // the first of our ram records is stored as headRecord - this.ramRecords = new ArrayDeque(this.maxRecordsInRamQueue); + this.ramRecords = new ArrayDeque<>(this.maxRecordsInRamQueue); } /** @@ -248,18 +248,6 @@ public void clear() { this.canAdd = true; } - /** - * Clean up disk resources in case clear() has not been explicitly called (as would be preferable) - * Closes the input and output streams associated with this DiskBackedQueue and deletes the temporary file - * - * @throws Throwable - */ - @Override - protected void finalize() throws Throwable { - this.closeIOResources(); - super.finalize(); // NB: intellij wanted me to do this. Need I? I'm not extending anything - } - /** * Write the present record to the end of a file representing the tail of the queue. * @throws RuntimeIOException @@ -420,4 +408,13 @@ public Object[] toArray() { public T1[] toArray(final T1[] a) { throw new UnsupportedOperationException("DiskBackedQueue does not support toArray(T1[] a)"); } + + /** + * Clean up disk resources in case clear() has not been explicitly called (as would be preferable) + * Closes the input and output streams associated with this DiskBackedQueue and deletes the temporary file + */ + @Override + public void close() { + clear(); + } } diff --git a/src/main/java/htsjdk/samtools/util/FileAppendStreamLRUCache.java b/src/main/java/htsjdk/samtools/util/FileAppendStreamLRUCache.java index 500b931824..86b453674c 100644 --- a/src/main/java/htsjdk/samtools/util/FileAppendStreamLRUCache.java +++ b/src/main/java/htsjdk/samtools/util/FileAppendStreamLRUCache.java @@ -56,7 +56,6 @@ public OutputStream makeValue(final File file) { // In case the file could not be opened because of too many file handles, try to force // file handles to be closed. System.gc(); - System.runFinalization(); try { return IOUtil.maybeBufferOutputStream(new FileOutputStream(file, true)); } diff --git a/src/main/java/htsjdk/samtools/util/SamRecordTrackingBuffer.java b/src/main/java/htsjdk/samtools/util/SamRecordTrackingBuffer.java index 46cf8bff14..d1bd8e03e5 100644 --- a/src/main/java/htsjdk/samtools/util/SamRecordTrackingBuffer.java +++ b/src/main/java/htsjdk/samtools/util/SamRecordTrackingBuffer.java @@ -54,7 +54,7 @@ * * @author bradtaylor */ -public class SamRecordTrackingBuffer { +public class SamRecordTrackingBuffer implements AutoCloseable { private int availableRecordsInMemory; // how many more records can we store in memory private final int blockSize; // the size of each block private final List tmpDirs; // the list of temporary directories to use @@ -78,7 +78,7 @@ public SamRecordTrackingBuffer(final int maxRecordsInRam, final int blockSize, f this.tmpDirs = tmpDirs; this.queueHeadRecordIndex = -1; this.queueTailRecordIndex = -1; - this.blocks = new ArrayDeque(); + this.blocks = new ArrayDeque<>(); this.header = header; this.clazz = clazz; } @@ -184,6 +184,7 @@ public void setResultState(final SamRecordWithOrdinal samRecordWithOrdinal, fina /** * Close IO resources associated with each underlying BufferBlock */ + @Override public void close() { while (!blocks.isEmpty()) { final BufferBlock block = blocks.pollFirst(); @@ -194,7 +195,7 @@ public void close() { /** * This stores blocks of records, either in memory or on disk, or both! */ - private class BufferBlock { + private class BufferBlock implements AutoCloseable { private final DiskBackedQueue recordsQueue; private final int maxBlockSize; private long currentStartIndex; @@ -256,7 +257,7 @@ public void add(final SamRecordWithOrdinal samRecordWithOrdinal) { } } - private int ensureIndexFitsInAnInt(final long value) { + private static int ensureIndexFitsInAnInt(final long value) { if (value < Integer.MIN_VALUE || Integer.MAX_VALUE < value) throw new SAMException("Error: index out of range: " + value); return (int)value; } @@ -288,7 +289,7 @@ public SamRecordWithOrdinal next() throws IllegalStateException { if (this.canEmit()) { try { // create a wrapped record for the head of the queue, and set the underlying record's examined information appropriately - final SamRecordWithOrdinal samRecordWithOrdinal = clazz.newInstance(); + final SamRecordWithOrdinal samRecordWithOrdinal = clazz.getDeclaredConstructor().newInstance(); samRecordWithOrdinal.setRecord(this.recordsQueue.poll()); samRecordWithOrdinal.setRecordOrdinal(this.currentStartIndex); samRecordWithOrdinal.setResultState(this.resultStateIndexes.get(ensureIndexFitsInAnInt(this.currentStartIndex - this.originalStartIndex))); @@ -316,6 +317,15 @@ public SamRecordWithOrdinal next() throws IllegalStateException { * Close disk IO resources associated with the underlying records queue. * This must be called when a block is no longer needed in order to prevent memory leaks. */ - public void clear() { this.recordsQueue.clear(); } + public void clear() { this.recordsQueue.close(); } + + /** + * Close disk IO resources associated with the underlying records queue. + * This must be called when a block is no longer needed in order to prevent memory leaks. + */ + @Override + public void close() { + clear(); + } } } diff --git a/src/test/java/htsjdk/samtools/SAMBinaryTagAndValueUnitTest.java b/src/test/java/htsjdk/samtools/SAMBinaryTagAndValueUnitTest.java index 06e41ee195..d202541ed3 100644 --- a/src/test/java/htsjdk/samtools/SAMBinaryTagAndValueUnitTest.java +++ b/src/test/java/htsjdk/samtools/SAMBinaryTagAndValueUnitTest.java @@ -11,19 +11,19 @@ public class SAMBinaryTagAndValueUnitTest extends HtsjdkTest { @DataProvider(name="allowedAttributeTypes") public Object[][] allowedTypes() { return new Object[][] { - {new String("a string")}, - {new Byte((byte) 7)}, - {new Short((short) 8)}, - {new Integer(0)}, - {new Character('C')}, - {new Float(0.1F)}, + {"a string"}, + {Byte.valueOf((byte) 7)}, + {Short.valueOf((short) 8)}, + {Integer.valueOf(0)}, + {Character.valueOf('C')}, + {Float.valueOf(0.1F)}, // unsigned longs - {new Long(0)}, - {new Long(BinaryCodec.MAX_UINT)}, + {Long.valueOf(0)}, + {Long.valueOf(BinaryCodec.MAX_UINT)}, // signed longs - {new Long(-1L)}, - {new Long(Integer.MAX_VALUE)}, - {new Long(Integer.MIN_VALUE)}, + {Long.valueOf(-1L)}, + {Long.valueOf(Integer.MAX_VALUE)}, + {Long.valueOf(Integer.MIN_VALUE)}, // array values {new byte[]{0, 1, 2}}, {new short[]{3, 4, 5}}, @@ -45,9 +45,9 @@ public void test_isAllowedConstructor(final Object value) { @DataProvider(name="notAllowedAttributeTypes") public Object[][] notAllowedTypes() { return new Object[][] { - {new Long(BinaryCodec.MAX_UINT + 1L)}, - {new Long(Integer.MIN_VALUE - 1L)}, - {new Double(0.3F)}, + {Long.valueOf(BinaryCodec.MAX_UINT + 1L)}, + {Long.valueOf(Integer.MIN_VALUE - 1L)}, + {Double.valueOf(0.3F)}, {new Object()}, {new Object[]{}}, {new Integer[]{}} @@ -76,7 +76,7 @@ public Object[][] allowedUnsignedArrayTypes() { @Test(dataProvider="allowedUnsignedArrayTypes") public void test_isAllowedUnsignedArrayAttribute(final Object value) { final short binaryTag = SAMTag.makeBinaryTag("UI"); - Assert.assertNotNull(new SAMBinaryTagAndUnsignedArrayValue(binaryTag, value)); + new SAMBinaryTagAndUnsignedArrayValue(binaryTag, value); } @DataProvider(name="notAllowedUnsignedArrayTypes") @@ -97,29 +97,29 @@ public void test_isNotAllowedUnsignedArrayAttribute(final Object value) { public Object[][] hashCopyEquals() { final short tag = SAMTag.makeBinaryTag("UI"); return new Object[][] { - {new SAMBinaryTagAndValue(tag, new String("a string")), new SAMBinaryTagAndValue(tag, new String("a string")), true, true}, - {new SAMBinaryTagAndValue(tag, new String("a string")), new SAMBinaryTagAndValue(tag, new String("different string")), false, false}, + {new SAMBinaryTagAndValue(tag, "a string"), new SAMBinaryTagAndValue(tag, "a string"), true, true}, + {new SAMBinaryTagAndValue(tag, "a string"), new SAMBinaryTagAndValue(tag, "different string"), false, false}, - {new SAMBinaryTagAndValue(tag, new Byte((byte) 0)), new SAMBinaryTagAndValue(tag, new Byte((byte) 0)), true, true}, - {new SAMBinaryTagAndValue(tag, new Byte((byte) 0)), new SAMBinaryTagAndValue(tag, new Byte((byte) 1)), false, false}, + {new SAMBinaryTagAndValue(tag, Byte.valueOf((byte) 0)), new SAMBinaryTagAndValue(tag, Byte.valueOf((byte) 0)), true, true}, + {new SAMBinaryTagAndValue(tag, Byte.valueOf((byte) 0)), new SAMBinaryTagAndValue(tag, Byte.valueOf((byte) 1)), false, false}, - {new SAMBinaryTagAndValue(tag, new Short((short) 0)), new SAMBinaryTagAndValue(tag, new Short((short) 0)), true, true}, - {new SAMBinaryTagAndValue(tag, new Short((short) 0)), new SAMBinaryTagAndValue(tag, new Short((short) 1)), false, false}, + {new SAMBinaryTagAndValue(tag, Short.valueOf((short) 0)), new SAMBinaryTagAndValue(tag, Short.valueOf((short) 0)), true, true}, + {new SAMBinaryTagAndValue(tag, Short.valueOf((short) 0)), new SAMBinaryTagAndValue(tag, Short.valueOf((short) 1)), false, false}, - {new SAMBinaryTagAndValue(tag, new Integer(0)), new SAMBinaryTagAndValue(tag, new Integer(0)), true, true}, - {new SAMBinaryTagAndValue(tag, new Integer(0)), new SAMBinaryTagAndValue(tag, new Integer(0)), true, true}, + {new SAMBinaryTagAndValue(tag, Integer.valueOf(0)), new SAMBinaryTagAndValue(tag, Integer.valueOf(0)), true, true}, + {new SAMBinaryTagAndValue(tag, Integer.valueOf(0)), new SAMBinaryTagAndValue(tag, Integer.valueOf(0)), true, true}, - {new SAMBinaryTagAndValue(tag, new Character('C')), new SAMBinaryTagAndValue(tag, new Character('C')), true, true}, - {new SAMBinaryTagAndValue(tag, new Character('C')), new SAMBinaryTagAndValue(tag, new Character('D')), false, false}, + {new SAMBinaryTagAndValue(tag, Character.valueOf('C')), new SAMBinaryTagAndValue(tag, Character.valueOf('C')), true, true}, + {new SAMBinaryTagAndValue(tag, Character.valueOf('C')), new SAMBinaryTagAndValue(tag, Character.valueOf('D')), false, false}, - {new SAMBinaryTagAndValue(tag,new Float(0.1F)), new SAMBinaryTagAndValue(tag, new Float(0.1F)), true, true}, - {new SAMBinaryTagAndValue(tag, new Float(0.1F)), new SAMBinaryTagAndValue(tag, new Float(0.2F)), false, false}, + {new SAMBinaryTagAndValue(tag,Float.valueOf(0.1F)), new SAMBinaryTagAndValue(tag, Float.valueOf(0.1F)), true, true}, + {new SAMBinaryTagAndValue(tag, Float.valueOf(0.1F)), new SAMBinaryTagAndValue(tag, Float.valueOf(0.2F)), false, false}, - {new SAMBinaryTagAndValue(tag,new Long(37L)), new SAMBinaryTagAndValue(tag, new Long(37L)), true, true}, - {new SAMBinaryTagAndValue(tag, new Long(37L)), new SAMBinaryTagAndValue(tag, new Long(38L)), false, false}, + {new SAMBinaryTagAndValue(tag,Long.valueOf(37L)), new SAMBinaryTagAndValue(tag, Long.valueOf(37L)), true, true}, + {new SAMBinaryTagAndValue(tag, Long.valueOf(37L)), new SAMBinaryTagAndValue(tag, Long.valueOf(38L)), false, false}, - {new SAMBinaryTagAndValue(tag,new Long(BinaryCodec.MAX_UINT)), new SAMBinaryTagAndValue(tag, new Long(BinaryCodec.MAX_UINT)), true, true}, - {new SAMBinaryTagAndValue(tag, new Long(BinaryCodec.MAX_UINT)), new SAMBinaryTagAndValue(tag, new Long(BinaryCodec.MAX_UINT-1)), false, false}, + {new SAMBinaryTagAndValue(tag,Long.valueOf(BinaryCodec.MAX_UINT)), new SAMBinaryTagAndValue(tag, Long.valueOf(BinaryCodec.MAX_UINT)), true, true}, + {new SAMBinaryTagAndValue(tag, Long.valueOf(BinaryCodec.MAX_UINT)), new SAMBinaryTagAndValue(tag, Long.valueOf(BinaryCodec.MAX_UINT-1)), false, false}, // arrays