Skip to content

Small speedup improvement on the inspection logic in Python #311

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

Merged
merged 1 commit into from
Aug 30, 2024

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou commented Aug 30, 2024

In addition to #309 to get through improving the extension performance #307


This PR adds more checks for simple primitive types before doing the "heavy load" for more complex types.

In this situation:

Screenshot from 2024-08-30 11-34-40

It goes from:
803 ms ± 19.1 ms per loop (mean ± std. dev. of 7 runs, 1 loop each) for _jupyterlab_variableinspector_dict_list
to
265 ms ± 14 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

Note that this means without #309 and this PR we were adding almost 1 second delay for each cell execution when the variable inspector UI was not open. Even though I'm not testing with so many variables and my arrays aren't too big.

This is a first PR of hopefully more performance improvements

@martinRenou martinRenou added the enhancement New feature or request label Aug 30, 2024
Copy link

Binder 👈 Launch a Binder on branch martinRenou/jupyterlab-variableInspector/small_speedup

@martinRenou martinRenou requested a review from krassowski August 30, 2024 09:36
@martinRenou
Copy link
Member Author

martinRenou commented Aug 30, 2024

cc. @trungleduc @achhina

@krassowski
Copy link
Collaborator

I guess a lot of the penalty is in stringification:

if str(obj)[0] == "<":
    return False
if  v in ['__np', '__pd', '__pyspark', '__tf', '__K', '__torch', '__ipywidgets', '__xr']:
    return obj is not None
if str(obj).startswith("_Feature"):
    # removes tf/keras objects
    return False

If object takes long to convert to string we are wasting a long time doing it twice just to peek on whether it starts with < or _Feature. We can probably save some milliseconds by moving the str() call to end or ideally getting rid of it altogether (I think for tf/keras we could look at str(type(obj)) to avoid importing the thing)

Copy link
Collaborator

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

Looks good to me, again let's merge the main in to make sure CI passes

@martinRenou
Copy link
Member Author

Thanks for your suggestion, I will have a try

@martinRenou martinRenou merged commit 7cdba26 into jupyterlab-contrib:main Aug 30, 2024
6 checks passed
@martinRenou martinRenou deleted the small_speedup branch August 30, 2024 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants