feat: optimize syscalls by only copying executable bits #1813
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
Author
|
@wolfv This also no longer copies the other permission bits (e.g. read/write). Im not sure if that would cause issues. I assume that either these are already copied because the underlying filesystem works like that or they are not copied but we get the "default" permissions of the user, which might arguably be more correct anyway. WDYT? |
Only copy the executable bit permissions when the source file is executable AND the destination doesn't already have the correct executable bits. This drastically reduces syscalls for non-executable files and executable files that already have correct permissions. Changes: - Created `copy_executable_permissions()` helper function to encapsulate the optimization logic - Applied optimization to reflink operations on Linux - Applied optimization to patched file operations (prefix replacement) - Only copy executable bits (0o111), not all permission bits - Skip permission updates if bits already match Performance impact for reflink operations: - Non-executable files: 1 syscall (metadata only) Previously: 2 syscalls (metadata + set_permissions) - Executable files with matching permissions: 2 syscalls (metadata source + dest) Previously: 2 syscalls (metadata + set_permissions) - Executable files needing update: 3 syscalls (metadata source + dest + set_permissions) Previously: 2 syscalls (metadata + set_permissions) Performance impact for patched files (prefix replacement): - Non-executable files: 0 additional syscalls Previously: 1 syscall (set_permissions) - Executable files with matching permissions: 1 syscall (metadata dest) Previously: 1 syscall (set_permissions) - Executable files needing update: 2 syscalls (metadata dest + set_permissions) Previously: 1 syscall (set_permissions) Since the vast majority of package files are not executable, and many executable files already have correct permissions after creation/reflink, this provides a significant performance improvement during package installation. The optimization only affects Unix systems. On Windows, permissions are handled differently and this optimization is not applicable. Fixes conda#486
baszalmstra
force-pushed
the
claude/implement-issue-486-011CUtGDc3P3dxwTpfv2dtKg
branch
from
dc8a610 to
57c6be1
Compare
Collaborator
Author
|
Running this locally on my WSL Ubuntu 22.04 (which doesn't support reflinking) the performance impact is small (~1%). Which is expected. I would be mostly interested to see if this has an impact on macOS. |
Collaborator
|
@baszalmstra how can I measure the impact? |
Collaborator
Author
|
Build I expect the biggest difference will be one linux with a filesystem that allows reflinks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Only copy the executable bit permissions when the source file is executable AND the destination doesn't already have the correct executable bits. This drastically reduces syscalls for non-executable files and executable files that already have correct permissions.
Changes:
copy_executable_permissions()helper function to encapsulate the optimization logicSince the vast majority of package files are not executable, and many executable files already have correct permissions after creation/reflink, this provides a significant performance improvement during package installation.
The optimization only affects Unix systems. On Windows, permissions are handled differently and this optimization is not applicable.
Fixes #486
AI Disclosure
Written by Claude Code Web