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

Use GetFileAttributesW for checking file existence on Windows #101431

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

YYF233333
Copy link
Contributor

@YYF233333 YYF233333 commented Jan 11, 2025

Partially address #78645. (solved by #101435)

Help to address #100109.

We are using _wfsopen in FileAccessWindows::file_exists which opens a file just to check it is there. Use GetFileAttributesW makes this function a lot faster. In MRP of #78645 reduce the lag by about a half (measured by eyes, not that accurate).

I used to complain that file operations on Windows were very slow. My apologies for wrongly blaming you, Windows :)

@YYF233333 YYF233333 requested a review from a team as a code owner January 11, 2025 13:37
@YYF233333
Copy link
Contributor Author

I see GLOBAL_LOCK_FUNCTION in DirAccessWindows::file_exists which uses GetFileAttributesW, so GetFileAttributesW is not as thread safe as _wfsopen? Shall I add it?

Copy link
Contributor

@kiroxas kiroxas left a comment

Choose a reason for hiding this comment

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

Reading metadata will be faster than opening the file, for sure.

@YYF233333
Copy link
Contributor Author

Tested and this do help address #100109.

master

1.gd load: 1.22ms
2.gd load: 1.46ms
3.gd load: 1.57ms
......
64.gd load: 14.75ms
65.gd load: 14.87ms
66.gd load: 15.11ms
67.gd load: 15.68ms
68.gd load: 16.14ms
69.gd load: 15.91ms

This PR

1.gd load: 1.12ms
2.gd load: 1.15ms
3.gd load: 1.34ms
......
65.gd load: 12.17ms
66.gd load: 12.27ms
67.gd load: 12.61ms
68.gd load: 12.77ms
69.gd load: 12.88ms

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

DirAccessWindows use the same function, so I do not see any reason to open the file here.

@akien-mga akien-mga modified the milestones: 4.x, 4.4 Jan 13, 2025
@akien-mga akien-mga merged commit 5877f94 into godotengine:master Jan 13, 2025
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@YYF233333 YYF233333 deleted the remote_tree_fix branch January 13, 2025 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants