Description

corona10

opened on May 21, 2023

Issue body actions

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.

cpython/Objects/dictobject.c

Lines 932 to 961 in be0c106

	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

• gh-104717: Remove duplicated lookup from unicodekeys_lookup_unicode #104716
• gh-104717: Add comment about manual loop unrolling #104718

Activity

1. corona10

   added

   type-featureA feature request or enhancementA feature request or enhancement

   3.11only security fixesonly security fixes

   3.12only security fixesonly security fixes

   on May 21, 2023
2. corona10 commented on May 21, 2023

   

   corona10

   on May 21, 2023

   MemberAuthor

   More actions

   cc @markshannon @methane
3. corona10

   added

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   and removed

   type-featureA feature request or enhancementA feature request or enhancement

   on May 21, 2023
4. methane commented on May 21, 2023

   

   methane

   on May 21, 2023

   Member

   More actions

   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.

   Reacted by Donghee Na
5. sunmy2019

   removed

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on May 21, 2023
6. corona10 commented on May 21, 2023

   

   corona10

   on May 21, 2023

   MemberAuthor

   More actions

   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.
7. corona10

   closed this as completedon May 21, 2023
8. corona10

   added 2 commits that reference this issue on May 21, 2023

   pythongh-104717: Add comment about manual loop unrolling

   9c38f67

   gh-104717: Add comment about manual loop unrolling (gh-104718)

   b9fcfa6

Issue actions