classification
Title: opcode cache for LOAD_GLOBAL emits false alarm in memory leak hunting
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.8
process
Status: closed Resolution: fixed
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-10-01 16:02 by yselivanov. This issue is now closed.

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 (5)
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];
 #endif
 
 /* 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 */
 
 #if OPCACHE_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) {
         co->co_opcache_flag++;
         if (co->co_opcache_flag == OPCACHE_MIN_RUNS) {
@@ -1244,6 +1245,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
 #endif
         }
     }
+#endif
 
 #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/refleak.py 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)
https://github.com/python/cpython/commit/eddef861b49f1635222a9e1771231c34a807debf
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:
https://github.com/python/cpython/pull/13789#issuecomment-498449735

"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.
msg353666 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-10-01 10:50
It seems like nobody came up with a solution for the debug mode since June. I close the issue. Reopen it if you would like to propose a solution.
msg353700 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2019-10-01 16:02
> It seems like nobody came up with a solution for the debug mode since June. I close the issue. Reopen it if you would like to propose a solution.

I think the only solution is to have a flag to disable optimizations, inlcluding this one.
History
Date User Action Args
2019-10-01 16:02:56yselivanovsetmessages: + msg353700
2019-10-01 10:50:13vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg353666

stage: patch review -> resolved
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