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

ui: load bundle split vendor and code #7135

Merged
merged 3 commits into from
Dec 10, 2023
Merged

ui: load bundle split vendor and code #7135

merged 3 commits into from
Dec 10, 2023

Conversation

nopcoder
Copy link
Contributor

@nopcoder nopcoder commented Dec 9, 2023

  • Update @duckdb/duckdb-wasm to v1.28.0
  • Vite build using chunks - split vendor code

Close #7134

@nopcoder nopcoder added area/UI Improvements or additions to UI minor-change Used for PRs that don't require issue attached labels Dec 9, 2023
@nopcoder nopcoder requested a review from a team December 9, 2023 19:06
@nopcoder nopcoder self-assigned this Dec 9, 2023
@nopcoder nopcoder added exclude-changelog PR description should not be included in next release changelog include-changelog PR description should be included in next release changelog and removed exclude-changelog PR description should not be included in next release changelog labels Dec 9, 2023
Copy link
Contributor

@arielshaqed arielshaqed left a comment

Choose a reason for hiding this comment

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

This looks generally cool. I have some questions though about the issue, based on this:

❯ du -sh dist/assets/*
268K    dist/assets/duckdb-browser-eh.worker-80bdeff3.js
320K    dist/assets/duckdb-browser-mvp.worker-91331f4b.js
17M     dist/assets/duckdb-eh-13ec0755.wasm
21M     dist/assets/duckdb-mvp-a4aa3c03.wasm
324K    dist/assets/index-0308a6b6.css
6.5M    dist/assets/index-90011776.js
  • Optimize cache usage. Code changes is much more common than package changes.

Not sure that this is true, but in any case neither is that common. We're saving, what, one <6MiB download per update? When DuckDB is 17MiB + 21MiB?

  • Parallel downloading. Download the vendor code and other chunks simultaneously.

Again, there is little to gain here given DuckDB.

We should measure and see how this changes things. So approving because this doesn't harm us, but not sure it how it helps our users.

@nopcoder
Copy link
Contributor Author

DuckDB wasm is dynamically loaded. Downloaded only when you open the duckdb shell.

Parallel load - this is how it looks like after the change:

image

If we will switch to dynamic load for the file viewers - render notebook for example we can split the vendor package and load only partial code.

The only benefit is the cache reuse - assume you change your code and not react.

@nopcoder nopcoder merged commit fbf3e2f into master Dec 10, 2023
42 of 45 checks passed
@nopcoder nopcoder deleted the chore/ui-load-chunks branch December 10, 2023 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/UI Improvements or additions to UI include-changelog PR description should be included in next release changelog minor-change Used for PRs that don't require issue attached
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI: Splitting the vendor code into a separate chunk in Vite
2 participants