Refactor Dispatcher to remove unnecessary indirection

[gmarkall]

This PR refactors the dispatcher so that the split between the C and C++ implementations no longer exists - the entire implementation is now in C++. The consensus in recent discussions was that this split is probably an artefact of the constraints of various C and C++ compilers of years past that no longer apply. This refactoring is mostly to remove the indirection the split caused, with one or two more modern idioms applied.

I've left in my commit history whilst making the changes so that the working is apparent. In summary, the changes required:

• Removal of _dispatcher.h and _dispatcherimpl.cpp.
• Renaming _dispatcher.c to _dispatcher.cpp
• Moving the methods and members of the Dispatcher object from _dispatcherimpl.cpp into the DispatcherObject in _dispatcher.cpp - note that this was then renamed to Dispatcher.
• Where the C proxy methods were used (e.g. dispatcher_resolve) they are replaced with calls to the Dispatcher methods instead (e.g. self->resolve).
• Replacement of C-style includes (string.h etc.) with C++ ones (cstring etc).
• Replacement of malloc / free with new / delete operators
• Some other minor changes to enable compilation of the dispatcher code with a C++ (e.g. casting string literals to (char*) as necessary).
• Now that _typeof.c is included by a C++ source, it has an extern "C" block added as appropriate.

[stuartarchibald]

/AzurePipelines run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[sklam]

I'm not sure about this. If there are multiple definitions, it should be an ambiguous resolution.

[gmarkall]

How could there be multiple definitions with no arguments?

[gmarkall]

(Note that this was just moved from https://github.com/numba/numba/pull/6349/files#diff-56cb1358ce78e8b6ba8ea05ac6a688da59d236c53f039245b01cb24a15364161L31-L35)

[sklam]

ah ok.

btw, there shouldn't be multiple definitions for no arguments. It will have to be a mistake.

[stuartarchibald]

Thanks for undertaking this refactor, it's remove a lot of indirection that was previously required due to Python 2.7/General C++ cross compatibility issues. The changes look good in general with just a few minor comments to resolve. Once done I'll push it through the build farm. Thanks again!

[gmarkall]

@stuartarchibald Many thanks for the review - I believe all comments are now addressed.

[stuartarchibald]

Thanks for the fix-ups, looks good.

[stuartarchibald]

Buildfarm ID: numba_smoketest_cuda_137.

[stuartarchibald]

Buildfarm ID: numba_smoketest_cpu_86.

[stuartarchibald]

Buildfarm ID: numba_smoketest_cpu_86.

passed

[stuartarchibald]

Buildfarm ID: numba_smoketest_cuda_137.

passed.

[stuartarchibald]

@sklam given the inherent risk involved when this part of the code is changed, did you want to take a look at this too? I've reviewed and the changes seem good, build farm is happy.

[sklam]

The changes look good.

I also did a quick benchmark on the dispatcher overhead with the following ipython script:

from numba import njit


@njit
def arity_0():
    return


@njit
def arity_1(x):
    return

@njit
def arity_2(x, y):
    return


@njit
def arity_n(*args):
    return

%timeit arity_0()
%timeit arity_1(1)
%timeit arity_2(1, 2)
%timeit arity_n(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)

On master 3d18f14, i'm getting:

62.5 ns ± 0.768 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
135 ns ± 3.04 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
150 ns ± 2.72 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
407 ns ± 10.2 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

With this patch, I get:

59 ns ± 1.49 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
121 ns ± 2.79 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
149 ns ± 2.42 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
387 ns ± 1.49 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

There's a clear reduction of overhead; esp, for 0-arity function.