Skip to content

bpo-35941: Fix performance regression in new code #12610

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.

Sign up for GitHub

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

Merged
zooba merged 2 commits into from
Sep 9, 2019
Merged

bpo-35941: Fix performance regression in new code #12610

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6e73a6
bpo-35941: Fix performance regression in new code
tiran Mar 28, 2019
7efc67a
Allow frozenset of trust_oids
zooba Sep 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
bpo-35941: Fix performance regression in new code 
Accumulate certificates in a set instead of doing a costly list contain
operation. A Windows cert store can easily contain over hundred
certificates. The old code would result in way over 5,000 comparison
operations

Signed-off-by: Christian Heimes <christian@python.org>
  • Loading branch information
@tiran
tiran committed Sep 9, 2019
commit a6e73a6e3e41fb76009cc8ceb4aaa5fa5e4e4e84
55 changes: 27 additions & 28 deletions Modules/_ssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5517,7 +5517,7 @@ parseKeyUsage(PCCERT_CONTEXT pCertCtx, DWORD flags)
}
return PyErr_SetFromWindowsErr(error);
}
retval = PySet_New(NULL);
retval = PyFrozenSet_New(NULL);
if (retval == NULL) {
goto error;
}
Expand Down Expand Up @@ -5591,19 +5591,6 @@ ssl_collect_certificates(const char *store_name)
return hCollectionStore;
}

/* code from Objects/listobject.c */

static int
list_contains(PyListObject *a, PyObject *el)
{
Py_ssize_t i;
int cmp;

for (i = 0, cmp = 0 ; cmp == 0 && i < Py_SIZE(a); ++i)
cmp = PyObject_RichCompareBool(PyList_GET_ITEM(a, i), el, Py_EQ);
return cmp;
}

/*[clinic input]
_ssl.enum_certificates
store_name: str
Expand All @@ -5626,7 +5613,7 @@ _ssl_enum_certificates_impl(PyObject *module, const char *store_name)
PyObject *keyusage = NULL, *cert = NULL, *enc = NULL, *tup = NULL;
PyObject *result = NULL;

result = PyList_New(0);
result = PySet_New(NULL);
if (result == NULL) {
return NULL;
}
Expand Down Expand Up @@ -5666,11 +5653,10 @@ _ssl_enum_certificates_impl(PyObject *module, const char *store_name)
enc = NULL;
PyTuple_SET_ITEM(tup, 2, keyusage);
keyusage = NULL;
if (!list_contains((PyListObject*)result, tup)) {
if (PyList_Append(result, tup) < 0) {
Py_CLEAR(result);
break;
}
if (PySet_Add(result, tup) == -1) {
Py_CLEAR(result);
Py_CLEAR(tup);
break;
Comment thread
tiran marked this conversation as resolved.
Outdated
}
Py_CLEAR(tup);
}
Expand All @@ -5694,7 +5680,14 @@ _ssl_enum_certificates_impl(PyObject *module, const char *store_name)
return PyErr_SetFromWindowsErr(GetLastError());
}

return result;
/* convert set to list */
if (result == NULL) {
return NULL;
} else {

@epicfaace epicfaace Aug 24, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You could remove this else statement (also in the corresponding code in _ssl_enum_crls_impl).

PyObject *lst = PySequence_List(result);
Py_DECREF(result);
return lst;
}
}

/*[clinic input]
Expand All @@ -5718,7 +5711,7 @@ _ssl_enum_crls_impl(PyObject *module, const char *store_name)
PyObject *crl = NULL, *enc = NULL, *tup = NULL;
PyObject *result = NULL;

result = PyList_New(0);
result = PySet_New(NULL);
if (result == NULL) {
return NULL;
}
Expand Down Expand Up @@ -5748,11 +5741,10 @@ _ssl_enum_crls_impl(PyObject *module, const char *store_name)
PyTuple_SET_ITEM(tup, 1, enc);
enc = NULL;

if (!list_contains((PyListObject*)result, tup)) {
if (PyList_Append(result, tup) < 0) {
Py_CLEAR(result);
break;
}
if (PySet_Add(result, tup) == -1) {
Py_CLEAR(result);
Py_CLEAR(tup);
break;
}
Py_CLEAR(tup);
}
Expand All @@ -5774,7 +5766,14 @@ _ssl_enum_crls_impl(PyObject *module, const char *store_name)
Py_XDECREF(result);
return PyErr_SetFromWindowsErr(GetLastError());
}
return result;
/* convert set to list */
if (result == NULL) {
return NULL;
} else {
PyObject *lst = PySequence_List(result);
Py_DECREF(result);
return lst;
}
}

#endif /* _MSC_VER */
Expand Down