From: Tuukka Lehtonen Date: Thu, 5 Apr 2018 13:33:58 +0000 (+0300) Subject: Finalizing improve startup time for fresh or rollback'd session X-Git-Tag: v1.43.0~136^2~504 X-Git-Url: https://gerrit.simantics.org/r/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F29%2F1629%2F5;p=simantics%2Fplatform.git Finalizing improve startup time for fresh or rollback'd session Finally fixed the long-sought InputStream reading bug from TransferableGraphFileReader.InputChannel.read(ByteBuffer). The ByteFileReader implementation expected the provided ReadableByteChannel to always provide a full buffer of data unless EOF was encountered and the InputChannel.read implementation was not taking care of that. Also added simpler utilities for reading TG's from files/InputStreams directly using static TransferableGraphFileReader.read(File/InputStream) methods. PlatformUtil.readTG can now finally use TransferableGraphFileReader for maximal performance. refs #7835 Change-Id: I170986bf271a409f9205fad808ae029becc1d3a4 --- diff --git a/bundles/org.simantics.db.layer0/src/org/simantics/db/layer0/migration/MigrationStateImpl.java b/bundles/org.simantics.db.layer0/src/org/simantics/db/layer0/migration/MigrationStateImpl.java index d7f6b4e95..f50bae78a 100644 --- a/bundles/org.simantics.db.layer0/src/org/simantics/db/layer0/migration/MigrationStateImpl.java +++ b/bundles/org.simantics.db.layer0/src/org/simantics/db/layer0/migration/MigrationStateImpl.java @@ -106,12 +106,10 @@ public class MigrationStateImpl implements MigrationState { return (T)tg; } - TransferableGraphFileReader reader = null; try { File modelFile = getProperty(MigrationStateKeys.MODEL_FILE); - reader = new TransferableGraphFileReader(modelFile); TimeLogger.log(MigrationStateImpl.class, "reading TG into memory from " + modelFile); - TransferableGraph1 tg = reader.readTG(); + TransferableGraph1 tg = TransferableGraphFileReader.read(modelFile); TimeLogger.log(MigrationStateImpl.class, "read TG into memory from " + modelFile); setProperty(MigrationStateKeys.CURRENT_TG, tg); return (T)tg; @@ -119,8 +117,6 @@ public class MigrationStateImpl implements MigrationState { throw e; } catch (Throwable t) { throw new DatabaseException(t); - } finally { - uncheckedClose(reader); } } else if (MigrationStateKeys.CURRENT_TGS.equals(key)) { diff --git a/bundles/org.simantics.db.testing/src/org/simantics/db/testing/common/TestingUtils.java b/bundles/org.simantics.db.testing/src/org/simantics/db/testing/common/TestingUtils.java index 819be7807..8f3a8463c 100644 --- a/bundles/org.simantics.db.testing/src/org/simantics/db/testing/common/TestingUtils.java +++ b/bundles/org.simantics.db.testing/src/org/simantics/db/testing/common/TestingUtils.java @@ -25,9 +25,7 @@ public class TestingUtils { public static Resource importModel(final Resource toLocation, File fromFile, String withName) throws Exception { - TransferableGraphFileReader importer = new TransferableGraphFileReader(fromFile); - TransferableGraph1 tg = importer.readTG(); - importer.close(); + TransferableGraph1 tg = TransferableGraphFileReader.read(fromFile); final DefaultPasteImportAdvisor advisor = new DefaultPasteImportAdvisor(toLocation) { @Override diff --git a/bundles/org.simantics.graph/src/org/simantics/graph/representation/TransferableGraph1Serializer.java b/bundles/org.simantics.graph/src/org/simantics/graph/representation/TransferableGraph1Serializer.java index 770d6c6ca..0baa40231 100644 --- a/bundles/org.simantics.graph/src/org/simantics/graph/representation/TransferableGraph1Serializer.java +++ b/bundles/org.simantics.graph/src/org/simantics/graph/representation/TransferableGraph1Serializer.java @@ -108,21 +108,11 @@ public class TransferableGraph1Serializer extends Serializer { @Override public Object deserialize(File file) throws IOException { - TransferableGraphFileReader reader = new TransferableGraphFileReader(file); - try { - return reader.readTG(); - } finally { - reader.close(); - } + return TransferableGraphFileReader.read(file); } public Object deserialize(InputStream in) throws IOException { - TransferableGraphFileReader reader = new TransferableGraphFileReader(in); - try { - return reader.readTG(); - } finally { - reader.close(); - } + return TransferableGraphFileReader.read(in); } @Override diff --git a/bundles/org.simantics.graph/src/org/simantics/graph/representation/TransferableGraphFileReader.java b/bundles/org.simantics.graph/src/org/simantics/graph/representation/TransferableGraphFileReader.java index 077eb6617..56a112e4d 100644 --- a/bundles/org.simantics.graph/src/org/simantics/graph/representation/TransferableGraphFileReader.java +++ b/bundles/org.simantics.graph/src/org/simantics/graph/representation/TransferableGraphFileReader.java @@ -14,6 +14,7 @@ import org.simantics.databoard.Bindings; import org.simantics.databoard.Datatypes; import org.simantics.databoard.binding.error.RuntimeDatatypeConstructionException; import org.simantics.databoard.binding.mutable.Variant; +import org.simantics.databoard.container.DataContainer; import org.simantics.databoard.container.DataContainers; import org.simantics.databoard.serialization.RuntimeSerializerConstructionException; import org.simantics.databoard.serialization.Serializer; @@ -22,6 +23,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** + * It is recommended to use {@link #read(File)} and {@link #read(InputStream)} + * for reading to ensure proper resource handling. + */ final public class TransferableGraphFileReader extends ByteFileReader { private static final Logger LOGGER = LoggerFactory.getLogger(TransferableGraphFileReader.class); @@ -53,11 +58,11 @@ final public class TransferableGraphFileReader extends ByteFileReader { final static class InputChannel implements ReadableByteChannel { final private InputStream stream; - + public InputChannel(InputStream stream) { this.stream = stream; } - + @Override public boolean isOpen() { return true; @@ -65,17 +70,25 @@ final public class TransferableGraphFileReader extends ByteFileReader { @Override public void close() throws IOException { + // NOTE: it is an intentional choice not to close the underlying stream here + // InputStreams given directly to TransferableGraphFileReader are expected to + // be closed outside of this implementation. } @Override public int read(ByteBuffer dst) throws IOException { - int pos = dst.position(); + int nRead; + int size = 0; + int position = dst.position(); int limit = dst.limit(); - int i=stream.read(dst.array(), pos, limit-pos); - //LOGGER.warn("Read " + i + " (expected " + dst.array().length + ")"); - return i; + // The users of this channel expect that the data is fully read at this point + while ((nRead = stream.read(dst.array(), position, limit - position)) != -1 && limit - position > 0) { + size += nRead; + position += nRead; + } + return size; } - + } private static boolean init = true; @@ -83,7 +96,37 @@ final public class TransferableGraphFileReader extends ByteFileReader { final private static int SIZE = 1<<18; final private static int HEADER = headerSize(); final private int header; - + + /** + * Reads a {@link DataContainer} containing a {@link TransferableGraph1} + * structure from the specified {@link File}. + * + * @param file the file to read from + * @return the TG contained by the file + * @throws IOException + */ + public static TransferableGraph1 read(File file) throws IOException { + try (TransferableGraphFileReader reader = new TransferableGraphFileReader(file)) { + return reader.readTG(); + } + } + + /** + * Reads a {@link DataContainer} containing a {@link TransferableGraph1} + * structure from the specified InputStream. Note that this implementation does + * not close the specified input stream, it is expected to be + * closed by the caller. + * + * @param input the input stream to read from + * @return the TG contained by the stream + * @throws IOException + */ + public static TransferableGraph1 read(InputStream input) throws IOException { + try (TransferableGraphFileReader reader = new TransferableGraphFileReader(input)) { + return reader.readTG(); + } + } + public TransferableGraphFileReader(File file) throws IOException { super(file, SIZE); if(init) { @@ -263,7 +306,7 @@ final public class TransferableGraphFileReader extends ByteFileReader { for(int i=0;i FORMAT_HANDLER = new FormatHandler() { - @Override - public Binding getBinding() { - return TransferableGraph1.BINDING; - } - @Override - public TransferableGraph1 process(DataContainer container) throws Exception { - return (TransferableGraph1) container.content.getValue(TransferableGraph1.BINDING); - } - }; - - @SuppressWarnings("unchecked") - private static Map> handlers = ArrayMap.make( - new String[] { - "graph:1", - "sharedLibrary:1" - }, - FORMAT_HANDLER, - FORMAT_HANDLER); - - private static TransferableGraph1 readTG(InputStream is) throws Exception { - // For an unknown reason this is totally broken when running the TestSCLOsgi - // in the SDK Tycho build. It returns incomplete results because the - // ReadableByteChannel used by ByteFileReader starts returning 0 unexpectedly. -// try (TransferableGraphFileReader reader = new TransferableGraphFileReader(is)) { -// return reader.readTG(); -// } - return DataContainers.readFile(new DataInputStream(is), handlers); - } private static TransferableGraph1 readTG(URL url) throws Exception { try (InputStream is = url.openStream()) { - return readTG(is); + return TransferableGraphFileReader.read(is); } }