Title: opcode cache for LOAD_GLOBAL emits false alarm in memory leak hunting
Type: Stage: patch review
Components: Interpreter Core Versions: Python 3.8
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: inada.naoki, vstinner, yselivanov
Priority: normal Keywords: patch

Created on 2019-06-03 20:34 by vstinner, last changed 2019-06-03 22:41 by vstinner.

Pull Requests
URL Status Linked Edit
PR 13787 merged inada.naoki, 2019-06-03 22:19
PR 13789 closed pablogsal, 2019-06-03 22:27
Messages (3)
msg344469 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-03 20:34
opcode cache for LOAD_GLOBAL introduced false alarm in memory leak hunting (python3 -m test -R 3:3 ...).

=> opcache: bpo-26219.

Before the change:

$ git checkout 91234a16367b56ca03ee289f7c03a34d4cfec4c8^
$ make && ./python -m test -R 3:3 test_pprint 
Tests result: SUCCESS

After the change:

$ git checkout 91234a16367b56ca03ee289f7c03a34d4cfec4c8
$ make && ./python -m test -R 3:3 test_pprint 
test_pprint leaked [4, 2, 4] memory blocks, sum=10

The problem is that at each iteration of regrtest -R 3:3 (6 iterations), a few more code objects get this opcache allocated.

There are different solutions to fix regrtest -R 3:3.

(*) Always optimize:

diff --git a/Python/ceval.c b/Python/ceval.c
index 411ba3d73c..6cd148efba 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -103,7 +103,7 @@ static long dxp[256];
 /* per opcode cache */
-#define OPCACHE_MIN_RUNS 1024  /* create opcache when code executed this time */
+#define OPCACHE_MIN_RUNS 1  /* create opcache when code executed this time */
 #define OPCACHE_STATS 0  /* Enable stats */

$ make && ./python -m test -R 3:3 test_pprint 
Tests result: SUCCESS

(*) Never optimmize: disable opcache until a better fix can be found

diff --git a/Python/ceval.c b/Python/ceval.c
index 411ba3d73c..3c85df6fea 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -1230,6 +1230,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
     f->f_stacktop = NULL;       /* remains NULL unless yield suspends frame */
     f->f_executing = 1;
+#if 0
     if (co->co_opcache_flag < OPCACHE_MIN_RUNS) {
         if (co->co_opcache_flag == OPCACHE_MIN_RUNS) {
@@ -1244,6 +1245,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
 #ifdef LLTRACE
     lltrace = _PyDict_GetItemId(f->f_globals, &PyId___ltrace__) != NULL;

$ make && ./python -m test -R 3:3 test_pprint 
Tests result: SUCCESS

(*) Find a way to explicitly deoptimize all code objects

Modules/gcmodule.c has a clear_freelists() function called by collect() if generation == NUM_GENERATIONS-1: on when gc.collect() is collected explicitly for example.

Lib/test/libregrtest/ also has a dash_R_cleanup() function which clears many caches.

Problem: currently, code objects are not explicitly tracked (for example, they are not tracked in a double linked list).

(*) Add way more warmup iterations to regrtest in buildbots.

I dislike this option. A build on a refleak buildbot worker already takes 2 to 3 hours. Adding more warmup would make a build even way more slower.
msg344485 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-03 22:38
New changeset eddef861b49f1635222a9e1771231c34a807debf by Victor Stinner (Inada Naoki) in branch 'master':
bpo-37146: disable opcache when Py_DEBUG is defined (GH-13787)
msg344486 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-03 22:41
INADA-san wrote PR 13787 to disable opcache when Python is compiled in debug mode. Pablo and me approved the change, so I merged it.

Pablo wrote a more robust solution, PR 13789, to disable opcache only in regrtest, to look for memory leaks. But INADA-san had a good argument against this approach:

"The code object will be optimized only when ++co->co_opcache_flag == opcacheminruns. So decreasing min_runs by _setopcacheminruns()will cause some hot codes will be not optimized forever. I don't want to expose such switch."

I would prefer to keep this issue until a long term approach is designed.
Date User Action Args
2019-06-03 22:41:07vstinnersetmessages: + msg344486
2019-06-03 22:38:13vstinnersetmessages: + msg344485
2019-06-03 22:27:26pablogsalsetpull_requests: + pull_request13673
2019-06-03 22:19:57inada.naokisetkeywords: + patch
stage: patch review
pull_requests: + pull_request13671
2019-06-03 20:34:43vstinnercreate