Segfault using pytest on large sympy test file #4406
Comments
|
I'm able to reproduce this on windows. While watching this, it appears to take analyzing the core dump (without a debug binary, spinning up docker right now to use a debug build) it appears to be a stack overflow. This doesn't reproduce with |
|
with which points at this: https://github.com/python/cpython/blob/4cf1f54eb764f856fda5a394d8598c0c90d69d77/Python/compile.c#L4791 |
|
Okay, so what does that mean? Is this a bug in pytest, cpython or both?
If you can see a simple way to reproduce this without using pytest I'll
take that as a bug report to cpython.
(Of course the `test_trinomials.py` file itself is questionable but that
doesn't let pytest/cpython off the hook for seg-faulting.)
|
|
its possibly an interaction issue where the ast we generate mismatches a potentially hidden/changed expectation of cpython - more investigation is needed |
|
The ~2891 line file expands to ~1029102 (yes million) lines when assert-rewritten. For example, this (relative to the rest of the file) small assert: assert rubi_test(rubi_integrate(x**m*(b*x**S(2) + c*x**S(4)), x), x, b*x**(m + S(3))/(m + S(3)) + c*x**(m + S(5))/(m + S(5)), expand=True, _diff=True, _numerical=True)is rewritten to this (ignore the invalid variable names, since pytest writes directly to the ast the names don't actually matter -- the Some of the assertions cause pytest to produce 3500+ variables for the assertion message: I suspect that's just too much code for python to handle (especially in so few functions) |
|
Issue 31113 for cpython looks similar to this. Apparently a fix went into Python 3.7.1. I've just tested it with Python 3.7.1 and it works: $ pytest sympy/integrals/rubi/rubi_tests/tests/test_trinomials.py
==================================================================== test session starts =====================================================================
platform darwin -- Python 3.7.1, pytest-4.0.0, py-1.7.0, pluggy-0.8.0
architecture: 64-bit
cache: yes
ground types: python
rootdir: /Users/enojb/current/sympy/sympy, inifile:
collected 5 items
sympy/integrals/rubi/rubi_tests/tests/test_trinomials.py sssss [100%]
================================================================= 5 skipped in 99.38 seconds =================================================================So it looks as if CPython has improved its ability to handle large files. It's still possible that pytest shouldn't be generating them though. |
|
code generation is how pytest makes pretty assertions, I don't think there's a way out of that. Since this is fixed in cpython, I'm going to close this. Thanks for the issue! |
|
Well... when I say that it works, it does take a long time and consume a lot of memory just to collect these tests (and then skip them!). I would expect other situations where this causes problems if pytest has no limits on the code it generates. |
|
your example here is pretty exceptional: a test containing 3k lines of 20+ operation assertions 😆 -- I wouldn't expect anyone to hit this normally |
|
we ought to consider thinking about proposing opt-out for files with high cost patterns like the one @oscarbenjamin has in the testsuite a number of assertions per file treshold could help @oscarbenjamin could you take a look at which parts of your file actually cause the issues (aka is it one special test with exorbitant complexity result for the assert statement, or is it she sheer number? based on this assessment we can think of something to make sense of this @nicoddemus @asottile i do wonder if it would be a sensible approach to switch from plain assertions to a metatool where the unaltered code of the assertion only is executed on top of a special namespace this would reduce the complexity of the ast significantly, moving the cost to different places instead |
Is the rewriting done per function or per module? Not rewriting skipped tests would solve the problem for this particular module.
Yeah, I can take a look. How can I see the output that pytest would generate from assert rewriting? It would be nice if there was a command line flag for that: $ pytest sympy/solvers/tests/test_ode.py -k test_classify_ode --show-assert-rewrite
It's probably worth considering the cost of assertion rewriting in less pathological cases as well as the example in this issue. A more typical test run for me would be: $ git clone https://github.com/sympy/sympy.git
$ cd sympy/
$ pytest -m 'not slow' sympy/solvers/tests/test_ode.py With that I get: $ pytest -m 'not slow' sympy/solvers/tests/test_ode.py --assert=plain # 329 seconds
$ pytest -m 'not slow' sympy/solvers/tests/test_ode.py # 334 secondsThe additional time is the same when running tests in parallel with pytest-xdist $ pytest -m 'not slow' sympy/solvers/tests/test_ode.py -n 3 --assert=plain # 193 seconds
$ pytest -m 'not slow' sympy/solvers/tests/test_ode.py -n 3 # 206 secondsSo it looks as if assert-rewriting |
|
It's also worth noting that if assert-rewriting slows every tested module then when running a large test suite to see a (hopefully!) small number of fails it would be quicker to run without assert rewriting and then to rerun only the failed tests with rewriting at the end. |
|
xdist likely compounds this as it discovers multiple times in each worker -- given your linear discovery overhead the rewrite seems to (only) take ~5 seconds. looking at that file it's not surprising why -- it has similar complex patterns to the problem-file from before (lots of assertion expressions with high numbers of locals and operations in the assertion) assertion rewriting is done per module -- this is the most basic unit of the python import system (and the cacheable unit via pyc files) I displayed the code by applying a patch similar to this one I'm not convinced normal test suites see this extreme level of overhead. On pytest's testsuite the total overhead is ~3 seconds (I took best-of-5 for both measurements below): pytest has ~4000 assertions On pre-commit's testsuite (the largest I "own": ~550 asserts) the overhead is .5s (1.5s vs 1.0s). |
|
collection coordination for xdist is a desired feature simply because right now the import system costs are problematic for some types of project (i believe cryptography is affected as well CC @alex ) |
|
I reran my timings for $ pytest -m 'not slow' sympy/solvers/tests/test_ode.py --assert=plain
$ pytest -m 'not slow' sympy/solvers/tests/test_ode.pyand after 5 runs of each I don't see a significant difference. Inter-run variability is bigger than the difference between using and not using I will still take a look at the issue with |
|
@oscarbenjamin note that I clean |
|
If I switch between using / not using |
|
The commandline parameter will take immediately, but you'll only see the rewrite overhead once as it gets cached into pyc files. |
|
@RonnyPfannschmidt as of the last time we checked (though it's been a while; cc: @reaperhulk), that's right -- despite having 100k tests, the cryptography test suite doesn't get a benefit from xdist because of this. |
|
Our issues with pytest-xdist are more related to memory than anything else. Test collection (on an i7-8850H) for cryptography takes ~20s and without assert rewrite it's ~19s, but our test suite takes 5-10 minutes so parallelization would be a win even with the unfortunate overhead of duplicate collection. Unfortunately, Python uses nearly 700MB of RAM per process just collecting the tests (and at test completion each process is ~1GB when we do n=4). Our CI system can't afford that kind of memory consumption. |
|
Do you get the same memory overhead with |
|
Yes, asset rewriting has negligible memory impact (at least in our specific case). |
|
thanks for the inputs, we will have to open 2-3 targeted followup issues |
I'm trying to use
pytestwithsympybut getting a segfault. Since bothpytestandsympyare pure Python I guess this is a bug in CPython. I'm finding it difficult to debug throughpytestto isolate the underlying bug though.I'm getting this segfault with Python 3.6 on OSX. I had something similar on Linux but I've now isolated it more precisely on OSX and haven't tested these exact steps on Linux since.
The environment is
Now clone
sympyand runpytestontest_trinomials.py. This file is 1.4MB and very repetitive. You can see it here. It takes some time before you see the segfault.The text was updated successfully, but these errors were encountered: