Skip to content

gh-145228: Increase performance of _config_dict_get_bool(PyObject *dict, const char *name, int *p_flag) by removing unused PY_DECREF(item)#145229

Closed
benediktjohannes wants to merge 2 commits intopython:mainfrom
benediktjohannes:patch-9
Closed

gh-145228: Increase performance of _config_dict_get_bool(PyObject *dict, const char *name, int *p_flag) by removing unused PY_DECREF(item)#145229
benediktjohannes wants to merge 2 commits intopython:mainfrom
benediktjohannes:patch-9

Conversation

@benediktjohannes
Copy link
Contributor

@benediktjohannes benediktjohannes commented Feb 25, 2026 •
edited by bedevere-app bot
Loading

Fixes #145228

In Python/interpconfig.c we use Py_DECREF(item); in line 118 here

static int
_config_dict_get_bool(PyObject *dict, const char *name, int *p_flag)
{
    PyObject *item;
    if (_config_dict_get(dict, name, &item) < 0) {
        return -1;
    }
    // For now we keep things strict, rather than using PyObject_IsTrue().
    int flag = item == Py_True;
    if (!flag && item != Py_False) {
        Py_DECREF(item);
        config_dict_invalid_type(name);
        return -1;
    }
    Py_DECREF(item);
    *p_flag = flag;
    return 0;
}

while it's not necessary here

    Py_DECREF(item);
    *p_flag = flag;
    return 0;

because we use if (!flag && item != Py_False) and if this returns true, then we return and only in the other case we come to

    Py_DECREF(item);
    *p_flag = flag;
    return 0;

which then means that

a. !flag is false which means that flag is true which means that item == PY_TRUE which doesn't have to be decremented
b. item == Py_FALSE which doesn't have to be decremented
c. both

(This was not tested, but I'm pretty sure from looking at the code, but please correct me if I'm mistaken)

@benediktjohannes
Increase performance of _config_dict_get_bool(PyObject *dict, const c…
2c7f7aa 
…har *name, int *p_flag) by removing unused PY_DECREF(item)
@bedevere-app bedevere-app bot mentioned this pull request Feb 25, 2026
Closed
@sergey-miryanov
Copy link
Contributor

sergey-miryanov commented Feb 25, 2026

I don't think we need a NEWS entry here. Also, this code is not a hot path, and perf changes will be negligible.

On the other hand, if this change will be accepted, I propose to rewrite whole condition in more readable version like:

if (item != Py_True && item != Py_False)

@sergey-miryanov
Copy link
Contributor

sergey-miryanov commented Feb 25, 2026

cc @picnixz

@benediktjohannes
Copy link
Contributor Author

benediktjohannes commented Feb 25, 2026

Hi! Thanks for the review! I'm not deep into this, so I'm not 100% sure whether the suggestion is correct (so I think that this is probably correct, but maybe there are some edge cases why this was done this way? but probably you're right), so if it's the same, I'll add the suggestion.

And I think that (although this might not be a hot path) the change makes sense because why should we leave unnecessary statements in the code (if we did this forever and we never removed "old" things, then the code will start to grow and grow without any end, so I don't really see a reson not to do this).

@benediktjohannes
Copy link
Contributor Author

benediktjohannes commented Feb 25, 2026

Hi! Thanks for the review! I'm not deep into this, so I'm not 100% sure whether the suggestion is correct (so I think that this is probably correct, but maybe there are some edge cases why this was done this way? but probably you're right), so if it's the same, I'll add the suggestion.

And I think that (although this might not be a hot path) the change makes sense because why should we leave unnecessary statements in the code (if we did this forever and we never removed "old" things, then the code will start to grow and grow without any end, so I don't really see a reson not to do this).

Thinking of something like item == null or any other thing (I didn't check this for this case, but just an example for those potential edge cases)

@picnixz
Copy link
Member

picnixz commented Feb 25, 2026

It is not a hot path and I don't think it will really help. So I prefer leaving things as is. Note that we sometimes prefer slightly slower code for the sake of maintenance.

@picnixz
Copy link
Member

picnixz commented Feb 25, 2026

because why should we leave unnecessary statements in the code

Because we do not want to add code churn. This is explained in the devguide as well.

@picnixz picnixz closed this Feb 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Increase performance of _config_dict_get_bool(PyObject *dict, const char *name, int *p_flag) by removing unused PY_DECREF(item)

3 participants

@benediktjohannes @sergey-miryanov @picnixz