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

Snapshot data memory managment #417

Closed
wants to merge 1 commit into from
Closed

Conversation

@jasssonpet jasssonpet self-assigned this Apr 12, 2016
@jasssonpet jasssonpet added this to the 2.0.0 milestone Apr 12, 2016
@atanasovg
Copy link
Contributor

Looks good but this also means that we are keeping the whole blob in memory. I was thinking whether we should enable the snapshot in release builds only. What do you think?

@jasssonpet
Copy link
Contributor Author

jasssonpet commented Apr 12, 2016

I think that enabling the snapshot in release mode only would be confusing for our users. My concerns are that they could would to edit the source code of the already snapshotted modules and that would make for an unlikely surprise when it comes time for releasing.

Also, enabling the snapshot in debug builds, would make apps start faster in debug mode and would make application development a bit faster and more pleasant. Users will be more likely to accept NativeScript if apps start faster in debug mode.

Another concern is that we could skip packaging the snapshotted modules in the npm plugin and that would make it's size smaller.

Having the same code path for both debug and release builds would also make testing easier and would catch more cases where something works in debug, but not in release mode.

My biggest concern, though, is that the runtime should be able to use snapshotted modules and debug just fine. The decision, whether to use the snapshot only in release should be decided in the CLI or the plugin, that provides the snapshot.

@atanasovg If memory is of an issue, I would suggest in researching memory-mapped files. Or maybe we could keep the snapshot data during the whole application lifecycle in debug config, but delete it once the V8 heap is initialized in release config. What do you think about that?

P.S. I've tested the embedded snapshot script with Node Inspector and it seems to work just fine, but the VS Code extension is not working properly and should be updated.

@Plamen5kov Plamen5kov modified the milestone: 2.0.0 Apr 18, 2016
This stops the debugger from crashing.
@jasssonpet jasssonpet force-pushed the jasssonpet/snapshot-data branch from f3cc7ed to 3da24d8 Compare April 18, 2016 12:55
@ns-bot
Copy link

ns-bot commented Apr 18, 2016

💚

@jasssonpet
Copy link
Contributor Author

Closing this in favor of #425.

@jasssonpet jasssonpet closed this Apr 19, 2016
@jasssonpet jasssonpet deleted the jasssonpet/snapshot-data branch April 19, 2016 13:12
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.

4 participants