Skip to content

bpo-37146: Deactivate opcode cache only when using huntrleaks in the test suite #24643

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
merged 5 commits into from
Feb 28, 2021
Merged

bpo-37146: Deactivate opcode cache only when using huntrleaks in the test suite #24643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0c0f5d8
bpo-37146: Deactivate opcode cache only when using huntrleaks in the …
pablogsal Feb 24, 2021
c11e70f
Update Include/cpython/ceval.h
pablogsal Feb 27, 2021
a9680bc
Update Python/ceval.c
pablogsal Feb 27, 2021
2356931
Update Python/ceval.c
pablogsal Feb 27, 2021
68b0f83
Add comment about bpo-3714
pablogsal Feb 27, 2021
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
2 changes: 2 additions & 0 deletions Include/cpython/ceval.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,5 @@ PyAPI_FUNC(Py_ssize_t) _PyEval_RequestCodeExtraIndex(freefunc);

PyAPI_FUNC(int) _PyEval_SliceIndex(PyObject *, Py_ssize_t *);
PyAPI_FUNC(int) _PyEval_SliceIndexNotNone(PyObject *, Py_ssize_t *);

PyAPI_FUNC(void) _PyEval_DeactivateOpCache(void);
1 change: 1 addition & 0 deletions Lib/test/libregrtest/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ def setup_tests(ns):

if ns.huntrleaks:
unittest.BaseTestSuite._cleanup = False
sys._deactivate_opcache()

if ns.memlimit is not None:
support.set_memlimit(ns.memlimit)
Expand Down
22 changes: 12 additions & 10 deletions Python/ceval.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,19 @@ static long dxp[256];
#endif

/* per opcode cache */
#ifdef Py_DEBUG
// --with-pydebug is used to find memory leak. opcache makes it harder.
// So we disable opcache when Py_DEBUG is defined.
// See bpo-37146
#define OPCACHE_MIN_RUNS 0 /* disable opcache */
#else
#define OPCACHE_MIN_RUNS 1024 /* create opcache when code executed this time */
#endif
static int opcache_min_runs = 1024; /* create opcache when code executed this many times */
#define OPCODE_CACHE_MAX_TRIES 20
#define OPCACHE_STATS 0 /* Enable stats */

// This function allows to deactivate the opcode cache. As different cache mechanisms may hold
// references, this can mess with the reference leak detector functionality so the cache needs
// to be deactivated in such scenarios to avoid false positives. See bpo-3714 for more information.
void
_PyEval_DeactivateOpCache(void)
{
opcache_min_runs = 0;
}

#if OPCACHE_STATS
static size_t opcache_code_objects = 0;
static size_t opcache_code_objects_extra_mem = 0;
Expand Down Expand Up @@ -1705,9 +1707,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
f->f_stackdepth = -1;
f->f_state = FRAME_EXECUTING;

if (co->co_opcache_flag < OPCACHE_MIN_RUNS) {
if (co->co_opcache_flag < opcache_min_runs) {
co->co_opcache_flag++;
if (co->co_opcache_flag == OPCACHE_MIN_RUNS) {
if (co->co_opcache_flag == opcache_min_runs) {
Copy link
Member

Choose a reason for hiding this comment

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

This may increase memory access in hot path. Would you run pypeformance to ensure no performance regression?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, will run this over the weekend.

Copy link
Member Author

@pablogsal pablogsal Feb 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Seems that there is no much different over the noise level:

❯ pyperf compare_to json/* --table --min-speed 2 -G
+----------------+--------------------------------------+-----------------------------------------+
| Benchmark      | 2021-02-27_11-31-master-c71d24f55828 | 2021-02-27_19-33-bpo-37136-a9680bcf9b84 |
+================+======================================+=========================================+
| unpickle       | 17.0 us                              | 17.4 us: 1.02x slower                   |
+----------------+--------------------------------------+-----------------------------------------+
| pickle         | 13.2 us                              | 13.7 us: 1.04x slower                   |
+----------------+--------------------------------------+-----------------------------------------+
| nqueens        | 121 ms                               | 126 ms: 1.04x slower                    |
+----------------+--------------------------------------+-----------------------------------------+
| Geometric mean | (ref)                                | 1.00x slower                            |
+----------------+--------------------------------------+-----------------------------------------+

Benchmark hidden because not significant (56): xml_etree_parse, scimark_sor, unpickle_list, nbody, unpack_sequence, regex_v8, chaos, meteor_contest, telco, python_startup, genshi_text, regex_compile, python_startup_no_site, json_loads, regex_dna, logging_simple, sympy_sum, xml_etree_iterparse, scimark_sparse_mat_mult, xml_etree_process, richards, chameleon, sqlalchemy_declarative, django_template, hexiom, pathlib, json_dumps, tornado_http, sympy_str, unpickle_pure_python, pyflate, pidigits, go, regex_effbot, deltablue, sympy_expand, xml_etree_generate, float, genshi_xml, logging_silent, raytrace, fannkuch, sqlalchemy_imperative, scimark_monte_carlo, logging_format, 2to3, sympy_integrate, mako, pickle_dict, pickle_list, spectral_norm, dulwich_log, sqlite_synth, scimark_lu, pickle_pure_python, scimark_fft, crypto_pyaes

if (_PyCode_InitOpcache(co) < 0) {
goto exit_eval_frame;
}
Expand Down
20 changes: 19 additions & 1 deletion Python/clinic/sysmodule.c.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions Python/sysmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1958,6 +1958,20 @@ sys_getandroidapilevel_impl(PyObject *module)
#endif /* ANDROID_API_LEVEL */


/*[clinic input]
sys._deactivate_opcache

Deactivate the opcode cache permanently
[clinic start generated code]*/

static PyObject *
sys__deactivate_opcache_impl(PyObject *module)
/*[clinic end generated code: output=00e20982bd012122 input=501eac146735ccf9]*/
{
_PyEval_DeactivateOpCache();
Py_RETURN_NONE;
}


static PyMethodDef sys_methods[] = {
/* Might as well keep this in alphabetic order */
Expand Down Expand Up @@ -2011,6 +2025,7 @@ static PyMethodDef sys_methods[] = {
SYS_GET_ASYNCGEN_HOOKS_METHODDEF
SYS_GETANDROIDAPILEVEL_METHODDEF
SYS_UNRAISABLEHOOK_METHODDEF
SYS__DEACTIVATE_OPCACHE_METHODDEF
{NULL, NULL} /* sentinel */
};

Expand Down