Skip to content

Fix working with large files in VFS #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
msimacek wants to merge 1 commit into
base: main
Choose a base branch
from
+86 −52
Open

Fix working with large files in VFS #49

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 86 additions & 52 deletions ...vm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@
import org.graalvm.python.embedding.VirtualFileSystem.HostIO;

import java.io.BufferedReader;
import java.io.ByteArrayOutputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.lang.invoke.VarHandle;
import java.io.OutputStream;
import java.net.URI;
import java.net.URL;
import java.nio.ByteBuffer;
import java.nio.channels.ClosedChannelException;
import java.nio.channels.NonWritableChannelException;
import java.nio.channels.SeekableByteChannel;
import java.nio.charset.Charset;
Expand Down Expand Up @@ -177,20 +177,41 @@ static AssertionError throwUnexpectedSubclass() {
}

private final class FileEntry extends BaseEntry {
private byte[] data;
private List<FileEntry> toExtract;

public FileEntry(String path) {
super(path);
}

private byte[] getData() throws IOException {
if (data == null) {
byte[] loaded = readResource(getResourcePath());
VarHandle.storeStoreFence();
data = loaded;
private InputStream openStream() throws IOException {
return getResourceUrl(getResourcePath()).openStream();
}

private byte[] readAllBytes() throws IOException {
try (InputStream stream = openStream()) {
return stream.readAllBytes();
}
}

private long getSize() throws IOException {
long size = 0;
byte[] tmp = new byte[1024];
try (InputStream stream = openStream()) {
while (true) {
int read = stream.read(tmp);
if (read < 0) {
return size;
}
size += read;
}
}
}

private void extractTo(Path target) throws IOException {
try (OutputStream is = Files.newOutputStream(target);
InputStream os = openStream()) {
os.transferTo(is);
}
return data;
}
}

Expand Down Expand Up @@ -596,7 +617,7 @@ private void checkPlatform() {
assert contentsEntry instanceof FileEntry;
String contents;
try {
contents = new String(((FileEntry) contentsEntry).getData(), StandardCharsets.UTF_8);
contents = new String(((FileEntry) contentsEntry).readAllBytes(), StandardCharsets.UTF_8);
} catch (IOException ex) {
throw new IllegalStateException(
String.format("IO error while reading venv contents file'%s'.", contentsPath), ex);
Expand Down Expand Up @@ -839,23 +860,6 @@ private URL getResourceUrl(String path) throws IOException {
return urls.get(0);
}

byte[] readResource(String path) throws IOException {
try (InputStream stream = getResourceUrl(path).openStream()) {
if (stream == null) {
throw new IllegalStateException("VFS.initEntries: could not read resource: " + path);
}
ByteArrayOutputStream buffer = new ByteArrayOutputStream();
int n;
byte[] data = new byte[4096];

while ((n = stream.readNBytes(data, 0, data.length)) != 0) {
buffer.write(data, 0, n);
}
buffer.flush();
return buffer.toByteArray();
}
}

private List<URL> getURLInRoot(List<URL> urls) {
return urls.stream().filter(x -> x.toString().startsWith(vfsRootURL)).findFirst().map(List::of)
.orElseGet(List::of);
Expand Down Expand Up @@ -951,7 +955,7 @@ private Path extractSingleFile(FileEntry toExtract) throws IOException {
Files.createDirectories(parent);

// write data extracted file
Files.write(extractedPath, toExtract.getData());
toExtract.extractTo(extractedPath);
finest("extracted '%s' -> '%s'", toExtract.getPlatformPath(), extractedPath);
}
return extractedPath;
Expand All @@ -968,14 +972,15 @@ void extractResources(Path externalResourceDirectory) throws IOException {
Path destFile = externalResourceDirectory.resolve(Path.of(resourcePath.substring(vfsRoot.length() + 2)));
if (entry instanceof DirEntry) {
Files.createDirectories(destFile);
} else {
assert entry instanceof FileEntry;
} else if (entry instanceof FileEntry fileEntry) {
Path parent = destFile.getParent();
if (parent != null) {
Files.createDirectories(parent);
}
finest("VFS.extractResources '%s' -> '%s'", resourcePath, destFile);
Files.write(destFile, readResource(resourcePath));
fileEntry.extractTo(destFile);
} else {
throw new IllegalStateException("Unexpected entry type: " + entry);
}
}
}
Expand Down Expand Up @@ -1105,64 +1110,93 @@ public SeekableByteChannel newByteChannel(Path p, Set<? extends OpenOption> opti
// appropriate python error
throw new FileSystemException(path.toString(), null, "Is a directory");
}
byte[] data = fileEntry.getData(); // throw the IOException before allocating SeekableByteChannel
return new SeekableByteChannel() {
long position = 0;
private long position = 0;
private long size = -1;
private InputStream is = fileEntry.openStream();

final byte[] bytes = data;
private void ensureOpen() throws IOException {
if (!isOpen()) {
throw new ClosedChannelException();
}
}

@Override
public int read(ByteBuffer dst) throws IOException {
if (position > bytes.length) {
return -1;
} else if (position == bytes.length) {
return 0;
ensureOpen();
int read;
if (dst.hasArray()) {
read = is.read(dst.array(), dst.arrayOffset() + dst.position(), dst.remaining());
if (read > 0) {
dst.position(dst.position() + read);
position += read;
}
} else {
int length = Math.min(bytes.length - (int) position, dst.remaining());
dst.put(bytes, (int) position, length);
position += length;
if (dst.hasRemaining()) {
position++;
byte[] tmp = new byte[dst.remaining()];
read = is.read(tmp);
if (read > 0) {
dst.put(tmp, 0, read);
position += read;
}
return length;
}
return read;
}

@Override
public int write(ByteBuffer src) throws IOException {
public int write(ByteBuffer src) {
finer("VFS.newByteChannel '%s'", String.format("read-only filesystem: '%s'", path));
throw new NonWritableChannelException();
}

@Override
public long position() throws IOException {
public long position() {
return position;
}

@Override
public SeekableByteChannel position(long newPosition) throws IOException {
position = Math.max(0, newPosition);
ensureOpen();
if (newPosition < 0) {
throw new IllegalArgumentException();
}
if (newPosition > position) {
is.skip(newPosition - position);
position = newPosition;
} else if (newPosition < position) {
is.close();
is = fileEntry.openStream();
is.skip(newPosition);
position = newPosition;
}
return this;
}

@Override
public long size() throws IOException {
return bytes.length;
ensureOpen();
if (size == -1) {
size = fileEntry.getSize();
}
return size;
}

@Override
public SeekableByteChannel truncate(long size) throws IOException {
public SeekableByteChannel truncate(long size) {
finer("VFS.newByteChannel '%s'", String.format("read-only filesystem: '%s'", path));
throw new NonWritableChannelException();
}

@Override
public boolean isOpen() {
return true;
return is != null;
}

@Override
public void close() throws IOException {
if (is != null) {
is.close();
is = null;
}
}
};
}
Expand Down Expand Up @@ -1303,7 +1337,7 @@ public Map<String, Object> readAttributes(Path p, String attributes, LinkOption.
attrs.put("isDirectory", entry instanceof DirEntry);
attrs.put("isSymbolicLink", extractable);
attrs.put("isOther", false);
attrs.put("size", (long) (entry instanceof FileEntry fileEntry ? fileEntry.getData().length : 0));
attrs.put("size", entry instanceof FileEntry fileEntry ? fileEntry.getSize() : 0L);
attrs.put("mode", 0555);
attrs.put("dev", 0L);
attrs.put("nlink", 1);
Expand Down Expand Up @@ -1415,7 +1449,7 @@ private long getEntryTotalSpace(BaseEntry entry) throws IOException {
try {
long size = entry.getResourcePath().length() + 1;
if (entry instanceof FileEntry fe) {
size = Math.addExact(size, fe.getData().length);
size = Math.addExact(size, fe.getSize());
} else if (entry instanceof DirEntry de) {
for (BaseEntry e : de.entries) {
size = Math.addExact(size, getEntryTotalSpace(e));
Expand Down