Performance regression in tslibs.tz_convert.TimeTZConvert.time_tz_convert_from_utc

[TomAugspurger]

https://pandas.pydata.org/speed/pandas/index.html#tslibs.tz_convert.TimeTZConvert.time_tz_convert_from_utc?p-size=1000000&p-tz=datetime.timezone.utc&commits=3701a9b1bfc3ad3890c2fb1fe1974a4768f6d5f8-522f855a38d52763c4e2b07480786163b91dad38

Somewhere in 3701a9b...522f855

[jorisvandenbossche]

To be clear, it's only a regression on master, after 1.1 (so only a target for 1.2, not 1.1.1) ?

[jorisvandenbossche]

Quite likely cause from that commit range: #35532

[jorisvandenbossche]

cc @lidavidm @jbrockmendel

Looking at the PR, it might also be caused by the added copy ? (the regression only happened for the case of UTC timezone, see view) In which case it might be something to ignore?

[lidavidm]

Probably because of the copy. As originally written, the patch did not involve the copy, if that's worth testing. UTC might be a fairly common timezone to run into as well.

(Side note, that's some super impressive benchmark infrastructure!)

[jbrockmendel]

It makes sense that the copy would hurt perf, but we would still expect the UTC case to be more performant than any of the other cases, which it no longer is

[lidavidm]

This is the patch without the copy:

From b4a1de2988ad85c92a705a9a131f0573696a94fd Mon Sep 17 00:00:00 2001
From: David Li <[email protected]>
Date: Mon, 3 Aug 2020 16:06:32 -0400
Subject: [PATCH] BUG: handle immutable arrays in tz_convert_from_utc (#35530)

---
 pandas/_libs/tslibs/tzconversion.pyx | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/pandas/_libs/tslibs/tzconversion.pyx b/pandas/_libs/tslibs/tzconversion.pyx
index 2b148cd88..2b24982d9 100644
--- a/pandas/_libs/tslibs/tzconversion.pyx
+++ b/pandas/_libs/tslibs/tzconversion.pyx
@@ -410,7 +410,7 @@ cpdef int64_t tz_convert_from_utc_single(int64_t val, tzinfo tz):
         return val + deltas[pos]
 
 
-def tz_convert_from_utc(int64_t[:] vals, tzinfo tz):
+def tz_convert_from_utc(const int64_t[:] vals, tzinfo tz):
     """
     Convert the values (in i8) from UTC to tz
 
@@ -424,7 +424,7 @@ def tz_convert_from_utc(int64_t[:] vals, tzinfo tz):
     int64 ndarray of converted
     """
     cdef:
-        int64_t[:] converted
+        const int64_t[:] converted
 
     if len(vals) == 0:
         return np.array([], dtype=np.int64)
@@ -435,7 +435,7 @@ def tz_convert_from_utc(int64_t[:] vals, tzinfo tz):
 
 @cython.boundscheck(False)
 @cython.wraparound(False)
-cdef int64_t[:] _tz_convert_from_utc(int64_t[:] vals, tzinfo tz):
+cdef const int64_t[:] _tz_convert_from_utc(const int64_t[:] vals, tzinfo tz):
     """
     Convert the given values (in i8) either to UTC or from UTC.
 
@@ -457,7 +457,7 @@ cdef int64_t[:] _tz_convert_from_utc(int64_t[:] vals, tzinfo tz):
         str typ
 
     if is_utc(tz):
-        converted = vals
+        return vals
     elif is_tzlocal(tz):
         converted = np.empty(n, dtype=np.int64)
         for i in range(n):
-- 
2.28.0

[jreback]

@lidavidm care to do a PR?

[lidavidm]

Will do, sorry about the delay here