Thanks for looking into this.

The PyPy garbage collector kicks into action when it sees there is too much memory allocated (the pypy total allocated in your analysis. It "knows" about memory allocated for PyObjects in the C-API, and about python objects allocated by the interpreter. But it cannot know about memory allocted directly in C by malloc and friends (calloc, _alloc, ...). There is a way to tell the PyPy GC about such allocations using __pypy__.add_memory_pressure. Is there much direct C allocation in scikit-learn (this includes use of PyMem_RawMalloc and friends)?

Another possible cause for lack of collection is that PyPy does not break C-API memory cycles. There is some discussion of the problem in a open PyPy issue. TL;DR: we hope adoption of HPy will eliminate the dual representation of objects in C and in RPython that makes this problem very difficult. However, I would expect such cycles to at least show up in the pypy total allocated statistic.

I check the Cython code for the loss in accordance to the snippet earlier. I think that there is 2 relevant piece of info. The allocation is done in the Python part:

So we have a np.empty there and then we call the Cython code.

In this part, the only thing that could be weird is the call to np.asarray. I don't know if there is an interaction between Cython, the garbage collector, and the memory view.

@lesteve would you mind to try the following patch that avoid the call to np.asarray and just write directly in the memoryview. Unfortunately, I cannot quickly install PyPy on my machine.

diff --git a/sklearn/_loss/_loss.pyx.tp b/sklearn/_loss/_loss.pyx.tp
index 8efeeea77f..915380d260 100644
--- a/sklearn/_loss/_loss.pyx.tp
+++ b/sklearn/_loss/_loss.pyx.tp
@@ -1045,7 +1045,7 @@ cdef class {{name}}(CyLossFunction):
             ):
                 loss_out[i] = sample_weight[i] * {{closs}}(y_true[i], raw_prediction[i]{{with_param}})
 
-        return np.asarray(loss_out)
+        # return np.asarray(loss_out)
 
     {{if closs_grad is not None}}
     def loss_gradient(
diff --git a/sklearn/_loss/loss.py b/sklearn/_loss/loss.py
index 11cb0e42c4..bd3d0df884 100644
--- a/sklearn/_loss/loss.py
+++ b/sklearn/_loss/loss.py
@@ -189,13 +189,14 @@ class BaseLoss:
         if raw_prediction.ndim == 2 and raw_prediction.shape[1] == 1:
             raw_prediction = raw_prediction.squeeze(1)
 
-        return self.closs.loss(
+        self.closs.loss(
             y_true=y_true,
             raw_prediction=raw_prediction,
             sample_weight=sample_weight,
             loss_out=loss_out,
             n_threads=n_threads,
         )
+        return loss_out
 
     def loss_gradient(
         self,

I found my linux machine and could try @lesteve script with the above changes. Interestingly, it removes the memory leak:

0 psutil: 0.199GB , pypy total allocated: 97.0MB
10 psutil: 0.227GB , pypy total allocated: 98.2MB
20 psutil: 0.227GB , pypy total allocated: 98.2MB
30 psutil: 0.227GB , pypy total allocated: 98.2MB
40 psutil: 0.227GB , pypy total allocated: 98.2MB
50 psutil: 0.227GB , pypy total allocated: 98.2MB
60 psutil: 0.227GB , pypy total allocated: 98.2MB
70 psutil: 0.227GB , pypy total allocated: 98.2MB
80 psutil: 0.227GB , pypy total allocated: 98.2MB
90 psutil: 0.227GB , pypy total allocated: 98.2MB
100 psutil: 0.227GB , pypy total allocated: 98.2MB
Total memory consumed:
    GC used:            85.2MB (peak: 97.2MB)
       in arenas:            62.0MB
       rawmalloced:          20.2MB
       nursery:              3.0MB
    raw assembler used: 664.6kB
    -----------------------------
    Total:              85.8MB

    Total memory allocated:
    GC allocated:            97.2MB (peak: 102.5MB)
       in arenas:            68.7MB
       rawmalloced:          25.5MB
       nursery:              3.0MB
    raw assembler allocated: 1.0MB
    -----------------------------
    Total:                   98.2MB

    Total time spent in GC:  11.162

So we should probably try to look at the np.asarray that we are using to convert memoryview back to arrays since that there is something going there.

We do not need those np.asarray calls when we pass pre-allocated out arguments to Cython functions. We can just remove them all. /cc @lorentzenchr.

We could report a minimal reproducer to Cython and/or PyPy so that they could work out a way to avoid this problem in the first place, +1 for simplifying our code anyways to drop the useless asarrays.

Great news. A smaller reproducer would be welcome, although I imagine it hits the C-python mutual reference cycle problem I mentioned above. At least I could add it to the PyPy FAQ of "things to avoid".

FYI with the fix from #27662 (comment), the full test suite still uses ~9.5GB on PyPy so this saved about 1.5GB. The Azure VM has 7GB of RAM (according to this) so more effort is needed to find other places where similar issues occur ...

@lesteve Did you modify all the np.asarray from the loss module?
I open #27670 and change the tests in case you want to give it a try if you only touch modify the loss function (and not the gradients, hessian, proba, etc.)

I also recall that we use np.asarray to convert memory-view back to python object when we changed C buffer by a read-only memory-view that was previously not working with older Cython version.

I check how many times the pattern happened:

> git grep "np.asarray(" | grep ".pyx:"  | wc -l
      37

We might need to fix those if we want to be on the safe side and hoping that what is in the asarray is already an array.

I could come up with the following minimal reproducer based on @lesteve script.

%%cython

import psutil
import gc
from functools import partial
import platform

import cython
import numpy as np

IS_PYPY = platform.python_implementation() == "PyPy"


def cy_func(cython.floating[::1] arr_out):
    cdef:
        int i
        int size_arr = arr_out.shape[0]
    for i in range(size_arr):
        arr_out[i] = 10.0

    return np.asarray(arr_out)


def func():
    for _ in range(100):
        arr_out = np.ones(1_000_000, dtype=np.float64)
        cy_func(arr_out)


def main():
    for i in range(10):
        memory_usage = psutil.Process().memory_info().rss / 1e9
        message = f'{i} psutil: {memory_usage:.3f}GB'
        if IS_PYPY:
            pypy_memory_usage = gc.get_stats().memory_allocated_sum
            message += f' , pypy total allocated: {pypy_memory_usage}'
        print(message)

        func()
        
    if IS_PYPY:
        print(gc.get_stats())


if __name__ == '__main__':
    main()

0 psutil: 0.287GB , pypy total allocated: 193.2MB
1 psutil: 1.087GB , pypy total allocated: 183.0MB
2 psutil: 1.888GB , pypy total allocated: 183.0MB
3 psutil: 2.688GB , pypy total allocated: 183.1MB
4 psutil: 3.489GB , pypy total allocated: 183.0MB
5 psutil: 4.289GB , pypy total allocated: 183.1MB
6 psutil: 5.089GB , pypy total allocated: 183.0MB
7 psutil: 5.890GB , pypy total allocated: 183.0MB
8 psutil: 6.690GB , pypy total allocated: 183.0MB
9 psutil: 7.490GB , pypy total allocated: 183.1MB
Total memory consumed:
    GC used:            49.0MB (peak: 204.6MB)
       in arenas:            0.2kB
       rawmalloced:          46.0MB
       nursery:              3.0MB
    raw assembler used: 921.6kB
    -----------------------------
    Total:              49.9MB

    Total memory allocated:
    GC allocated:            181.0MB (peak: 207.9MB)
       in arenas:            128.7MB
       rawmalloced:          72.9MB
       nursery:              3.0MB
    raw assembler allocated: 2.0MB
    -----------------------------
    Total:                   183.0MB

    Total time spent in GC:  4.735

Thanks for the minimum reproducer. I opened a pypy issue.

So maybe we can wait before attempting to fix the 37 occurrences of this pattern in scikit-learn. Please let us know @mattip if this is too complex to fix on the PyPy/Cython side: we could then change scikit-learn to avoid this pattern in the first place.

np.asarray(memoryview) is THE canonical way of converting memoryviews to numpy arrays, see https://cython.readthedocs.io/en/latest/src/userguide/memoryviews.html#coercion-to-numpy.
Removing this pattern should be a last and temporary resort as the bug is not caused by scikit-learn.

I agree when the allocation is done by the Cython code itself. But in this case, the array is allocated (with numpy) and passed by the Python function as input argument to the Cython function. The Python caller could just return the original array it allocated and the Cython function would not return anything. This would be slightly more efficient (by creating a single numpy array instead of 2 numpy arrays that share the same data buffer).

PyPy timed out 3 times out of the 6 last runs, this is going in the right direction but not quite there yet ...

When it does not time out there is a weird matplotlib issue, see #27648 (edit seemed like it was specific to matplotlib 3.8.0 for some reason)

Saving the reproducer as test_pypy.pyx, and running cythonize test_pypy.pyx results in a c-extension module. I can use it to recreate the problem

$ pypy3.9 -m venv /tmp/venv3
$ source /tmp/venv3/bin/activate
$ python -m pip install numpy cython
$ cythonize temp_pypy
$ python
>>>> import test_pypy, numpy as np
>>>> arr_out = np.ones(1_000_000, dtype = np.float64)
>>>> a = test_pypy.cy_func(arr_out)
>>>> type(a)
<class 'numpy.ndarray'>
>>>> type(a.base)
<class 'memoryview'>
>>>> type(a.base.obj)
<class 'test_pypy._memoryviewslice'>
>>>> a.base.obj.base is arr_out
True

So cython has a class _memoryviewslice which has a __dealloc__ method, and hangs on to the original object, which is wrapped in a memoryview, which is wrapped in an ndarray. This is going to create gc cycles between objects in a way that I am not sure even the cpyext-gc-cycle branch will be able to untangle.

Can we close as #27670 is merged?

I would keep this one open since I may have another look in the future. Having said that, this is not very high priority, since PyPy tests time out a lot rarely after #27670 was merged.

A quick look at #27750 seems to indicate that PyPy tests timed out 5 times since November 8 (i.e. in 23 days or so) so the situation is a lot better than previously when it was always timing out.

The slowest test by far is tests/test_multioutput.py::test_classifier_chain_tuple_order[list], at 267 seconds where the next slowest is 162 seconds. Thanks for looking at this.

Thanks for the input! I said "timed out" which is not very accurate. I am reasonably confident the tests get killed because they use too much memory.

I am reasonably confident the tests get killed because they use too much memory.

Which in turns causes the CI host to swap memory and slow the test run to the point of reaching the job-level timeout.

We could very easily reduce the n_estimators of the RF in test_classifier_chain_tuple_order from the default 100 to 10.

As discussed at the last dev meeting, since none of us plans to invest more effort in fixing the root cause of the memory usage problem regularly observed on the PyPy nightly CI run, I posted the following call for help on our linkedin account:

• https://www.linkedin.com/feed/update/urn:li:share:7178692423802331137/

and on my personal mastodon account:

• https://sigmoid.social/deck/@ogrisel/112167085151516949

feel free to re-post.

Thanks, I think if someone has some interest in scikit-learn on PyPy and wants to look at this, a good starting point could be to:

• set-up a local environment with dependencies using conda-forge and compile scikit-learn from source
• use my branch https://github.com/lesteve/scikit-learn/tree/investigate-pypy-memory-usage where I added some memory usage information for each test and analyse the produced json to see if some memory usage can be traced to a particular test, module, etc ...