Refactor registry init. by stuartarchibald · Pull Request #6948 · numba/numba

stuartarchibald · 2021-04-20T09:17:08Z

This moves the implementations of core supported features out of
the base context and into explicit targets. This is to ensure that
targets only carry implementations of things they actually
support.

This moves the implementations of core supported features out of the base context and into explicit targets. This is to ensure that targets only carry implementations of things they actually support.

gmarkall · 2021-04-20T09:55:58Z

from numba import cuda
import numpy as np


@cuda.jit
def f(x):
    s = slice(1, 10, 2)
    x[0] = s.start
    x[1] = s.stop
    x[2] = s.step


x = np.zeros(3, dtype=np.int64)

f[1, 1](x)

print(x)

prints

[ 1 10  2]

with this PR even though the slicing registry is not installed on the CUDA target - how is it working now? (Not that it shouldn't be, of course!)

stuartarchibald · 2021-04-20T10:46:07Z

RE slicing. Seems like a lot of registries are being populated as a side effect of import. In the case of slicing it's stencil from numba's __init__ which triggers an import chain that reaches it.

As title.

stuartarchibald · 2021-04-20T14:14:16Z

+        unexpected = set(banlist) & set(modlist)
        self.assertFalse(unexpected, "some modules unexpectedly imported")

+    def test_no_impl_import(self):


@gmarkall thoughts on having a test like this for the CUDA target? Essentially something to make sure that from numba import cuda / from numba.cuda import jit doesn't trigger import of modules that are known to have registration side effects.

Sounds good, I'll have an experiment with adding a test like this.

Seems that from numba import cuda ends up importing numba.cpython.cmathimpl and numba.cpython.mathimpl.

... though, this seems to be quite easily fixable.

Test and fix added in 0611f05 - what do you think @stuartarchibald ?

Thanks @gmarkall, the patch looks good. Are you happy for me to include it in this branch?

Yes please!

Great, thanks for confirming :)

As title.

stuartarchibald · 2021-04-20T16:39:40Z

RE slicing. Seems like a lot of registries are being populated as a side effect of import. In the case of slicing it's stencil from numba's __init__ which triggers an import chain that reaches it.

The rest of the patches on this PR are to deal with this! They make it so that modules that register lowering implementations are not imported at import time, or at decoration time, but only once compilation starts. The side effect of this is that nothing (not even the NRT) is jit compiled on import, this speeds up import times by about 40% locally. These patches also move all the lowering registrations out of the base context and onto the specific targets, such that e.g. the CPU just registers things it needs and CUDA just registers things it needs.

sklam · 2021-04-21T22:48:02Z

@stuartarchibald, the only way I can think of to avoid the move for typed.Dict and typed.List is to use PEP562 to redirect the import. I tried it at sklam@8230acb. Is it valid?

stuartarchibald · 2021-04-22T15:18:53Z

@stuartarchibald, the only way I can think of to avoid the move for typed.Dict and typed.List is to use PEP562 to redirect the import. I tried it at sklam@8230acb. Is it valid?

Thanks @sklam, I think this approach may well work, it's similar to previous shims. As discussed OOB, first Python 3.6 removal is required, will open a new PR for that.

This also required a small fix to prevent the registration of mathimpl and cmathimpl when `cuda` was imported.

Conflicts: numba/cuda/target.py

stuartarchibald · 2021-04-27T11:38:09Z

@gmarkall @sklam both your patches are now on this branch, many thanks.

As title.

stuartarchibald · 2021-04-28T16:09:39Z

+# Initialize typed containers
+import numba.typed


@sklam Do you recall why this is here? Could it go into the CPU context?

I think this is needed to register the typed containers for cases where they are used by implicitly; e.g. d = {}

sklam

The changes look good. It's good to see the reduction in import time and that the import side-effects are better controlled.

I profile the import time again from python -X importtime -c "import numba". Here is a descending list of cumulative import time:

       self |  cumulative | package
       0.2% |       26.3% | numba.core.config
       0.1% |       21.6% | llvmlite.binding
       0.1% |       21.2% | numba.core.decorators
       0.1% |       20.9% | numba.stencils.stencil
       0.1% |       19.7% | numba.core.types
       0.1% |       19.6% | llvmlite.binding.dylib
       1.1% |       19.0% | llvmlite.binding.ffi
       7.0% |       17.8% | pkg_resources
       5.3% |       17.2% | numpy
       0.1% |       14.7% | numba.core.registry
       0.1% |       14.5% | numba.core.dispatcher
       0.7% |       14.1% | numba.core.compiler
       0.1% |        6.4% | numba.core.callconv
       1.3% |        6.3% | numba.core.base
       0.0% |        5.9% | numba.testing
       0.2% |        5.6% | numba.core.ir_utils

If we are interested in further importtime reduction, we should try using PEP562 in numba/__init__.py to defer more of the import

stuartarchibald · 2021-04-29T18:17:42Z

Thanks for the review @sklam.

Refactor registry init.

f5dc3e6

This moves the implementations of core supported features out of the base context and into explicit targets. This is to ensure that targets only carry implementations of things they actually support.

stuartarchibald requested review from esc and sklam as code owners April 20, 2021 09:17

stuartarchibald added the 3 - Ready for Review label Apr 20, 2021

stuartarchibald added this to the Numba 0.54 RC milestone Apr 20, 2021

stuartarchibald added the Effort - short Short size effort needed label Apr 20, 2021

stuartarchibald added 3 commits April 20, 2021 14:57

Delay imports and force registration off the import path.

baa912b

As title.

Enforce import strictness

98f733f

Fix missing type annotation

c4387fe

stuartarchibald commented Apr 20, 2021

View reviewed changes

stuartarchibald added 3 commits April 20, 2021 15:19

Fix hashing test

7014338

Fix issue with typed list and unroller.

14d7c0e

As title.

Load lowering on refresh not init

295e836

stuartarchibald added Effort - long Long size effort needed and removed Effort - short Short size effort needed labels Apr 20, 2021

gmarkall and others added 4 commits April 27, 2021 12:33

Add test that importing CUDA has no registration side effects

e152f4d

This also required a small fix to prevent the registration of mathimpl and cmathimpl when `cuda` was imported.

Use PEP562 to delay import

dc81b71

Merge branch 'master' into wip/registry_refactor

c4f8579

Conflicts: numba/cuda/target.py

Remove cmathimpl from CUDA import sequence

eac2e47

stuartarchibald added 5 commits April 27, 2021 13:22

Fix PEP 562 usage.

b664476

As title.

Fix CUDA datetime support.

dd7541e

As title.

Fix flake8

00ead57

Dead code removal

80da0ef

Make jitclass boxing delayed

ea0403f

stuartarchibald commented Apr 28, 2021

View reviewed changes

sklam approved these changes Apr 28, 2021

View reviewed changes

sklam added 5 - Ready to merge Review and testing done, is ready to merge and removed 3 - Ready for Review labels Apr 28, 2021

sklam merged commit 1c5a9cd into numba:master Apr 29, 2021

stuartarchibald mentioned this pull request Apr 30, 2021

Remove use of jit on Numba import. #6161

Closed

gmarkall mentioned this pull request May 4, 2021

CUDA: Remove catch of NotImplementedError in target.py #6997

Merged

stuartarchibald mentioned this pull request Sep 8, 2021

nrt_external API broken in 0.54 #7385

Closed

2 tasks done

stuartarchibald mentioned this pull request Dec 15, 2021

Thread safety issue in compiler init #2517

Closed

stuartarchibald mentioned this pull request Oct 19, 2022

Stub handling for type check purposes #8521

Open

Conversation

stuartarchibald commented Apr 20, 2021

Uh oh!

gmarkall commented Apr 20, 2021

Uh oh!

stuartarchibald commented Apr 20, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuartarchibald commented Apr 20, 2021

Uh oh!

sklam commented Apr 21, 2021

Uh oh!

stuartarchibald commented Apr 22, 2021

Uh oh!

stuartarchibald commented Apr 27, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sklam left a comment

Choose a reason for hiding this comment

Uh oh!

stuartarchibald commented Apr 29, 2021

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants