Antoine Pitrou

Results 823 comments of Antoine Pitrou

Another issue is this reproducer for the C/GLib failures: ```python >>> i1 = pa.array([1,0], pa.int16()) >>> i2 = pa.array([2], pa.int16()) >>> indices = pa.chunked_array([i1, i2]) >>> pa.array([1,0,2], pa.int16()).take(indices) [ ,...

It should be noted that this entails significant regression on our micro-benchmarks (when only one thread is working and there's little memory pressure): ``` --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Non-regressions: (14) --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- benchmark baseline...

Note that there's something inherently suboptimal in this PR: we're trading the concatenation of the chunked values (essentially allocating a new values array) against the resolution of many chunked indices...

> Yeah, but this kind of branching is really bad for the confidence in unit tests. I will see what can be done. We can do that in a later...

Something you could do in this PR, though, is to resolve the indices in chunks of say 1024. This would at least ease the allocation cost and memory pressure.

I submitted PR #43772 to add benchmarks with a smaller selection factor (the current take benchmarks would have a selection factor of 1.0). We could merge that PR before yours,...

I ran the new benchmarks (those with a small selection factor) and the results are more varied, see below: ``` ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Non-regressions: (13) ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- benchmark baseline contender change % counters...

> @pitrou wouldn't it make sense to keep the responsibility for concatenation to a layer above the kernels? Like a query optimizer? They are in a better position to make...

By building on this (arguably simplified) analysis: > we're trading the concatenation of the chunked values (essentially allocating a new values array) against the resolution of many chunked indices (essentially...