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

Find available kernelspecs more efficiently #3136

Merged
merged 2 commits into from Dec 19, 2017

Conversation

Projects
None yet
3 participants
@takluyver
Copy link
Member

takluyver commented Dec 15, 2017

Using find_kernel_specs() followed by get_kernel_spec() has quadratic performance with the number of available kernels, because it effectively rescans for each available kernel. Hands up, I designed this, and I can't think why I did it this way.

I thought this was going to require some ugly workaround, but thankfully @captainsafia was on the case two years ago, adding the get_all_specs() method (jupyter/jupyter_client#93). So all I've done here is switch the notebook code to use it.

This should improve performance loading pages for anyone with several kernels installed.

Closes gh-3135

@takluyver

This comment has been minimized.

Copy link
Member Author

takluyver commented Dec 15, 2017

It's failing the test for listing kernelspecs when one is invalid, and I don't think that can easily be fixed here; I'll need to change it in jupyter_client.

@takluyver

This comment has been minimized.

Copy link
Member Author

takluyver commented Dec 18, 2017

Reopening to test with jupyter_client 5.2, which was just released.

@takluyver takluyver closed this Dec 18, 2017

@takluyver takluyver reopened this Dec 18, 2017

@blink1073

This comment has been minimized.

Copy link
Member

blink1073 commented Dec 18, 2017

Looks like appveyor will fail until the jupyter_client conda package makes its way through the wickets.

@takluyver

This comment has been minimized.

Copy link
Member Author

takluyver commented Dec 19, 2017

Reopening to test with jupyter_client 5.2, which was just released.

@takluyver takluyver closed this Dec 19, 2017

@takluyver takluyver reopened this Dec 19, 2017

setup.py Outdated
@@ -148,7 +148,7 @@
'ipython_genutils',
'traitlets>=4.2.1',
'jupyter_core>=4.4.0',
'jupyter_client',
'jupyter_client>=4.2.0',

This comment has been minimized.

@minrk

minrk Dec 19, 2017

Member

5.2, since it needed that to pass the tests? Or is the invalid-spec issue separate?

This comment has been minimized.

@takluyver

takluyver Dec 19, 2017

Author Member

Yeah, it probably should be 5.2.

@blink1073

This comment has been minimized.

Copy link
Member

blink1073 commented Dec 19, 2017

Thanks!

@blink1073 blink1073 merged commit 782e9ce into jupyter:master Dec 19, 2017

4 checks passed

codecov/patch 90.9% of diff hit (target 0%)
Details
codecov/project Absolute coverage decreased by -0.03% but relative coverage increased by +12.38% compared to ca50f1a
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.