feat: enable cache for VFS #209
Conversation
| asset_cache_hash_path = vfs_cache_dir / cas_prefix | ||
| _ensure_paths_within_directory(str(asset_cache_hash_path), [cas_prefix]) | ||
|
|
||
| vfs_cache_dir.mkdir(mode=0o700, exist_ok=True) |
There was a problem hiding this comment.
If the directory already exists, does this ensure its mode is 0o700?
There was a problem hiding this comment.
No. My thinking was that there shouldn't be anything else creating this directory first, but it makes sense to change this to creating directory and then call os.chmod which will work even if it already exists.
There was a problem hiding this comment.
I think you would do both - create the directory requesting the mode, and then also do an os.chmod.
npmacl
force-pushed
the
npmac_enable_vfs_cache
branch
2 times, most recently
from
50b6620 to
53e6dce
Compare
|
|
||
| # Validate hashes are alphanumeric | ||
| for path in decoded_manifest.paths: | ||
| if re.fullmatch("[a-zA-Z0-9]+", path.hash) is None: |
There was a problem hiding this comment.
Did you happen to profile this bit as well on large manifests?
There was a problem hiding this comment.
I put this validation in a for loop and timed running through 1 million alphanumerical strings 25 characters long, it took ~13.5 seconds.
There was a problem hiding this comment.
The way to optimize this is to create a compiled regex at file global scope, and then use that to do the match here. This code is compiling the regex and doing the match every time.
There was a problem hiding this comment.
There was a problem hiding this comment.
Ok - not great, but not absolutely terrible. I think maybe we can revisit and possibly remove this down the line as we make other security improvements if this was just another offshoot of the symlink/running as deadline-worker issues.
There was a problem hiding this comment.
gave that a try, and it brought it down to ~4.5 seconds. I'll update that, thanks Mark!
Signed-off-by: Nathan MacLeod <[email protected]>
npmacl
force-pushed
the
npmac_enable_vfs_cache
branch
from
53e6dce to
152f662
Compare
Signed-off-by: Nathan MacLeod <[email protected]>
Signed-off-by: Nathan MacLeod <[email protected]>
* feat: enable cache for VFS Signed-off-by: Nathan MacLeod <[email protected]> * switched to precompiling alphanumerical regex Signed-off-by: Nathan MacLeod <[email protected]> * ran fmt Signed-off-by: Nathan MacLeod <[email protected]> --------- Signed-off-by: Nathan MacLeod <[email protected]>
What was the problem/requirement? (What/Why)
What was the solution? (How)
Change the launch command to enable the cache.
What is the impact of this change?
Depending on the job, should result in improved performance.
How was this change tested?
hatch run all:test,hatch run lint,hatch run fmtWas this change documented?
no
Is this a breaking change?
no