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

avoid using the user.Current on windows to avoid slow startup #1617

Merged
merged 2 commits into from
Mar 3, 2024

Conversation

Catalyn45
Copy link
Contributor

@Catalyn45 Catalyn45 commented Feb 29, 2024

If the user is domain joined, it can experience slow startups (aprox. 4-5 seconds, sometimes longer). After some investigation it seems that user.Current() makes a request to get the user info from active directory which will time out, and then it will fallback on the local info.

this articles gives a very good example with docker commands.
https://www.fearofoblivion.com/slow-docker-commands-on-domain-joined-pc

I also used process explorer and find the same event on the lf.exe process taking long time and failing with BAD NETWORK PATH.

This fix uses the user.UserHomeDir() and os.Getenv("USERNAME") to get the user details which will get them directly from the environment and solves the issue.

Copy link
Collaborator

@joelim-work joelim-work left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, this patch looks fine overall and I think we can try this.

Currently the CI is reporting a failure because the new code isn't formatted correctly. If you can fix that I will approve the PR.

@Catalyn45
Copy link
Contributor Author

@joelim-work sure, sorry about that. I formated the file with go fmt, it should be good now.

@gokcehan
Copy link
Owner

gokcehan commented Mar 3, 2024

It looks good to me as well so let's give it a try. Thank you very much @Catalyn45 for investigating the issue and sending the patch and @joelim-work for the review.

@gokcehan gokcehan merged commit 8ee8592 into gokcehan:master Mar 3, 2024
4 checks passed
@gokcehan gokcehan mentioned this pull request Mar 31, 2024
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.

Slow startup on Windows
3 participants