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

directory-entry-stream read batching #142

Open
guybedford opened this issue Nov 30, 2023 · 3 comments
Open

directory-entry-stream read batching #142

guybedford opened this issue Nov 30, 2023 · 3 comments

Comments

@guybedford
Copy link
Contributor

This was originally brought up by @dbaeumer - having to have a function call per directory entry to read through the directory entry stream might introduce unnecessary performance costs for iterating large directories.

It could be useful to have the ability to fetch a chunked list by specifying a read variant that takes some kind of upper chunk limit of directory counts at a time to batch reads together.

@sunfishcode
Copy link
Member

Yes, this makes sense to look into. The current iterator API is inspired by POSIX readdir which iterates over entries one at a time, with typical implementations reading in batches under the covers. But if benchmarks show that batching would be faster, we should consider it.

@guybedford
Copy link
Contributor Author

When this was originally raised it was based on performance benchmarking, which was later clarified to not be using the Wasmtime compliation cache when running the component benchmarks - when enabled the benchmarks demonstrated equality with preview1 and less call overhead.

@dbaeumer do you think this is something that should still be a priority for consideration to provide a noticeable performance improvement?

@dbaeumer
Copy link

dbaeumer commented Jan 9, 2024

@guybedford for VS Code this is not necessary anymore. The performance problem raised from the fact that we add to do a context switch for every read. I decided to but the result in shared memory to avoid such a context switch.

However other implementation might hit the same problem. But from the VS Code side this can be closed

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

No branches or pull requests

3 participants