Skip to content

fix n^2 performance issue in coalesce_streams preprocessor#5535

Merged
ivanov merged 3 commits into
ipython:masterfrom
minrk:nbconvert-debug
Apr 8, 2014
Merged

fix n^2 performance issue in coalesce_streams preprocessor#5535
ivanov merged 3 commits into
ipython:masterfrom
minrk:nbconvert-debug

Conversation

@minrk
Copy link
Copy Markdown
Member

@minrk minrk commented Apr 6, 2014

for n consecutive stream outputs, \r fix would be compiled n times, and applied to each ith output (n-i) times.

  • move pattern to module level
  • apply replacement after coalescing outputs

An example notebook from nbviewer with ~1k outputs that was taking 90 seconds to render now takes 3 seconds.

@minrk
Copy link
Copy Markdown
Member Author

minrk commented Apr 6, 2014

Also fix some logging, so this can be more easily debugged in the future, and various typos, etc. found in nearby docstrings.

@minrk
Copy link
Copy Markdown
Member Author

minrk commented Apr 6, 2014

This bug is bad enough that I think it's actually what is responsible for bringing down nbviewer recently.

@minrk minrk added this to the 2.1 milestone Apr 6, 2014
minrk added 2 commits April 5, 2014 18:34
for n consecutive stream outputs, `\r` fix would be compiled n times,
and applied to each output (n-i) times.

- move pattern to module level
- apply replacement after coalescing outputs
also cleanup spelling and grammar in a few docstrings
Comment thread IPython/nbformat/v3/nbbase.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How about just making output_type not be a keyword argument/have a default argument of None?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

better, thanks

@rgbkrk
Copy link
Copy Markdown
Member

rgbkrk commented Apr 7, 2014

At least for nbviewer, it seems like we would find this better if we were tracking render/load times more aggressively. Right now it all appears in the logs, but we're not tracking performance of the same notebooks over time.

and fix bug that this would have revealed
ivanov added a commit that referenced this pull request Apr 8, 2014
fix n^2 performance issue in coalesce_streams preprocessor
@ivanov ivanov merged commit 5f909c1 into ipython:master Apr 8, 2014
@minrk minrk deleted the nbconvert-debug branch April 8, 2014 00:30
minrk added a commit that referenced this pull request Apr 9, 2014
…rocessor

for n consecutive stream outputs, `\r` fix would be compiled n times, and applied to each ith output (n-i) times.

- move pattern to module level
- apply replacement after coalescing outputs

An example notebook from nbviewer with ~1k outputs that was taking 90 seconds to render now takes 3 seconds.
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
fix n^2 performance issue in coalesce_streams preprocessor
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.

3 participants