Skip to content

Performance improvements: lazy loading, optimized/minimized js bundles etc#350

Closed
richarddd wants to merge 7 commits intosaghul:masterfrom
richarddd:lazy-loading
Closed

Performance improvements: lazy loading, optimized/minimized js bundles etc#350
richarddd wants to merge 7 commits intosaghul:masterfrom
richarddd:lazy-loading

Conversation

@richarddd
Copy link
Copy Markdown

@richarddd richarddd commented Jan 8, 2023

This is a huge PR and basically touches every js file in this project but the startup performance gains are at least 2x.

TODO

  • Fix tests/test-exec.js test, all other tests are passing.

Lazy loading for all non-essential modules

Almost every module (except native ones) are lazy loaded only when needed. This means that startup performance is at least 2x (often even 3x) faster. For example:

fetch(....) only loads the fetch polyfill and related code when used.

Bundle optimizations

console

  • Lazy loads table and trace
  • Uses internal qjs text encoder for performance
  • Uses JSON.stringify with replacer for formatting rather than util.format (lots of code in that module)

Blob

  • Removed browser-specific code (document etc)
  • Optimized for ES2020
  • ES6 compliant (uses class inheritence etc)

path

  • Path is bundled individually for posix and win32. No need to have path that support both.

Future work
Could be moved to C

URL

  • Switch to whatwg-url implementation.
  • Moved out URLPattern to separate module

Future work
Could be optimized using https://uriparser.github.io/

cypto

  • Uses native uuidv4 implementation

Other

Native text encoder/decoder

  • Exposes core.textEncode core.textDecode from sys.c

Improve script runner

  • Added support for running a js/cjs/mjs file directly without entering the main js function. This means that executing a single script is faster since there is no need to load and parse opts. ./tjs index.js vs /tjs run index.js. This "runner" only executes if the first command ends with .js or .cjs or .mjs

Linting cleanup

Ran prettier with the correct lint rues throughout the entire project for consistency.

Future work

  • Consider using prettier code formatting rather than custom ones (double quote strings, indentation etc)
  • Consider using internal text decoding/encoding from qjs rather than TextEncoding in js.

@saghul
Copy link
Copy Markdown
Owner

saghul commented Jan 8, 2023

Thank you!

There is a lot to unpack here :-)

I'll take a look this week!

I'd like to see this land in multiple PRs. That is, many of the improvements proposed here are not related to the lazy loading and we can land those first before making The Big Change.

@richarddd
Copy link
Copy Markdown
Author

Yes I was going to do that originally but then I had to change a couple of things in order to get lazy loading working such as changing some polyfills to ponyfills because they are trying to define properties that I’m currently setting. For example the URL polyfill can’t access globalThis.URL since that’s the property I’m loading and setting (will cause nasty errors and seg faults).
And since this PR is a breaking change of the APIs it might be better to do a big bang.
Another idea is to create a pre-release branch until we have found a way forward we’re happy with (regarding the naming and APIs etc)

@saghul
Copy link
Copy Markdown
Owner

saghul commented Jan 10, 2023

Phew, that was a long read!

Before I begin, thanks a ton for doing this! Let's unpack.

There is no way I am merging such a large PR, we need to break this down into individual self-contained changes that slowly work towards the end goal, and once all changes are in, it would be virtually equivalent to this PR. I have refrained from making comments while reading the code, because it's just too much to review at once.

So, here how I think we can begin to break it down:

  • console improvements
  • path improvements
  • Blob improvements
  • URL improvements
  • Native uuidv4 (note the current proposed implementation seems to use a static seed, which sounds bad)
  • Native text encode / decode
  • Optimized script runner: this I'd like us to keep the same syntax as the current CLI, but just avoid the JS roundtrip in there are no more arguments
  • Break apart the stdlib into individual modules like tjs:assert, tjs:path, etc. (we should probably move ffi here, to tjs:ffi

Things NOT to do (yet):

  • Run prettier or any not currently existing linter: it just adds churn at this point. Let's focus on the changes themselves, then maybe consider linting the whole project in one single pass.

All these things can be done under the current internal "architecture". They won't bring that big speedup from the lazy loading, but they'll get us ready-er for it.

Now on to the big one.

LAZY LOAD ALL THE THINGS!

On this step we'd move to lazy loading all the big things. I'd like us to do this in a couple of steps:

  • Introduce the mechanism and lazy load a couple of the biggest offenders
  • Make sure the "internal" modules are not loadable by user code
  • Gradually migrate the rest of the modules

An important part for me on this last step, due to the complexity it adds will be to define a way by which we'll measure. Also, how will we know we haven't regressed in startup speed? Perhaps there needs to be a test that checks for this with some baseline, so make sure it stays that way.


Hope that makes sense! Happy to work together towards this goal. It'll take a bit of time, but we'll get there.

@richarddd
Copy link
Copy Markdown
Author

Yes that sounds like a plan! I'll get to it!

One module we also would migrate is the TextEncoder which is terribly slow. And again, the more modules we can move to native the better. I will close this one and work according to the plan :)

Thanks for your review!

@richarddd richarddd closed this Jan 11, 2023
@saghul
Copy link
Copy Markdown
Owner

saghul commented Jan 12, 2023

Awesome! I think the migration to having the stdlib in separate modules (again, lol) and having them lazy-loaded can be done in parallel.

Let me know where you plan to start and I can tackle some of the items too.

@richarddd
Copy link
Copy Markdown
Author

Awesome! I think the migration to having the stdlib in separate modules (again, lol) and having them lazy-loaded can be done in parallel.

Let me know where you plan to start and I can tackle some of the items too.

I’m currently very limited time wise and will not likely have the bandwidth to do this ☹️ Feel free to take what you need from my mega-pr and do this step by step

@saghul
Copy link
Copy Markdown
Owner

saghul commented Jan 18, 2023

👍 sounds good!

saghul added a commit that referenced this pull request Feb 14, 2023
saghul added a commit that referenced this pull request Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants