-
Notifications
You must be signed in to change notification settings - Fork 33
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
#96 Improve takeSnapshot
#97
base: main
Are you sure you want to change the base?
Conversation
@@ -5,6 +5,17 @@ All notable changes to this project will be documented in this file. | |||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), | |||
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | |||
|
|||
## [2.5.0] - 2024-09-17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can remove the changelog, we're using github releases for release notes ;)
setTimeout(() => this.takeSnapshot(), 500); | ||
return new Promise((resolve) => { | ||
// There might not be available worker, let it start. | ||
setTimeout(() => resolve(this.takeSnapshot()), 500); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we pass the same options to the second call?
setTimeout(() => resolve(this.takeSnapshot()), 500); | |
setTimeout(() => resolve(this.takeSnapshot(options)), 500); |
if (callback) { callback('heap snapshot done'); } | ||
}); | ||
const getHeapSnapshot = (callback) => { | ||
if (hasEnoughMemory()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about an early return if not enough memory to avoid having the main function body enclosed in an if statement?
This PR improves takeSnapshot method