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

Lazily instantiate codemirror instances #1615

Merged
merged 3 commits into from May 18, 2015

Conversation

Projects
None yet
3 participants
@samccone

By taking advantage of the fact that a user can not interact
with a codemirror instance until the editor is in the viewport, we can be
smart and only set the codemirror editors up once a user scrolls them into view.

For small notebooks this change has a limited impact, however for large
notebooks with 100s of code cells this change results in a significant
performance boost to the application, especially in the initial time to
render metric for notebooks.


Tested With http://nbviewer.ipython.org/url/norvig.com/ipython/TSPv3.ipynb

Initial setup work
  • Download the ipython notebook
  • Convert to a beaker notebook
  • Save to filesystem

Testing Process

  • Start the profiler
  • Open the bkr notebook
  • Once the initial map paints, stop the profiler

screen shot 2015-05-14 at 2 31 40 pm

screen shot 2015-05-14 at 2 31 57 pm

Source analysis


Ref #1546
Fixes #178

@ildipo

This comment has been minimized.

Show comment
Hide comment
@ildipo

ildipo May 14, 2015

The power of JS libraries....
It looks nice

ildipo commented May 14, 2015

The power of JS libraries....
It looks nice

@scottdraves

This comment has been minimized.

Show comment
Hide comment
@scottdraves

scottdraves May 14, 2015

Member

i found i could get a code cell that had not been instantiated on screen
screen shot 2015-05-14 at 3 31 30 pm

Member

scottdraves commented May 14, 2015

i found i could get a code cell that had not been instantiated on screen
screen shot 2015-05-14 at 3 31 30 pm

@samccone

This comment has been minimized.

Show comment
Hide comment
@samccone

samccone May 15, 2015

I did some follow up time measurements on my computer (these numbers will be highly variable depending on the system)

New: (lazy codecell instantiation)
16.42 seconds

Old: (eager codecell instantiation) 
26.5 seconds

Tested With http://nbviewer.ipython.org/url/norvig.com/ipython/TSPv3.ipynb

Time from clicking on the recent notebook item on the dashboard until being able to interact with the first codecell

I did some follow up time measurements on my computer (these numbers will be highly variable depending on the system)

New: (lazy codecell instantiation)
16.42 seconds

Old: (eager codecell instantiation) 
26.5 seconds

Tested With http://nbviewer.ipython.org/url/norvig.com/ipython/TSPv3.ipynb

Time from clicking on the recent notebook item on the dashboard until being able to interact with the first codecell

@scottdraves

This comment has been minimized.

Show comment
Hide comment
@scottdraves

scottdraves May 16, 2015

Member

Thanks, that's a fantastic improvement in performance.

However, I can see the code cells instantiating as I scroll, which is pretty annoying.

Instead of starting them out as unstyled text, can you initialize them to the right font size
with the box around them, but otherwise static? That should be really fast, and should look
exactly like they do after they become real CodeMirrors, except for the text changing color for
the highlighting.

Member

scottdraves commented May 16, 2015

Thanks, that's a fantastic improvement in performance.

However, I can see the code cells instantiating as I scroll, which is pretty annoying.

Instead of starting them out as unstyled text, can you initialize them to the right font size
with the box around them, but otherwise static? That should be really fast, and should look
exactly like they do after they become real CodeMirrors, except for the text changing color for
the highlighting.

samccone added some commits May 14, 2015

Lazily instantiate codemirror instances
By taking advantage of the fact that a user can not interact
with codemirror instance until they are in the viewport, we can be
smart and only set the codemirror editors up once a user scrolls them into view.

For small notebooks this change has a limited impact, however for large
notebooks with 100s of code cells this change results in a significant
performance boost to the application, especially in the initial time to
render metric for notebooks.

ref #1546
Fixes #178
@samccone

This comment has been minimized.

Show comment
Hide comment

updated

scottdraves added a commit that referenced this pull request May 18, 2015

Merge pull request #1615 from twosigma/sjs/scroll-code-mirror
Lazily instantiate codemirror instances

@scottdraves scottdraves merged commit 680747d into master May 18, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@scottdraves

This comment has been minimized.

Show comment
Hide comment
@scottdraves

scottdraves May 18, 2015

Member

🐬 🐬

Member

scottdraves commented May 18, 2015

🐬 🐬

@samccone

This comment has been minimized.

Show comment
Hide comment
@samccone

samccone May 18, 2015

👯 👯 👯 👯

👯 👯 👯 👯

@samccone samccone deleted the sjs/scroll-code-mirror branch May 18, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment