Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: Improve Jupyter performance #163

Merged
merged 6 commits into from
Feb 16, 2024

Conversation

DanielSchiavini
Copy link
Collaborator

What I did

Improved the jupyter plugin performance considerably
Fixed the empty elements while calling javascript

How I did it

  • Minified JS with command npx uglify-js --mangle -o jupyter.min.js -- jupyter.js
  • Instead of injecting the whole ethersJS library, just inline the wallet calls
  • Only inject the JS triggers once for JupyterLab - in colab this does not work

How to verify it

  • Jupyter plugin should keep on working but much better

Description for the changelog

  • Improved and simplified JupyterLab and Colab plugin

Cute Animal Picture

image

- Minified JS with command `npx uglify-js --mangle -o jupyter.min.js -- jupyter.js`
- Instead of injecting the whole ethersJS library, just inline the wallet calls
@DanielSchiavini DanielSchiavini changed the title Improve Jupyter performance fix: Improve Jupyter performance Feb 16, 2024
@charles-cooper
Copy link
Member

i'm not convinced we want the complexity of introducing minification into the build process. fwiw after compression the minified version saves 900 bytes:

(boa) ~/titanoboa $ gzip -v < boa/integrations/jupyter/jupyter.js | wc
 61.7%
      6      47    1822
(boa) ~/titanoboa $ gzip -v < boa/integrations/jupyter/jupyter.min.js | wc
 47.0%
      3      19     949

@charles-cooper
Copy link
Member

code review note: discussed refactoring the jupyter.js file into a full-on extension, but it seems to introduce more complexity than it is worth.

Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

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

lgtm after removing the dead variables

@charles-cooper charles-cooper merged commit ef3f909 into vyperlang:master Feb 16, 2024
5 of 8 checks passed
@charles-cooper
Copy link
Member

just tested this -- works amazing! and it's deployed to try.vyperlang.org now :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants