Fix performance issues when creating multiple instances of Period (#12903, #11831) #12909

rs2 · 2016-04-16T22:08:18Z

tests added / passed
passes git diff upstream/master | flake8 --diff
whatsnew entry

jreback · 2016-04-16T22:25:32Z

pandas/tseries/constants.py

@@ -0,0 +1,7 @@
+US_RESO = 0
+MS_RESO = 1


don't add another module instead put these in frequencies

jreback · 2016-04-16T22:26:04Z

pls run the asv suite for relevant benchmarks

jreback · 2016-04-16T22:32:19Z

needs a whatsnew entry

jreback · 2016-04-17T13:51:52Z

also pls show the asv runs for the period benchmarks

…test_tslib.py

…ps in asv_benchmarks/period.py

rs2 · 2016-04-20T08:57:33Z

@jreback , @tswicegood Is there a doc describing the hoops I need to jump through to get Travis and Jeff happy to merge? 😄 I.e. the names of the scripts I need to run to add asv benchmarks, pass coverage and pep8 requirements?

jorisvandenbossche · 2016-04-20T09:02:25Z

@rs2 see @jreback's previous comments:

add a note in the whatsnew file about the performance improvement (doc/source/whatsnew/v0.18.1)
run the asv benchmarks and paste the results here, see http://pandas-docs.github.io/pandas-docs-travis/contributing.html#running-the-performance-test-suite (you can select the relevant ones to run, related to periods and timeseries plotting)
for travis, you just have a failing test for which you need to fix the code, it seems related to the moving of the constants.

jreback · 2016-04-25T15:21:28Z

can you show some benchmark results (prob need some asv's for this as well).

rs2 · 2016-04-26T11:34:50Z

Will submit another pull request

jreback · 2016-04-26T12:09:53Z

just force push to this PR
opening multiple PRs for the same issue is confusing and more work for others

rs2 · 2016-04-26T12:26:06Z

The failing test is not mine.

jreback · 2016-04-26T12:38:34Z

doc/source/whatsnew/v0.18.1.txt

@@ -243,7 +243,7 @@ Performance Improvements


 - Improved performance of ``DataFrame.to_sql`` when checking case sensitivity for tables. Now only checks if table has been created correctly when table name is not lower case. (:issue:`12876`)
-
+- Improved performance of creating multiple instances of Period. This has regressed from ``O(1)`` to ``O(n)`` in terms on number of calls to ``_find_and_load`` between ``0.15.2`` and ``0.18.0``. (:issue:`12903`, :issue:`11831`)


just say improved perf in period construction and plotting of periods

jreback · 2016-04-26T12:41:56Z

@rs2 that test test runner has been known failing for a while.

codecov-io · 2016-04-26T12:54:41Z

Current coverage is 83.84%

Merging #12909 into master will increase coverage by +0.10%

@@           master     #12909   diff @@
========================================
  Files         134        136     +2   
  Lines       49565      49716   +151   
  Methods         0          0          
  Branches        0          0          
========================================
+ Hits        41507      41684   +177   
+ Misses       8058       8032    -26   
  Partials        0          0

6 files in pandas/tseries were modified. more
- Misses +4
- Hits +3
2 files in pandas/tools were modified. more
- Misses +1
- Hits +53
2 files in pandas/core were modified. more
- Misses +2
- Hits -2
4 files in pandas were modified. more
- Misses +2
- Hits +8
2 files (not in diff) in pandas were modified. more
- Misses -7
- Hits +7

Powered by Codecov. Last updated by 5cbe9f7

rs2 · 2016-04-26T13:45:04Z

Adding asv benchmarks...

rs2 · 2016-04-26T14:47:13Z

`asv` benchmarks

From 5.89s to 525ms:

Before the change:

~/pandas_master/asv_bench$ asv dev -b create_period
· Discovering benchmarks
· Running 1 total benchmarks (1 commits * 1 environments * 1 benchmarks)
[  0.00%] ·· Building for /home/pashok/anaconda3/bin/python
[  0.00%] ·· Benchmarking /home/pashok/anaconda3/bin/python
[100.00%] ··· Running period.create_period_index_from_date_range.time_period_index                                                                                                    5.89s

After the change

~/pandas/asv_bench$ asv dev -b create_period
· Discovering benchmarks
· Running 1 total benchmarks (1 commits * 1 environments * 1 benchmarks)
[  0.00%] ·· Building for /home/pashok/anaconda3/bin/python
[  0.00%] ·· Benchmarking /home/pashok/anaconda3/bin/python
[100.00%] ··· Running period.create_period_index_from_date_range.time_period_index                                                                                   525.84ms

jreback · 2016-04-26T14:52:07Z

asv_bench/benchmarks/period.py

+
+    def time_period_index(self):
+        # Simulate irregular PeriodIndex
+        PeriodIndex(date_range('1985', periods=10000).to_pydatetime(), freq='D')


just make this 1000 (it will be the same ratio, but the previous versions will take less time)

jorisvandenbossche · 2016-04-26T14:54:48Z

Nice speedup! Did you run the benchmarks on py2 or py3?

jreback · 2016-04-26T15:00:28Z

~/pandas/asv_bench$ asv continuous -b period (-b just matches so you can see results (will compare)

rs2 · 2016-04-26T15:46:42Z

@jorisvandenbossche : plot asv benchmarks:

Before the change

~/pandas_master/asv_bench$ asv dev -b foo
· Discovering benchmarks
· Running 2 total benchmarks (1 commits * 1 environments * 2 benchmarks)
[  0.00%] ·· Building for /home/pashok/anaconda3/bin/python
[  0.00%] ·· Benchmarking /home/pashok/anaconda3/bin/python
[ 50.00%] ··· Running foo_bar.test_foo_bar.time_plot_date_ranged_df                                                                                                                   1.81s
[100.00%] ··· Running foo_bar.test_foo_bar.time_plot_plain_df                                                                                                                      129.33ms
~/pandas_master/asv_bench$ python --version
Python 3.5.1 :: Anaconda 2.5.0 (64-bit)
~/pandas_master/asv_bench$

After the change

~/pandas/asv_bench$ asv dev -b foo
· Discovering benchmarks
· Running 2 total benchmarks (1 commits * 1 environments * 2 benchmarks)
[  0.00%] ·· Building for /home/pashok/anaconda3/bin/python
[  0.00%] ·· Benchmarking /home/pashok/anaconda3/bin/python
[ 50.00%] ··· Running foo_bar.test_foo_bar.time_plot_date_ranged_df                                                                                                  214.24ms
[100.00%] ··· Running foo_bar.test_foo_bar.time_plot_plain_df                                                                                                        123.06ms
~/pandas/asv_bench$ python --version
Python 3.5.1 :: Anaconda 2.5.0 (64-bit)
~/pandas/asv_bench$

import numpy as np
import pandas as pd


class test_foo_bar(object):

    goal_time = 0.2
    N = 2000

    def setup(self):
        self.date_ranged_df = pd.DataFrame(np.random.randn(self.N, 5), index=pd.date_range('1/1/1975', periods=self.N))
        self.plain_df = pd.DataFrame(np.random.randn(self.N, 5))

    def time_plot_date_ranged_df(self):
        self.date_ranged_df.plot()

    def time_plot_plain_df(self):
        self.plain_df.plot()

rs2 · 2016-04-26T19:02:30Z

Anything else needs to be done to be able to merge?

jreback · 2016-04-26T19:06:17Z

looks good. ping when green.

jreback · 2016-04-26T19:21:48Z

thanks @rs2

blbradley · 2016-04-26T20:06:16Z

This was a big TODO I had when moving Period to Cython. Thank you for working it out @rs2.

Fix performance issues when creating multiple instances of Period (#1…

fec1b51

…2903, #11831)

rs2 mentioned this pull request Apr 16, 2016

Import statements in period.pyx significantly impact performance #12903

Closed

jreback reviewed Apr 16, 2016
View reviewed changes

pandas/tseries/constants.py

@@ -0,0 +1,7 @@

US_RESO = 0

MS_RESO = 1

Copy link

Contributor

jreback Apr 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't add another module instead put these in frequencies

jreback added Performance Memory or execution speed performance Period Period data type labels Apr 17, 2016

rs2 added 5 commits April 19, 2016 22:45

Moved constants to frequencies.py

5b3e291

Added a whatsnew entry + ensured constants are imported correctly by …

8f254e3

…test_tslib.py

Reworded whatsnew

8bcfd57

Added asv benchmark for Period, PeriodIndex

1c5a2ab

Make RESO constants global in period.pyx and reduce the number of loo…

0d9712d

…ps in asv_benchmarks/period.py

rs2 closed this Apr 26, 2016

rs2 reopened this Apr 26, 2016

jreback reviewed Apr 26, 2016
View reviewed changes

jreback added this to the 0.18.1 milestone Apr 26, 2016

jreback closed this in 7fbc600 Apr 26, 2016

rs2 mentioned this pull request Feb 6, 2017

Not all commits are appearing as contributions resume/resume.github.com#242

Closed

jorisvandenbossche mentioned this pull request Nov 9, 2018

PERF: Speeds up creation of Period, PeriodArray, with Offset freq #23589

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix performance issues when creating multiple instances of Period (#12903, #11831) #12909

Fix performance issues when creating multiple instances of Period (#12903, #11831) #12909

rs2 commented Apr 16, 2016 •

edited

Loading

jreback Apr 16, 2016

jreback commented Apr 16, 2016

jreback commented Apr 16, 2016

jreback commented Apr 17, 2016

rs2 commented Apr 20, 2016

jorisvandenbossche commented Apr 20, 2016

jreback commented Apr 25, 2016

rs2 commented Apr 26, 2016

jreback commented Apr 26, 2016

rs2 commented Apr 26, 2016

jreback Apr 26, 2016

jreback commented Apr 26, 2016

codecov-io commented Apr 26, 2016 •

edited

Loading

rs2 commented Apr 26, 2016

rs2 commented Apr 26, 2016

jreback Apr 26, 2016

jorisvandenbossche commented Apr 26, 2016

jreback commented Apr 26, 2016

rs2 commented Apr 26, 2016 •

edited

Loading

rs2 commented Apr 26, 2016

jreback commented Apr 26, 2016

jreback commented Apr 26, 2016

blbradley commented Apr 26, 2016

		@@ -243,7 +243,7 @@ Performance Improvements


		- Improved performance of ``DataFrame.to_sql`` when checking case sensitivity for tables. Now only checks if table has been created correctly when table name is not lower case. (:issue:`12876`)

		- Improved performance of creating multiple instances of Period. This has regressed from ``O(1)`` to ``O(n)`` in terms on number of calls to ``_find_and_load`` between ``0.15.2`` and ``0.18.0``. (:issue:`12903`, :issue:`11831`)

Fix performance issues when creating multiple instances of Period (#12903, #11831) #12909

Fix performance issues when creating multiple instances of Period (#12903, #11831) #12909

Conversation

rs2 commented Apr 16, 2016 • edited Loading

jreback Apr 16, 2016

Choose a reason for hiding this comment

jreback commented Apr 16, 2016

jreback commented Apr 16, 2016

jreback commented Apr 17, 2016

rs2 commented Apr 20, 2016

jorisvandenbossche commented Apr 20, 2016

jreback commented Apr 25, 2016

rs2 commented Apr 26, 2016

jreback commented Apr 26, 2016

rs2 commented Apr 26, 2016

jreback Apr 26, 2016

Choose a reason for hiding this comment

jreback commented Apr 26, 2016

codecov-io commented Apr 26, 2016 • edited Loading

Current coverage is 83.84%

rs2 commented Apr 26, 2016

rs2 commented Apr 26, 2016

asv benchmarks

Before the change:

After the change

jreback Apr 26, 2016

Choose a reason for hiding this comment

jorisvandenbossche commented Apr 26, 2016

jreback commented Apr 26, 2016

rs2 commented Apr 26, 2016 • edited Loading

Before the change

After the change

rs2 commented Apr 26, 2016

jreback commented Apr 26, 2016

jreback commented Apr 26, 2016

blbradley commented Apr 26, 2016

rs2 commented Apr 16, 2016 •

edited

Loading

codecov-io commented Apr 26, 2016 •

edited

Loading

`asv` benchmarks

rs2 commented Apr 26, 2016 •

edited

Loading