Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Min/Max performance #22

Merged
merged 2 commits into from
Oct 10, 2021
Merged

Improve Min/Max performance #22

merged 2 commits into from
Oct 10, 2021

Conversation

DavidPratt512
Copy link
Contributor

Code Changes

I have found that initializing a namedtuple costs about twice the time as initializing a tuple. With respect to this module, I setup a list of 1M single-digit integers to profile the rolling objects in the minmax module against using the line_profiler tool. I used a window size of 50 -- although the particulars of how many numbers and window sizing is inconsequential to the profiling results below.

Briefly, here are the results of profiling the original _update_window() function for the Max object:

Total time: 4.00087 s
File: rolling/rolling/minmax.py
Function: _update_window at line 136

Line #      Hits         Time  Per Hit   % Time  Line Contents
==============================================================
   136                                               @profile
   137                                               def _update_window(self, new):
   138    999951     503410.0      0.5     12.6          self._i += 1
   139    999951     961123.0      1.0     24.0          new_pair = pair(new, self._i + self.window_size)
   140                                                   # remove smaller values from the end of the buffer
   141   1999390    1052818.0      0.5     26.3          while self._buffer and self._buffer[-1].value <= new:
   142    999439     522761.0      0.5     13.1              self._buffer.pop()
   143    999951     466735.0      0.5     11.7          self._buffer.append(new_pair)
   144                                                   # remove any maxima that die on this iteration
   145   1000464     493749.0      0.5     12.3          while self._buffer[0].death <= self._i:
   146       513        272.0      0.5      0.0              self._buffer.popleft()

Of note is the Per Hit field. According to the line_profiler documentation, this is

the average amount of time spent executing the line once in the timer's units.

The timer's unit for this particular test is 1 microsecond.

Line 139 takes one microsecond to run due to creating the pair namedtuple object.

Switching from namedtuples to tuples, here are the results after profiling:

Total time: 3.46069 s
File: rolling/rolling/minmax.py
Function: _update_window at line 139

Line #      Hits         Time  Per Hit   % Time  Line Contents
==============================================================
   139                                               @profile
   140                                               def _update_window(self, new):
   141    999951     473326.0      0.5     13.7          self._i += 1
   142                                                   #new_pair = pair(new, self._i + self.window_size)
   143    999951     452888.0      0.5     13.1          new_pair = (new, self._i + self.window_size)
   144                                                   # remove smaller values from the end of the buffer
   145   1999390    1072247.0      0.5     31.0          while self._buffer and value(self._buffer[-1]) <= new:
   146    999439     474975.0      0.5     13.7              self._buffer.pop()
   147    999951     453792.0      0.5     13.1          self._buffer.append(new_pair)
   148                                                   # remove any maxima that die on this iteration
   149   1000464     533205.0      0.5     15.4          while death(self._buffer[0]) <= self._i:
   150       513        258.0      0.5      0.0              self._buffer.popleft()

Here, you can see line 143 running in half the time as before.

A final (very minor) change is to use if over while when removing any maxima/minima that should exit the window. Making this change won't disrupt the end-result because at most one value enters the window in any iteration. Here are the results after profiling:

Total time: 3.53752 s
File: rolling/rolling/minmax.py
Function: _update_window at line 137

Line #      Hits         Time  Per Hit   % Time  Line Contents
==============================================================
   137                                               @profile
   138                                               def _update_window(self, new):
   139    999951     511497.0      0.5     14.5          self._i += 1
   140    999951     467928.0      0.5     13.2          new_pair = (new, self._i + self.window_size)
   141                                                   # remove smaller values from the end of the buffer
   142   1999390    1094480.0      0.5     30.9          while self._buffer and value(self._buffer[-1]) <= new:
   143    999439     475525.0      0.5     13.4              self._buffer.pop()
   144    999951     454227.0      0.5     12.8          self._buffer.append(new_pair)
   145                                                   # remove any maxima that die on this iteration
   146    999951     533587.0      0.5     15.1          if death(self._buffer[0]) <= self._i:
   147       513        279.0      0.5      0.0              self._buffer.popleft()

The notable metric here is the reduced number of Hits on line 146 relative to the previous two profiling results.

Bookkeeping

All tests pass.
I have also incremented the patch version and added versions 0.3.0 and 0.3.1 (this PR) to the changelog.

Namedtuples have about twice the time overhead as tuples when
initializing. This change reduces the runtime of the _update_window()
function in the minmax module by about 12%.
Update changelog to record 0.3.0 and 0.3.1 changes.
@ajcr
Copy link
Owner

ajcr commented Oct 10, 2021

This looks great! Thank you very much for the detailed analysis and the code improvements, @DavidPratt512

I'll try and push a new release to pypi in the next few days (I think it's been quite a while since the last release and others will benefit from the changes made in this PR).

@ajcr ajcr merged commit ec1f0fc into ajcr:master Oct 10, 2021
@DavidPratt512
Copy link
Contributor Author

Hey @ajcr,

No problem :) Happy to help.

@DavidPratt512 DavidPratt512 deleted the minmax-performance branch October 10, 2021 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants