man-group / arctic

High performance datastore for time series and tick data
https://arctic.readthedocs.io/en/latest/
GNU Lesser General Public License v2.1
3.06k stars 584 forks source link

Compression requires GIL in prange #529

Closed bashtage closed 6 years ago

bashtage commented 6 years ago

Arctic Version

# 1.64 (Master)

Arctic Store

# VersionStore, TickStore, or ChunkStore

Platform and version

Any

Description of problem and/or code sample that reproduces the issue

_compressarr does not use @cython.boundscheck(False) and so some of the advantage of using prange may be lost;

This may be by design although I don't see why the GIL would be required here since there are no Python manipulations.

image

jamesblackburn commented 6 years ago

Very likely because we didn't know better :)

Would gratefully accept a PR with some benchmark showing the speedup.

bmoscon commented 6 years ago

that just looks like its saving the state of the gil and releasing if necessary, or am i missing something? The only block in there that is explicitly without the GIL is the with nogil block

jamesblackburn commented 6 years ago

Fixed in: pull/530