◐ Shell
clean mode source ↗

GH-100288: Skip extra work when failing to specialize `LOAD_ATTR` by brandtbucher · Pull Request #101354 · python/cpython

@brandtbucher

I think this was missed in #100753.

This removes a dict lookup, a dict version, and a cache write from one case where we fail to specialize method loads (due to the existence of a non-managed instance __dict__).

markshannon

Choose a reason for hiding this comment

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

Thanks. Could you also move the body of the if statement into the switch case.

}
}
if (dictkind == MANAGED_VALUES || dictkind == OFFSET_DICT) {
if (dictkind == MANAGED_VALUES) {

Choose a reason for hiding this comment

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

Since there is only the one case now, it would make the code easier to follow if this were moved into the case MANAGED_VALUES: below.

@brandtbucher

Can I just get rid of the switch entirely? Each branch in the above if/else logic maps to exactly one case, so we might as well just lift the bodies of the cases up there.

@markshannon

Can I just get rid of the switch entirely? Each branch in the above if/else logic maps to exactly one case, so we might as well just lift the bodies of the cases up there.

Sounds good to me. We can then get rid of ObjectDictKind as well.

@markshannon

markshannon

@brandtbucher

Confirmed locally that this doesn't change the stats.

carljm added a commit to carljm/cpython that referenced this pull request

Jan 31, 2023