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

Improve performance of watcher #6012

Closed
wants to merge 19 commits into from
Closed

Conversation

hez2010
Copy link
Member

@hez2010 hez2010 commented Sep 2, 2021

Details of Changes
File creation is usually a combination of creation (creating a file handle) and modification (writing initial content), given that UpdateFileOrFolderAsync is slow, we introduced performance regression unexpectedly in #5992. This PR aims to resolve the performance issue of UpdateFileOrFolderAsync.

  • Lowered priority of file update
  • Avoid checking sync status for folders which don't support sync
  • Parallel update

Validation
How did you test these changes?

  • Built and ran the app
  • Tested the changes for accessibility

Screenshots (optional)
Before:

before.mp4

After:

after.mp4

}
}
}
});
}
finally
Copy link
Member Author

Choose a reason for hiding this comment

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

There's not supposed to be exception here, but should we catch in case?

@hez2010 hez2010 force-pushed the watcher-imp branch 3 times, most recently from df3a9a2 to 809331b Compare September 3, 2021 11:14
@hez2010
Copy link
Member Author

hez2010 commented Sep 3, 2021

This PR is ready for review. Do you think I should merge this PR with #6028 since I implemented a AsyncManualResetEvent in #6028 which may cause merge confliction?

@yaira2
Copy link
Member

yaira2 commented Sep 3, 2021

This PR is ready for review. Do you think I should merge this PR with #6028 since I implemented a AsyncManualResetEvent in #6028 which may cause merge confliction?

Seems like a good idea, thanks!

@hez2010
Copy link
Member Author

hez2010 commented Sep 3, 2021

Done.

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.

2 participants