-
Notifications
You must be signed in to change notification settings - Fork 3
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
CLI commands are slow to start #102
Comments
It's also possible that |
There are two parts to this one.
To do the simple AB testing we need to Obtain the following data points.
From that we can infer the time taken for From there we can get a more detailed point of view by attaching a debugger to the code and running profiling on it. The profiling with be node specific so we can't get a picture of what the |
Running the built linux executable using
Running the bundled code gives us 0.271s.
Just the built code with node gives us 0.301s.
As a sanity check, ls takes about 0.009s, and running node that just prints
So given these results
It seems most of the time is spent by So moving forward we need to see how we can optimise |
I did a quick and dirty package of a
Subtracting the |
There is a compression option. It defaults to None of the |
Is esbuild minified?
26 Mar 2024 18:56:33 Brian Botha ***@***.***>:
…
There is a compression option. It defaults to *none* and switching it to something else seems to increase the time.
—
Reply to this email directly, view it on GitHub[#102 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAE4OHJVOAIAWUJ2J3W2SWTY2EEY7AVCNFSM6AAAAABCOZNOHGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJZGQ3DGMRQGM].
You are receiving this because you are subscribed to this thread.
[Tracking image][https://github.com/notifications/beacon/AAE4OHKH2ESI4E54L3CW5Q3Y2EEY7A5CNFSM6AAAAABCOZNOHGWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTYL2ICG.gif]
|
base case real 0m0.923s removed real 0m0.949s removed real 0m0.980s No real advantages. And yes, |
Doing manual testing on the office mac, I removed all the extra native modules for platforms we don't need. This brought execution down from
So that's a good chunk right there. While not a full solution, reducing the size of the final binary is definitely something we want to do. So I'll explore this a bit more. |
Here are some debug logs from testing on the mac. The We can see that there is 28.2MB of data being included. A good chunk of this, 19.8MB is just the quic and db universal binaries. But ignoring the size of the files for a sec, the number of files can have an impact too. So we should look into reducing all kinds of bloat here. Notably the Moving forward I'm going to prune more of the unneeded files that are being included but also look into building the db and quic binaries with more optimisation if possible. |
Marking all of the We really need to be more selective about this. Maybe I can modify it to mark only |
Yea this is a lower priority for now. We can address this later. Focus on finishing the other bugs and I'll return to this. It might require some architectural changes.
27 Mar 2024 14:16:17 Brian Botha ***@***.***>:
…
Marking all of the *optionalDependencies* as *--external* in *esbuild* is resulting in a bunch of dependencies we don't want being pulled into the *vfs*. This seems problimatic since it's including stuff like *async-init* or *timer* in the *vfs* but also bundled inside of the *polykey.js* code. I'm pretty sure maintaining 2 versions of a dependency like this should be causing issues with anything that uses *instanceOf*. I'm really not sure why this hasn't broken *timer*.
We really need to be more selective about this. Maybe I can modify it to mark only *.node* files as external.
—
Reply to this email directly, view it on GitHub[#102 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAE4OHONPAVEJNOZ5WPFONLY2IMV5AVCNFSM6AAAAABCOZNOHGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRRG42DOMBYGU].
You are receiving this because you commented.
[Tracking image][https://github.com/notifications/beacon/AAE4OHLUR2BXK4A4E4QOCWDY2IMV5A5CNFSM6AAAAABCOZNOHGWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTYQFUY2.gif]
|
OK, I'll put it on hold for now. With some testing I got the mac execution down to 0.48s. There is definitely a lot we can do to reduce bloat in the final executable. The I think we need to consider updating The sodium native source code is being included in the packaged vfs as well. So we'll need to exclude that. That alone is adding about 1.8MB to the output. |
#84 is the high level issue surrounding this problem. Recommend solving that at higher level. |
A few ideas:
Tagging @brynblack and @aryanjassal too. |
We need to understand the V8 system internals a bit better (node --v8-options). And even optimise the memory utilisation too. Having the HTTP status page MatrixAI/Polykey#412 and improving the MatrixAI/Polykey#635 would be critical to enabling us to do this work empirically instead of just trying random stuff. It needs to be more methodical from this point onwards. |
Specification
There seems to be a lead time before running Polykey does anything. this is demonstrated by running
polykey --help
to get the help text. This alone takes about 0.5 seconds on my work laptop.Compare this to
polykey bootstrap
command which does some password hashing and state creation.So a good 500ms is spent doing something before anything really runs. We can start by checking that we're importing all of polykey before running any command. After that the likely culprit is commander.
Additional context
Related: #40
Tasks
Polykey
during the import stage of the code. Large parts ofPolykey
should only be imported when the command is run, no when it is defined.Polykey-CLI
.The text was updated successfully, but these errors were encountered: