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

Investigate node-js performance and delays on exiting #9297

Closed
ekpyron opened this issue Jul 2, 2020 · 8 comments
Closed

Investigate node-js performance and delays on exiting #9297

ekpyron opened this issue Jul 2, 2020 · 8 comments
Labels

Comments

@ekpyron
Copy link
Member

ekpyron commented Jul 2, 2020

@frangio first reported that the loading times of soljson.js in node-js increased significantly between 0.6.8 and 0.6.9 (along with the size increase due to adding Z3). This indeed seems to be the case for node 10, which has quite high loading times in general.

We probably don't need to care too much about node 10, which is rather old by now - but node > 10 has an even weirder issue.

Some benchmarks by @frangio https://gist.github.com/frangio/e70e524e3357ea143df77b7d3197c0a7

Apparently neither loading the file, nor compiling it, nor executing it is what actually causes the slow execution. At least I personally would be fine with the increase from 200ms to 600ms there, because it's still "fast enough" - but apparently exiting the node-js process causes a significant delay and 2.5 vs 7.3 seconds is really not nice, especially if it looks like it could just be ~200ms and the rest is wasted process shutdown time.

@frangio
Copy link
Contributor

frangio commented Jul 2, 2020

An interesting point is that manually calling process.exit() will exit node without delay.

@ekpyron
Copy link
Member Author

ekpyron commented Jul 2, 2020

Oh, wow, interesting! I did use abort() and saw that that makes it exit fast, but expected that to only work in such a "disgraceful" way of exiting - I hadn't even considered that process.exit() might also have that effect :-)!
I'm starting to get the feeling that this might actually primarily be an issue in node itself and we should actually ask them to look into it or at least explain it...

@ekpyron
Copy link
Member Author

ekpyron commented Jul 3, 2020

Pinging @cameel here since he seems to be looking into this as well.

@ekpyron
Copy link
Member Author

ekpyron commented Jan 17, 2022

Looking at this again superficially, I think nodejs/node#36616 (comment) may be responsible for this. The issue is open and active, so we can hope that this will finally be resolved on the node end eventually...

@ekpyron
Copy link
Member Author

ekpyron commented Jan 17, 2022

Actually, these days I do see this issue even with explicit abort() or process.exit(0)...
But I just tried nodejs/node#36616 (comment) and that does seem to help, i.e.
running with node --wasm-dynamic-tiering solves the issue for me...

@ekpyron
Copy link
Member Author

ekpyron commented Dec 7, 2022

Given the most recent comment in the linked nodejs issue, it might be worthwhile to double check if this is fixed in the latest node versions.

@frangio
Copy link
Contributor

frangio commented Jan 20, 2023

Indeed this seems fixed on Node 18! Seeing this very clearly in the test suite for solidity-ast (npm test test/solc.js).

@ekpyron
Copy link
Member Author

ekpyron commented Feb 7, 2023

Closing this as fixed on the node side.

@ekpyron ekpyron closed this as completed Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants