Fix a performance regression (and address #1532)

[kevindoran]

Previous commits (starting around 1814bc1) relating to #1532 made a performance regression by forcing an array copy. It causes ~25x slowdown on my machine.

Here we remove the regression, and properly chunk-read in the event of smaller subsets of large arrays being requested, which is a fix for the problem from #1532.

Edit: I previously got confused with a related issue in SpikeingInterface, #3303, which I think is connected.

[zm711]

Thanks for this @kevindoran. Seems like in trying to fix the original issue I may have kept it the same/made it worse. Appreciate the fix. Could we have you post some sort of benchmark in a comment about the improvement in performance (more like a terminal screenshot or copy of output from the terminal rather than just saying 25x)?

I would propose that you

1. keep the performance improvement in this PR
2. split off the performance regression test into a separate PR

I suggest this because we haven't discussed as maintainers if we want to support performance testing specifically and in general we try to avoid monkey patching as much as we can. So if you open that test as a separate PR that will give us an opportunity to discuss it as a concept for Neo without preventing us from merging this PR (so long as tests pass). How's that sound?

[kevindoran]

The test is a logical test; it doesn't measure execution time or memory usage. The monkey patch just adds a counter: it counts how many read calls there is, which is something we can deduce and assert logically, for example, it shouldn't be called multiple times when the whole stimulus is requested, but should be called multiple times when a small number of channels are requested.

If it was a performance test that measure execution time (and differ between machines), I agree that it wouldn't really belong in the codebase at the moment, as there isn't a dedicated machine they could run on.

[kevindoran]

Regarding the 25x. I was doing preprocessing and a spike sort that required 3 full reads of the recording. With the fix, subsequent reads hdf5 can use read cache in ram (I think). So exact speed-ups will depend on how many reads, and the size of the recording relative to available ram (and disk and cpu).

Phase_01.brw, 8 x 65536-frame chunks = 4.29 GB per pass
timeit -n 1 -r 3

script        time                          throughput   multiple
stock_warm    ~3.6 min (prime + 3 passes)    0.080 GB/s   1x
fixed_warm    ~8 s (prime + 3 passes)        2.03 GB/s    25.4x
stock_cold    ~2.7 min (3 passes)            0.079 GB/s   1x
fixed_cold    ~13 s (3 passes)               0.97 GB/s    12.3x