From a358d0c53ba3ba89b81ac955aaf42e0ac196e8ed Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Wed, 25 Sep 2019 19:01:26 +0200 Subject: [PATCH] reftable: move AutoCloseable to ReftableReader MergedReftable is not used as an AutoCloseable, because closing tables is currently handled by DfsReftableStack#close. Encode that a MergedReftable is a list of ReftableReaders. The previous code suggested that we could form nested trees of MergedReftables, which is not how we use reftables. Change-Id: Icbe2fee8a5a12373f45fc5f97d8b1a2b14231c96 Signed-off-by: Han-Wen Nienhuys --- .../storage/reftable/MergedReftableTest.java | 2 +- .../internal/storage/dfs/DfsReftableStack.java | 10 +++++----- .../storage/dfs/ReftableBatchRefUpdate.java | 8 ++++---- .../storage/reftable/MergedReftable.java | 18 ++++-------------- .../internal/storage/reftable/Reftable.java | 6 +----- .../storage/reftable/ReftableCompactor.java | 12 +++++------- .../storage/reftable/ReftableReader.java | 2 +- 7 files changed, 21 insertions(+), 37 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/MergedReftableTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/MergedReftableTest.java index dd42aa0b38..c20db7b96b 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/MergedReftableTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/MergedReftableTest.java @@ -414,7 +414,7 @@ public class MergedReftableTest { } private static MergedReftable merge(byte[]... table) { - List stack = new ArrayList<>(table.length); + List stack = new ArrayList<>(table.length); for (byte[] b : table) { stack.add(read(b)); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReftableStack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReftableStack.java index b6b540388a..59621a4e67 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReftableStack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsReftableStack.java @@ -48,11 +48,11 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import org.eclipse.jgit.internal.storage.reftable.Reftable; +import org.eclipse.jgit.internal.storage.reftable.ReftableReader; /** * Tracks multiple open - * {@link org.eclipse.jgit.internal.storage.reftable.Reftable} instances. + * {@link org.eclipse.jgit.internal.storage.reftable.ReftableReader} instances. */ public class DfsReftableStack implements AutoCloseable { /** @@ -86,7 +86,7 @@ public class DfsReftableStack implements AutoCloseable { } private final List files; - private final List tables; + private final List tables; private DfsReftableStack(int tableCnt) { this.files = new ArrayList<>(tableCnt); @@ -109,14 +109,14 @@ public class DfsReftableStack implements AutoCloseable { * @return unmodifiable list of tables, in the same order the files were * passed to {@link #open(DfsReader, List)}. */ - public List readers() { + public List readers() { return Collections.unmodifiableList(tables); } /** {@inheritDoc} */ @Override public void close() { - for (Reftable t : tables) { + for (ReftableReader t : tables) { try { t.close(); } catch (IOException e) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/ReftableBatchRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/ReftableBatchRefUpdate.java index 364052bf93..b19ffffba9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/ReftableBatchRefUpdate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/ReftableBatchRefUpdate.java @@ -406,7 +406,7 @@ public class ReftableBatchRefUpdate extends BatchRefUpdate { private boolean canCompactTopOfStack(ReftableConfig cfg) throws IOException { DfsReftableStack stack = refdb.stack(); - List readers = stack.readers(); + List readers = stack.readers(); if (readers.isEmpty()) { return false; } @@ -427,10 +427,10 @@ public class ReftableBatchRefUpdate extends BatchRefUpdate { private ReftableWriter.Stats compactTopOfStack(OutputStream out, ReftableConfig cfg, byte[] newTable) throws IOException { - List stack = refdb.stack().readers(); - Reftable last = stack.get(stack.size() - 1); + List stack = refdb.stack().readers(); + ReftableReader last = stack.get(stack.size() - 1); - List tables = new ArrayList<>(2); + List tables = new ArrayList<>(2); tables.add(last); tables.add(new ReftableReader(BlockSource.from(newTable))); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/MergedReftable.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/MergedReftable.java index f5d612f3e1..a4394066ea 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/MergedReftable.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/MergedReftable.java @@ -67,13 +67,11 @@ import org.eclipse.jgit.lib.ReflogEntry; * A {@code MergedReftable} is not thread-safe. */ public class MergedReftable extends Reftable { - private final Reftable[] tables; + private final ReftableReader[] tables; /** * Initialize a merged table reader. *

- * The tables in {@code tableStack} will be closed when this - * {@code MergedReftable} is closed. * * @param tableStack * stack of tables to read from. The base of the stack is at @@ -81,12 +79,12 @@ public class MergedReftable extends Reftable { * {@code tableStack.size() - 1}. The top of the stack (higher * index) shadows the base of the stack (lower index). */ - public MergedReftable(List tableStack) { - tables = tableStack.toArray(new Reftable[0]); + public MergedReftable(List tableStack) { + tables = tableStack.toArray(new ReftableReader[0]); // Tables must expose deletes to this instance to correctly // shadow references from lower tables. - for (Reftable t : tables) { + for (ReftableReader t : tables) { t.setIncludeDeletes(true); } } @@ -161,14 +159,6 @@ public class MergedReftable extends Reftable { return m; } - /** {@inheritDoc} */ - @Override - public void close() throws IOException { - for (Reftable t : tables) { - t.close(); - } - } - int queueSize() { return Math.max(1, tables.length); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/Reftable.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/Reftable.java index 860d044262..8f64261766 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/Reftable.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/Reftable.java @@ -58,7 +58,7 @@ import org.eclipse.jgit.lib.SymbolicRef; /** * Abstract table of references. */ -public abstract class Reftable implements AutoCloseable { +public abstract class Reftable { /** * References to convert into a reftable * @@ -295,8 +295,4 @@ public abstract class Reftable implements AutoCloseable { } return new SymbolicRef(ref.getName(), dst, ref.getUpdateIndex()); } - - /** {@inheritDoc} */ - @Override - public abstract void close() throws IOException; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableCompactor.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableCompactor.java index 2effda4bcb..c73c245be5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableCompactor.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableCompactor.java @@ -69,7 +69,7 @@ import org.eclipse.jgit.lib.ReflogEntry; */ public class ReftableCompactor { private final ReftableWriter writer; - private final ArrayDeque tables = new ArrayDeque<>(); + private final ArrayDeque tables = new ArrayDeque<>(); private long compactBytesLimit; private long bytesToCompact; @@ -188,12 +188,10 @@ public class ReftableCompactor { * @throws java.io.IOException * update indexes of a reader cannot be accessed. */ - public void addAll(List readers) throws IOException { - tables.addAll(readers); - for (Reftable r : readers) { - if (r instanceof ReftableReader) { - adjustUpdateIndexes((ReftableReader) r); - } + public void addAll(List readers) throws IOException { + for (ReftableReader r : readers) { + tables.add(r); + adjustUpdateIndexes(r); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableReader.java index 75e98958f4..c5e667449a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableReader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableReader.java @@ -78,7 +78,7 @@ import org.eclipse.jgit.util.NB; * {@code ReftableReader} is not thread-safe. Concurrent readers need their own * instance to read from the same file. */ -public class ReftableReader extends Reftable { +public class ReftableReader extends Reftable implements AutoCloseable { private final BlockSource src; private int blockSize = -1; -- 2.39.5