From a52e828b6162c04d509181d219d2dccde541286e Mon Sep 17 00:00:00 2001 From: "Ilya.Kazakevich" Date: Fri, 6 Dec 2024 19:47:09 +0100 Subject: [PATCH] Eel: Introduce `ReadResult` to reuse it all over the system. Files, sockets, and process pipes all support reading. Read's result is either `EOF` or some bytes written to a buffer provided to `read`. APIs usually return a number of bytes read with `-1` means EOF, but this information is redundant as it can always be deduced from `ByteBuffer` position. As we do not need this number, we introduce enum which is simpler (and faster) than sealed class and also removes possible inconsistency between number of bytes and buffer position advance. GitOrigin-RevId: cffb7b5121f05c40aec186019ee7c3b0fe400d16 --- .../platform/eel/impl/fs/EelFsResultImpl.kt | 3 -- .../eel/impl/fs/LocalEelFileSystemApi.kt | 19 +++++---- .../com/intellij/platform/eel/ReadResult.kt | 40 +++++++++++++++++++ .../platform/eel/fs/EelFileSystemApi.kt | 15 +++---- .../community/impl/nio/IjentNioFileChannel.kt | 21 +++++----- 5 files changed, 66 insertions(+), 32 deletions(-) create mode 100644 platform/eel/src/com/intellij/platform/eel/ReadResult.kt diff --git a/platform/eel-impl/src/com/intellij/platform/eel/impl/fs/EelFsResultImpl.kt b/platform/eel-impl/src/com/intellij/platform/eel/impl/fs/EelFsResultImpl.kt index b88f5591ed15..74214bd9eb48 100644 --- a/platform/eel-impl/src/com/intellij/platform/eel/impl/fs/EelFsResultImpl.kt +++ b/platform/eel-impl/src/com/intellij/platform/eel/impl/fs/EelFsResultImpl.kt @@ -13,9 +13,6 @@ object EelFsResultImpl { data class Ok(override val value: T) : EelResult.Ok data class Error(override val error: E) : EelResult.Error - data class BytesReadImpl(override val bytesRead: Int) : EelOpenedFile.Reader.ReadResult.Bytes - data object EOFImpl : EelOpenedFile.Reader.ReadResult.EOF - data class DiskInfoImpl(override val totalSpace: ULong, override val availableSpace: ULong) : EelFileSystemApi.DiskInfo data class FullBytesReadImpl(override val bytes: ByteArray) : EelFileSystemApi.FullReadResult.Bytes diff --git a/platform/eel-impl/src/com/intellij/platform/eel/impl/fs/LocalEelFileSystemApi.kt b/platform/eel-impl/src/com/intellij/platform/eel/impl/fs/LocalEelFileSystemApi.kt index 886b637d96eb..3c0869110820 100644 --- a/platform/eel-impl/src/com/intellij/platform/eel/impl/fs/LocalEelFileSystemApi.kt +++ b/platform/eel-impl/src/com/intellij/platform/eel/impl/fs/LocalEelFileSystemApi.kt @@ -5,6 +5,7 @@ import com.intellij.openapi.util.SystemInfoRt import com.intellij.platform.eel.EelResult import com.intellij.platform.eel.EelUserPosixInfo import com.intellij.platform.eel.EelUserWindowsInfo +import com.intellij.platform.eel.ReadResult import com.intellij.platform.eel.fs.* import com.intellij.platform.eel.fs.EelFileSystemApi.FileWriterCreationMode.* import com.intellij.platform.eel.path.EelPath @@ -157,13 +158,13 @@ abstract class NioBasedEelFileSystemApi(@VisibleForTesting val fs: FileSystem) : nioOptions += StandardOpenOption.READ val byteChannel: SeekableByteChannel = nioPath.fileSystem.provider().newByteChannel(nioPath, nioOptions) object : EelOpenedFile.ReaderWriter, EelOpenedFile.Writer by LocalEelOpenedFileWriter(this, byteChannel, path) { - override suspend fun read(buf: ByteBuffer): EelResult = + override suspend fun read(buf: ByteBuffer): EelResult = doRead(this@NioBasedEelFileSystemApi, byteChannel, buf) override suspend fun read( buf: ByteBuffer, offset: Long, - ): EelResult = + ): EelResult = doRead(this@NioBasedEelFileSystemApi, byteChannel, offset, buf) } } @@ -208,11 +209,11 @@ private class LocalEelOpenedFileReader( private val byteChannel: SeekableByteChannel, private val path_: EelPath.Absolute, ) : EelOpenedFile.Reader { - override suspend fun read(buf: ByteBuffer): EelResult = + override suspend fun read(buf: ByteBuffer): EelResult = doRead(eelFs, byteChannel, buf) override suspend fun read(buf: ByteBuffer, offset: Long): EelResult< - EelOpenedFile.Reader.ReadResult, + ReadResult, EelOpenedFile.Reader.ReadError > = doRead(eelFs, byteChannel, offset, buf) @@ -279,12 +280,11 @@ private fun doRead( eelFs: NioBasedEelFileSystemApi, byteChannel: SeekableByteChannel, buf: ByteBuffer, -): EelResult = +): EelResult = eelFs.wrapIntoEelResult { val read = byteChannel.read(buf) - if (read >= 0) EelFsResultImpl.BytesReadImpl(read) - else EelFsResultImpl.EOFImpl + ReadResult.fromNumberOfReadBytes(read) } private fun doRead( @@ -292,15 +292,14 @@ private fun doRead( byteChannel: SeekableByteChannel, offset: Long, buf: ByteBuffer, -): EelResult = +): EelResult = eelFs.wrapIntoEelResult { val oldPosition = byteChannel.position() byteChannel.position(offset) val read = byteChannel.read(buf) byteChannel.position(oldPosition) - if (read >= 0) EelFsResultImpl.BytesReadImpl(read) - else EelFsResultImpl.EOFImpl + ReadResult.fromNumberOfReadBytes(read) } private fun doSeek( diff --git a/platform/eel/src/com/intellij/platform/eel/ReadResult.kt b/platform/eel/src/com/intellij/platform/eel/ReadResult.kt new file mode 100644 index 000000000000..e9cb1d0a49b8 --- /dev/null +++ b/platform/eel/src/com/intellij/platform/eel/ReadResult.kt @@ -0,0 +1,40 @@ +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.platform.eel + +import com.intellij.platform.eel.ReadResult.EOF +import com.intellij.platform.eel.ReadResult.NOT_EOF + + +/** + * When reading from a channel/file/socket/pipe/stream, you might end up with: + * * [EOF]: "no data was read" because you've reached the end: semantics is barely the same as `-1` in many APIs. + * * [NOT_EOF]: some data might be read because it isn't the end (yet). + * + * To see how much bytes were read, compare [java.nio.ByteBuffer.position] with the one you had before read, i.e: + * ```kotlin + * var before = 0 + * while(file.read(buffer) != EOF) { + * println("I read ${buffer.position() - before} bytes") + * before = buffer.position() + * assert(buffer.hasRemaining) {"Oops, the buffer is full"} + * } + * ``` + */ +enum class ReadResult { + EOF, + NOT_EOF; + + companion object { + /** + * ```kotlin + * fromNumberOfReadBytes(stream.read(buffer)) + * ``` + */ + fun fromNumberOfReadBytes(bytesRead: Int): ReadResult = if (bytesRead < -1) { + throw IllegalArgumentException("Number of bytes read must be in -1..INT_MAX, can't be $bytesRead") + } + else { + if (bytesRead == -1) EOF else NOT_EOF + } + } +} \ No newline at end of file diff --git a/platform/eel/src/com/intellij/platform/eel/fs/EelFileSystemApi.kt b/platform/eel/src/com/intellij/platform/eel/fs/EelFileSystemApi.kt index c029de287704..cae381ecaa5f 100644 --- a/platform/eel/src/com/intellij/platform/eel/fs/EelFileSystemApi.kt +++ b/platform/eel/src/com/intellij/platform/eel/fs/EelFileSystemApi.kt @@ -5,6 +5,7 @@ import com.intellij.platform.eel.EelResult import com.intellij.platform.eel.EelUserInfo import com.intellij.platform.eel.EelUserPosixInfo import com.intellij.platform.eel.EelUserWindowsInfo +import com.intellij.platform.eel.ReadResult import com.intellij.platform.eel.fs.EelFileSystemApi.StatError import com.intellij.platform.eel.path.EelPath import org.jetbrains.annotations.CheckReturnValue @@ -481,9 +482,10 @@ sealed interface EelOpenedFile { /** * Reads data from the current position of the file (see [tell]) * - * If the remote file is read completely, then this function returns [ReadResult] with [ReadResult.EOF]. - * Otherwise, if there are any data left to read, then it returns [ReadResult.Bytes]. - * Note, that [ReadResult.Bytes] can be `0` if [buf] cannot accept new data. + * If the remote file is read completely, + * then this function returns [ReadResult] with [ReadResult.EOF]. + * Otherwise, if there are any data left to read, then it returns [ReadResult.NOT_EOF]. + * See [ReadResult] for usage receipts. * * This operation modifies the file's cursor, i.e. [tell] may show different results before and after this function is invoked. * @@ -502,13 +504,6 @@ sealed interface EelOpenedFile { @CheckReturnValue suspend fun read(buf: ByteBuffer, offset: Long): EelResult - sealed interface ReadResult { - interface EOF : ReadResult - interface Bytes : ReadResult { - val bytesRead: Int - } - } - sealed interface ReadError : EelFsError { interface UnknownFile : ReadError, EelFsError.UnknownFile interface InvalidValue : ReadError, EelFsError diff --git a/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioFileChannel.kt b/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioFileChannel.kt index 45b373887e4e..edfc56371dcf 100644 --- a/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioFileChannel.kt +++ b/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioFileChannel.kt @@ -3,6 +3,7 @@ package com.intellij.platform.ijent.community.impl.nio import com.intellij.openapi.diagnostic.logger import com.intellij.openapi.diagnostic.thisLogger +import com.intellij.platform.eel.ReadResult import com.intellij.platform.eel.fs.EelFileInfo import com.intellij.platform.eel.fs.EelFileSystemApi import com.intellij.platform.eel.fs.EelOpenedFile @@ -61,9 +62,10 @@ internal class IjentNioFileChannel private constructor( var totalRead = 0L fsBlocking { handleThatSmartMultiBufferApi(dsts, offset, length) { buf -> - val read = when (val res = ijentOpenedFile.read(buf).getOrThrowFileSystemException()) { - is EelOpenedFile.Reader.ReadResult.Bytes -> res.bytesRead - is EelOpenedFile.Reader.ReadResult.EOF -> return@fsBlocking + val before = buf.position() + val read = when (ijentOpenedFile.read(buf).getOrThrowFileSystemException()) { + ReadResult.NOT_EOF -> buf.position() - before + ReadResult.EOF -> return@fsBlocking } totalRead += read } @@ -233,6 +235,7 @@ internal class IjentNioFileChannel private constructor( is EelOpenedFile.Reader -> Unit is EelOpenedFile.Writer -> throw NonReadableChannelException() } + val before = dst.position() val readResult = fsBlocking { if (position == null) { ijentOpenedFile.read(dst) @@ -242,8 +245,8 @@ internal class IjentNioFileChannel private constructor( } }.getOrThrowFileSystemException() return when (readResult) { - is EelOpenedFile.Reader.ReadResult.Bytes -> readResult.bytesRead - is EelOpenedFile.Reader.ReadResult.EOF -> -1 + ReadResult.NOT_EOF -> dst.position() - before + ReadResult.EOF -> -1 } } @@ -335,14 +338,14 @@ internal class IjentNioFileChannel private constructor( // There are classes like `jdk.internal.jimage.BasicImageReader` that create a memory map and keep reading the file // with usual methods. // The current position in the file should remain the same after the copying. - when (val r = ijentOpenedFile.read(buffer, position).getOrThrowFileSystemException()) { - is EelOpenedFile.Reader.ReadResult.Bytes -> { - position += r.bytesRead + when (ijentOpenedFile.read(buffer, position).getOrThrowFileSystemException()) { + ReadResult.NOT_EOF -> { + position += buffer.position() buffer.flip() outputChannel.write(buffer) buffer.clear() } - is EelOpenedFile.Reader.ReadResult.EOF -> break + ReadResult.EOF -> break } } }