Skip to content

Duplicated lookup on unicodekeys_lookup_unicode #104717

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

Closed
corona10 opened this issue May 21, 2023 · 3 comments
Closed

Duplicated lookup on unicodekeys_lookup_unicode #104717

corona10 opened this issue May 21, 2023 · 3 comments
Labels
3.11 only security fixes 3.12 only security fixes

Comments

@corona10
Copy link
Member

corona10 commented May 21, 2023

There are duplicated lookups on unicodekeys_lookup_unicode.
I am trying to understand this code as manual loop unrolling for performance. but I am not sure that it is intended.

ix = dictkeys_get_index(dk, i);
if (ix >= 0) {
PyDictUnicodeEntry *ep = &ep0[ix];
assert(ep->me_key != NULL);
assert(PyUnicode_CheckExact(ep->me_key));
if (ep->me_key == key ||
(unicode_get_hash(ep->me_key) == hash && unicode_eq(ep->me_key, key))) {
return ix;
}
}
else if (ix == DKIX_EMPTY) {
return DKIX_EMPTY;
}
perturb >>= PERTURB_SHIFT;
i = mask & (i*5 + perturb + 1);
ix = dictkeys_get_index(dk, i);
if (ix >= 0) {
PyDictUnicodeEntry *ep = &ep0[ix];
assert(ep->me_key != NULL);
assert(PyUnicode_CheckExact(ep->me_key));
if (ep->me_key == key ||
(unicode_get_hash(ep->me_key) == hash && unicode_eq(ep->me_key, key))) {
return ix;
}
}
else if (ix == DKIX_EMPTY) {
return DKIX_EMPTY;
}
perturb >>= PERTURB_SHIFT;
i = mask & (i*5 + perturb + 1);

First introduced from f8a95df

Please let me know this that is intended.

Linked PRs

@corona10 corona10 added the type-feature A feature request or enhancement label May 21, 2023
@corona10 corona10 added 3.11 only security fixes 3.12 only security fixes labels May 21, 2023
@corona10
Copy link
Member Author

cc @markshannon @methane

@corona10 corona10 added type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels May 21, 2023
@methane
Copy link
Member

methane commented May 21, 2023

Yes, this is intended, manual loop unrolling.
This is the hottest path in dictobject.c.
I don't remember tha actual number of performance gain, but I had confirmed that this had some gain.

@sunmy2019 sunmy2019 removed the type-bug An unexpected behavior, bug, or error label May 21, 2023
@corona10
Copy link
Member Author

corona10 commented May 21, 2023

Okay then I will close the issue.

Yes, this is intended, manual loop unrolling.

It will be great if there is a comment about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 only security fixes
Projects
None yet
Development

No branches or pull requests

3 participants