Stream: git-wasmtime

Topic: wasmtime / issue #7496 Question about creating module cac...

Wasmtime GitHub notifications bot (Nov 08 2023 at 09:26):

iawia002 opened issue #7496:

Feature

When I'm using the cache, I always encounter the following warning. Initially, I thought it was a failure to write the cache, but upon closer inspection, I found that the cache file was actually created successfully.

Failed to write file with rename,
lock path: /Users/xxx/Library/Caches/BytecodeAlliance.wasmtime/modules/wasmtime--1699432190543/KhAxrGdzhkuy9f-lsCzlq73dYFuGl3yOqWQyOpL8cBY.wip-atomic-write-mod,
target path: /Users/xxx/Library/Caches/BytecodeAlliance.wasmtime/modules/wasmtime--1699432190543/KhAxrGdzhkuy9f-lsCzlq73dYFuGl3yOqWQyOpL8cBY,
err: No such file or directory (os error 2)

So, I delved into the code and have some questions about the handling of writing cache files. According to the current implementation, every time a new module is written to the cache will inevitably trigger this warning because the directory doesn't exist. I looked at the comments in the code and found that the intention here is to optimize system calls.

// Optimize syscalls: first, try writing to disk. It should succeed in most cases.
// Otherwise, try creating the cache directory and retry writing to the file.

However, facing this warning every time we run a new module is quite confusing, I'm wondering if we can modify the implementation here.

Implementation

Check if the parent directory exists before writing the file. If it doesn't exist, create the directory first and then proceed with writing the file.

I'm not entirely sure about the performance impact of checking the path's existence every time. If it's feasible, I can go ahead and implement it this way.

Benefit

We won't encounter the warning anymore, and the code will be a bit more concise.

Wasmtime GitHub notifications bot (Nov 08 2023 at 10:13):

bjorn3 commented on issue #7496:

Moving the warn!() from fs_write_atomic to https://github.com/bytecodealliance/wasmtime/blob/b9f2a3060b95a479a5e52ad410f77d43bf1a2a59/crates/cache/src/lib.rs#L199 should fix this.

Wasmtime GitHub notifications bot (Nov 08 2023 at 10:14):

bjorn3 edited a comment on issue #7496:

Moving the warn!() from fs_write_atomic to https://github.com/bytecodealliance/wasmtime/blob/b9f2a3060b95a479a5e52ad410f77d43bf1a2a59/crates/cache/src/lib.rs#L199 should fix this. Or in other words only warn when the write failed after the directory is already created.

Wasmtime GitHub notifications bot (Nov 08 2023 at 15:52):

alexcrichton closed issue #7496:

Feature

When I'm using the cache, I always encounter the following warning. Initially, I thought it was a failure to write the cache, but upon closer inspection, I found that the cache file was actually created successfully.

Failed to write file with rename,
lock path: /Users/xxx/Library/Caches/BytecodeAlliance.wasmtime/modules/wasmtime--1699432190543/KhAxrGdzhkuy9f-lsCzlq73dYFuGl3yOqWQyOpL8cBY.wip-atomic-write-mod,
target path: /Users/xxx/Library/Caches/BytecodeAlliance.wasmtime/modules/wasmtime--1699432190543/KhAxrGdzhkuy9f-lsCzlq73dYFuGl3yOqWQyOpL8cBY,
err: No such file or directory (os error 2)

So, I delved into the code and have some questions about the handling of writing cache files. According to the current implementation, every time a new module is written to the cache will inevitably trigger this warning because the directory doesn't exist. I looked at the comments in the code and found that the intention here is to optimize system calls.

// Optimize syscalls: first, try writing to disk. It should succeed in most cases.
// Otherwise, try creating the cache directory and retry writing to the file.

However, facing this warning every time we run a new module is quite confusing, I'm wondering if we can modify the implementation here.

Implementation

Check if the parent directory exists before writing the file. If it doesn't exist, create the directory first and then proceed with writing the file.

I'm not entirely sure about the performance impact of checking the path's existence every time. If it's feasible, I can go ahead and implement it this way.

Benefit

We won't encounter the warning anymore, and the code will be a bit more concise.

Last updated: Nov 22 2024 at 17:03 UTC