fix: prevent exponential memory growth in UnionArray

[jpivarski]

I decided to just make UnionArray.simplified not allow lazy carries. It should always return correct results and it can only have a performance impact on wide RecordArrays that go through UnionArray.simplified (perhaps via ak.concatenate).

The new test would fail for the old code: for 5 loops, the NumpyArray length gets up to 32, and we assert <= 2.

I only had to change one test, which was hard-coded to a particular Form. It was for testing allow_noncanonical_form=True in ak.from_buffers. I think that feature is still being tested with the updated test.

@agoose77, am I missing anything here? Would you do anything differently?

[ianna]

@jpivarski - it looks like the tests on Mac are segfaulting and not only for this PR. I'm checking locally.

[agoose77]

@jpivarski I agree with your reasoning. I haven't given much more thought to alternative solutions, but we can always revisit this in future!

[ianna]

@jpivarski - it looks like the tests on Mac are segfaulting and not only for this PR. I'm checking locally.

I can confirm that I can reproduce the segfault locally on MacOS 11.6 with pyarrow 16.1.0 (released yesterday). It happens in tests/test_0080_flatpandas_multiindex_rows_and_columns.py

Changing back to pyarrow 16.0.0 fixes the issue.

[jpivarski]

Although we could hold back pyarrow<16.1.0, that would have to be temporary; we'd have to remove the constraint after pyarrow gets fixed. We can only expect pyarrow to get fixed if the error is reported to them, so I tried to do that, but #3122.

[ianna]

Although we could hold back pyarrow<16.1.0, that would have to be temporary; we'd have to remove the constraint after pyarrow gets fixed. We can only expect pyarrow to get fixed if the error is reported to them, so I tried to do that, but #3122.

I uninstalled both awkward and awkward-cpp and have a minimum reproducer of a segfault with:

% python
Python 3.11.5 (main, Sep 11 2023, 08:19:27) [Clang 14.0.6 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import pyarrow
zsh: segmentation fault  python

The test_pyarrow.py has only one line:
import pyarrow

 % lldb python test_pyarrow.py 
(lldb) target create "python"
Current executable set to '/Users/yana/anaconda3/bin/python' (x86_64).
(lldb) settings set -- target.run-args  "test_pyarrow.py"
(lldb) run
Process 34536 launched: '/Users/yana/anaconda3/bin/python' (x86_64)
Process 34536 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=EXC_I386_GPFLT)
    frame #0: 0x0000000129c8685e libarrow.1601.dylib`malloc_conf_init_helper + 302
libarrow.1601.dylib`malloc_conf_init_helper:
->  0x129c8685e <+302>: movzbl (%rbx), %eax
    0x129c86861 <+305>: testb  %al, %al
    0x129c86863 <+307>: je     0x129c8936e               ; <+11326>
    0x129c86869 <+313>: movq   %rbx, %rcx
(lldb) 

[jpivarski]

That reproducer must depend on the environment because it didn't segfault immediately on import for me. I also noticed in the output that this is x86-64. My laptop is ARM (M2). That's likely relevant.

Could you send as much information as you can to the Arrow developers? Since we also need a short-term solution, we can put a version cap on Arrow in our tests for now. There are a few pull requests that need it.

[ianna]

That reproducer must depend on the environment because it didn't segfault immediately on import for me. I also noticed in the output that this is x86-64. My laptop is ARM (M2). That's likely relevant.

Could you send as much information as you can to the Arrow developers? Since we also need a short-term solution, we can put a version cap on Arrow in our tests for now. There are a few pull requests that need it.

I've opened an issue apache/arrow#41696

[jpivarski]

Awesome! It looks like they're dealing with it right away.

[lgray]

This PR introduces massive data inefficiencies for HEP analyses that involve combinations of multiple flavors of leptons, or generally gluing multiple tables together. We should reconsider. I would propose we revert this PR for now, there doesn't seem to be an issue this fixes, but rather one that might happen. I appreciate that, but we should be more careful and reconcile this with the needs of the primary user base.

[jpivarski]

Replaced lazy projection with eager projection here.