PERF: optimize NaT lookups in cython modules
State: closed

By making `NaT` a cdef'd object that we can cimport, we take a module-level lookup out of each check of `if obj is NaT`.  Since we tend to do this check a lot, avoiding these global lookups can get us some mileage:

```
In [3]: vals = np.array([pd.NaT for _ in range(10**6)])
In [4]: %timeit pd.to_datetime(vals)
```

master:
```
10 loops, best of 3: 33.1 ms per loop
100 loops, best of 3: 29.9 ms per loop
10 loops, best of 3: 25.8 ms per loop
10 loops, best of 3: 20 ms per loop
10 loops, best of 3: 31 ms per loop
10 loops, best of 3: 19.8 ms per loop
10 loops, best of 3: 26.6 ms per loop
10 loops, best of 3: 32.2 ms per loop
```

PR
```
10 loops, best of 3: 13.6 ms per loop
10 loops, best of 3: 20.6 ms per loop
10 loops, best of 3: 14.1 ms per loop
100 loops, best of 3: 14.7 ms per loop
10 loops, best of 3: 14.4 ms per loop
10 loops, best of 3: 20.9 ms per loop
10 loops, best of 3: 20.8 ms per loop
100 loops, best of 3: 16.6 ms per loop
```

Labels:
name: Datetime
description: Datetime data dtype
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Performance
description: Memory or execution speed performance

Comments:
Hello @jbrockmendel! Thanks for submitting the PR.

 - There are no PEP8 issues in the file [`pandas/core/arrays/datetimes.py`](https://github.com/jbrockmendel/pandas/blob/81e8987c083abbc99628f3faa26468917f72bd2b/pandas/core/arrays/datetimes.py) !

 - There are no PEP8 issues in the file [`pandas/core/arrays/period.py`](https://github.com/jbrockmendel/pandas/blob/81e8987c083abbc99628f3faa26468917f72bd2b/pandas/core/arrays/period.py) !


Also moved some NaTType properties from the python class to the cython class, as those are supposed to be marginally more efficient
Travis failures are pickle plotting, not clearly related.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24008?src=pr&el=h1) Report
> Merging [#24008](https://codecov.io/gh/pandas-dev/pandas/pull/24008?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/5b0610b875476a6f3727d7e9bedb90d370c669b5?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/24008/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/24008?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #24008      +/-   ##
==========================================
- Coverage   42.46%   42.46%   -0.01%     
==========================================
  Files         161      161              
  Lines       51557    51554       -3     
==========================================
- Hits        21892    21890       -2     
+ Misses      29665    29664       -1
```

| Flag | Coverage Δ | |
|---|---|---|
| #single | `42.46% <100%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24008?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/arrays/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/24008/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lcy5weQ==) | `63.41% <ø> (-0.08%)` | :arrow_down: |
| [pandas/core/arrays/period.py](https://codecov.io/gh/pandas-dev/pandas/pull/24008/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3BlcmlvZC5weQ==) | `36.97% <100%> (-0.15%)` | :arrow_down: |
| [pandas/core/reshape/tile.py](https://codecov.io/gh/pandas-dev/pandas/pull/24008/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS90aWxlLnB5) | `11.69% <0%> (+0.06%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24008?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24008?src=pr&el=footer). Last update [5b0610b...8dc29f7](https://codecov.io/gh/pandas-dev/pandas/pull/24008?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24008?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@9d85b22`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/24008/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/24008?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##             master   #24008   +/-   ##
=========================================
  Coverage          ?   42.44%           
=========================================
  Files             ?      161           
  Lines             ?    51559           
  Branches          ?        0           
=========================================
  Hits              ?    21886           
  Misses            ?    29673           
  Partials          ?        0
```

| Flag | Coverage Δ | |
|---|---|---|
| #single | `42.44% <100%> (?)` | |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24008?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/arrays/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/24008/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lcy5weQ==) | `63.41% <ø> (ø)` | |
| [pandas/core/arrays/period.py](https://codecov.io/gh/pandas-dev/pandas/pull/24008/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3BlcmlvZC5weQ==) | `36.97% <100%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24008?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24008?src=pr&el=footer). Last update [9d85b22...05c2ce0](https://codecov.io/gh/pandas-dev/pandas/pull/24008?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

> I find the introduction of NAT extra-confusing. What actually does this buy?

A ton of dict lookups.  Consider `nattype.is_null_datetimelike`, in particular the line `elif val is NaT:`.  The C code for this in that status quo is:

```
  __pyx_t_4 = __Pyx_GetModuleGlobalName(__pyx_n_s_NaT); if (unlikely(!__pyx_t_4)) __PYX_ERR(0, 686, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_4);
  __pyx_t_1 = (__pyx_v_val == __pyx_t_4);
  __Pyx_DECREF(__pyx_t_4); __pyx_t_4 = 0;
  __pyx_t_3 = (__pyx_t_1 != 0);
  if (__pyx_t_3) {
```

In the PR the C code is:

```
  __pyx_t_1 = (__pyx_v_val == ((PyObject *)__pyx_v_6pandas_5_libs_6tslibs_7nattype_NAT));
  __pyx_t_3 = (__pyx_t_1 != 0);
  if (__pyx_t_3) {
```

In the status quo it has to look up "NaT" in the module-level namespace dict (and +/- refcount).  In the PR we avoid that dict lookup.  It adds up because it is a dict lookup we do _a lot_.

this adds IMHO a huge amount of mental overhead. I would either call this ``c_NaT``, leave it as ``NaT`` but make it a c-importable (not sure if that is possible), or make this a private attribute ``_NaT`` and put a cdef accessor function to return this.
> leave it as NaT but make it a c-importable (not sure if that is possible)

Agreed that would be ideal, but AFAICT cython won't allow `NaT` to be both cimport-able and import-able.

> or make this a private attribute _NaT and put a cdef accessor function to return this.

Can you elaborate on what you have in mind with "cdef accessor function"?

I'm broadly indifferent between NAT vs c_NaT vs _NaT in terms of what we call the cdef'd version within `nattype`.  I much prefer `cimport NAT as NaT` within other cython modules so only the imports are affected and not the rest of the code.
> Can you elaborate on what you have in mind with "cdef accessor function"?

I meant this, but this is not going to help the problem.
```
cdef get_c_nat():
    return _NaT
```

how about if you rename what you are calling ``NAT`` -> ``c_NaT``, then I think could be on-board (and import that as such). Then i think the distinction is very clear and explicit, no wondering is this a c-object or not. yes references in the cython modules would change but you are already changing them.
lgtm. do we have sufficient asv's for this? 
>  do we have sufficient asv's for this?

The %timeit results in the OP are all that's available ATM.  I'm poking at an idea that would make it easier to identify what asvs to run for any particular commit/PR, will see how that pans out.
thanks!