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

reduce readFileSync syscall count #23887

Open
littledivy opened this issue May 19, 2024 · 2 comments
Open

reduce readFileSync syscall count #23887

littledivy opened this issue May 19, 2024 · 2 comments
Labels
bug Something isn't working perf performance related

Comments

@littledivy
Copy link
Member

littledivy commented May 19, 2024

https://x.com/justjs14/status/1791963104881844360

image

Repro: https://gist.github.com/billywhizz/1d6fee4d3aea1030324a7a93de444d31

@littledivy littledivy added bug Something isn't working perf performance related labels May 19, 2024
@sigmaSd
Copy link
Contributor

sigmaSd commented May 21, 2024

Its probably due to permissions check, I think in the case of read when comparing paths deno need to canonicalize and normalize the paths, to know its allowed or not

Maybe absolute flags (as in --allow-read instead of --allow-read=path, etc) should not suffer the sandbox performance hit, since they can work in theory with just an extra boolean check and more importantly with no extra syscalls

@littledivy
Copy link
Member Author

It's a regression from #23208

This getcwd code path should never reach with --allow-all and generic --allow-read

deno/ext/fs/std_fs.rs

Lines 904 to 905 in fabd9a2

let cwd = current_dir()?;
normalize_path(cwd.join(path))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working perf performance related
Projects
None yet
Development

No branches or pull requests

2 participants