New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile numba code with nogil=True #159

Merged
merged 1 commit into from Jun 12, 2018

Conversation

Projects
None yet
2 participants
@mrocklin
Collaborator

mrocklin commented Jun 12, 2018

This helps when parallelizing around sparse

Compile numba code with nogil=True
This helps when parallelizing around sparse
@mrocklin

This comment has been minimized.

Collaborator

mrocklin commented Jun 12, 2018

Generic example:

In [1]: import numba

In [2]: import numpy as np

In [3]: x = np.random.random(100000000)

In [4]: @numba.jit(nopython=True)
   ...: def f(x):
   ...:     total = 0
   ...:     for i in range(len(x)):
   ...:         total += x[i]
   ...:     return total
   ...: 

In [5]: f(x)  # run once to compile
Out[5]: 50001234.14227918

In [6]: %load_ext ptime

In [7]: %ptime -n 4 f(x)
Total serial time:   0.52 s
Total parallel time: 0.50 s
For a 1.05X speedup across 4 threads

In [8]: @numba.jit(nopython=True, nogil=True)
   ...: def f(x):
   ...:     total = 0
   ...:     for i in range(len(x)):
   ...:         total += x[i]
   ...:     return total
   ...: 

In [9]: f(x)  # run once to compile
Out[9]: 50001234.14227918

In [10]: %ptime -n 4 f(x)
Total serial time:   0.52 s
Total parallel time: 0.14 s
For a 3.63X speedup across 4 threads
@codecov

This comment has been minimized.

codecov bot commented Jun 12, 2018

Codecov Report

Merging #159 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #159   +/-   ##
=======================================
  Coverage   96.92%   96.92%           
=======================================
  Files          11       11           
  Lines        1205     1205           
=======================================
  Hits         1168     1168           
  Misses         37       37
Impacted Files Coverage Δ
sparse/coo/indexing.py 100% <100%> (ø) ⬆️
sparse/coo/umath.py 96.85% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4104eab...d3c73cd. Read the comment docs.

@hameerabbasi

This comment has been minimized.

Collaborator

hameerabbasi commented Jun 12, 2018

Is there any risk of incorrect results when doing this, assuming no race conditions?

@mrocklin

This comment has been minimized.

Collaborator

mrocklin commented Jun 12, 2018

That's probably more a question for you than it is for me. Do any of these functions rely on global state or mutate their inputs? If they were called concurrently in multiple threads would that be a problem?

@mrocklin

This comment has been minimized.

Collaborator

mrocklin commented Jun 12, 2018

Usually the answer is "no", but we may be doing inplace modifications for performance sake,.

@mrocklin

This comment has been minimized.

Collaborator

mrocklin commented Jun 12, 2018

Previously these functions were run with a giant lock around them so that no other Python function would run at the same time. Now we are removing that lock.

@hameerabbasi

This comment has been minimized.

Collaborator

hameerabbasi commented Jun 12, 2018

Do any of these functions rely on global state or mutate their inputs?

No, they don't do that. :-) Thanks for this, I wondered why the dask.array version was as slow as it was.

@hameerabbasi

This gets a +1. Thanks for this.

@mrocklin

This comment has been minimized.

Collaborator

mrocklin commented Jun 12, 2018

I wondered why the dask.array version was as slow as it was.

There are very likely other reasons than just this. This is something that will likely improve over time as it gets some use.

@mrocklin mrocklin merged commit b51d749 into pydata:master Jun 12, 2018

4 checks passed

ci/circleci: build_27 Your tests passed on CircleCI!
Details
ci/circleci: build_36 Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 96.92%)
Details
codecov/project 96.92% (+0%) compared to 4104eab
Details

@mrocklin mrocklin deleted the mrocklin:nogil branch Jun 12, 2018

@mrocklin

This comment has been minimized.

Collaborator

mrocklin commented Jun 12, 2018

Thanks for the review @hameerabbasi . Merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment