Skip to content
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.

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

FileService.readFile reads whole file instead of only the requested part (inefficient in general, even fails for >2GB files) #13161

Closed
dfriederich opened this issue Dec 7, 2023 · 1 comment · Fixed by #13197
Assignees
Labels
filesystem issues related to the filesystem performance issues related to performance
Milestone

Comments

@dfriederich
Copy link
Contributor

When using FileService.readFile to only read a part of a file ({ length: 2 } option), the API still reads in the full file and then only returns the requested bytes.
This did cause in my usage to detect the file type that it failed for files > 2GB unnecessarily.
But even for files < 2GB, silently reading in much more than was requested is quite inefficient. The handling is according tot he source comment, and optimization. Maybe it should not happen if a length is requested?

Not a big problem for me, I'm not using as workaround FileService.readFileStream
`// Workaround for FileService.readFile to just read the file unbuffered as optimization.
// This breaks to read the first 2 bytes of a > 2GB file as in that code flow it reads the whole monster and then cuts
// afterwards. The workaround bypasses that code and reads the file as a stream.

async function readFileBuffered(fileService: FileService, resource: URI, options?: ReadFileOptions): Promise {
const stream = await fileService.readFileStream(resource, options);
return {
...stream,
value: await BinaryBufferReadableStream.toBuffer(stream.value)
};
}
`

but through still worth reporting as issue to possibly improve.

@msujew msujew added filesystem issues related to the filesystem performance issues related to performance labels Dec 7, 2023
@martin-fleck-at
Copy link
Contributor

@dfriederich Thank you for raising this, I'll have a look at this soon as it seems quite the loss of performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
filesystem issues related to the filesystem performance issues related to performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants