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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

os: add fast path to mkdir_all #19869

Merged
merged 4 commits into from
Nov 14, 2023
Merged

Conversation

ttytm
Copy link
Member

@ttytm ttytm commented Nov 14, 2023

mkdir_all is used at many places to ensure a destination directory exists - even when it's likely that it already does. Often it is not wrapped in an os.exists(<dst_dir>) condition. When it is, it should imho not be necessary to add the condition.

This PR increases the performance of mkdir_all when it is called outside a conditional block like mentioned above and the destition dir is already present. It does it by using an early return fast path. The performance increase is more than 300%.

It's the way go does it:
https://github.com/golang/go/blob/dd88f23a2006307f835f42063b5168ec56c2c428/src/os/path.go#L18-L24

The error is kept the same that would be thrown further down the function.

馃 Generated by Copilot at e11c57e

Improve os.create_folder function by checking output path validity. Prevent file overwriting and nested folder creation.

馃 Generated by Copilot at e11c57e

  • Add a check for the output path existence and type before creating a folder (link). This prevents overwriting an existing file or creating a nested folder in os.create_folder.

vlib/os/os.v Outdated Show resolved Hide resolved
Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs a test for the new error too, in vlib/os/file_test.v or in vlib/os/os_test.c.v .

@spytheman
Copy link
Member

The CI failure needs a rebase over master. It is not related to this PR.

@spytheman spytheman merged commit b7400fc into vlang:master Nov 14, 2023
48 checks passed
@ttytm ttytm deleted the os/mkdir_all-fast-path branch December 15, 2023 22:09
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

Successfully merging this pull request may close these issues.

None yet

2 participants