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

msys2-runtime: restore fast path for current user primary group #57

Merged
merged 1 commit into from Aug 28, 2023

Conversation

rglidden
Copy link

@rglidden rglidden commented Aug 24, 2023

Commit a5bcfe6 removed an optimization that fetches the default group from the current user token, as it is sometimes not accurate such as when groups like the builtin Administrators group is the primary group.

However, removing this optimization causes extremely poor performance when connected to some Active Directory environments.

Restored this optimization as the default behaviour, and added a group: db-accurate option to nsswitch.conf that can be used to disable the optimization in cases where accurate group information is required.

This fixes git-for-windows/git#4459

@rglidden rglidden changed the title Revert "Cygwin: uinfo: don't special case current user" to test if it is the root cause of slow performance on Windows msys2-runtime: restore fast path for current user primary group Aug 28, 2023
Commit a5bcfe6 removed an optimization that fetches the
default group from the current user token, as it is sometimes
not accurate such as when groups like the builtin
Administrators group is the primary group.

However, removing this optimization causes extremely poor
performance when connected to some Active Directory
environments.

Restored this optimization as the default behaviour, and
added a `group: db-accurate` option to `nsswitch.conf` that
can be used to disable the optimization in cases where
accurate group information is required.

This fixes git-for-windows/git#4459

Signed-off-by: Richard Glidden <richard@glidden.org>
@rglidden rglidden marked this pull request as ready for review August 28, 2023 18:31
Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

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

Excellent, thank you so much!

@dscho dscho merged commit 25de8b8 into git-for-windows:main Aug 28, 2023
2 checks passed
dscho added a commit to dscho/MSYS2-packages that referenced this pull request Aug 28, 2023
A recent commit removed an optimization from the Cygwin runtime that
fetches the default group from the current user token: At times, this is
not accurate such as when groups like the builtin Administrators group
is the primary group.

However, removing this optimization causes extremely poor
performance when connected to some Active Directory
environments.

Reflecting git-for-windows/msys2-runtime#57,
this update restores this optimization as the default behaviour. To
still allow the user to opt into the correct behavior, a `group:
db-accurate` option is provided for `nsswitch.conf`.

This fixes git-for-windows/git#4459

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
for local accounts, Active Directory for domain account. Examples:
for local accounts, Active Directory for domain account. For the current
user, the default group is obtained from the current user token to avoid
additional lookups to the group database. <literal>db-accuarte</literal>
Copy link

Choose a reason for hiding this comment

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

db-accuarte --> db-accurate

Copy link
Member

Choose a reason for hiding this comment

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

Since we do not build the documentation, this can easily be done in a (non-critical) follow-up PR. @placaze how about it?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for catching that @placaze . Created #58 to fix it.

dscho added a commit to dscho/msys2-runtime that referenced this pull request Sep 6, 2023
msys2-runtime: restore fast path for current user primary group
dscho added a commit to dscho/msys2-runtime that referenced this pull request Nov 29, 2023
msys2-runtime: restore fast path for current user primary group
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants