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.

Sign up for GitHub

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

Jump to bottom

XS GC timing changes after snapshot #3577

Closed
dckc opened this issue Aug 2, 2021 · 1 comment · Fixed by #3607
Closed

XS GC timing changes after snapshot #3577

dckc opened this issue Aug 2, 2021 · 1 comment · Fixed by #3607
Assignees
@dckc
Labels
bug Something isn't working moddable-P2 Moddable collaboration: future priority xsnap the XS execution tool
Milestone
Testnet: Metering...

Comments

@dckc
Copy link
Member

dckc commented Aug 2, 2021 •
edited
Loading

Description

Restoring from an XS snapshot results in a different heap organization, causing GC to happen at different times.

To Reproduce

As in the GC after snapshot vs restore test c4f0cec https://github.com/Agoric/agoric-sdk/blob/snap-gc-sync/packages/xsnap/test/test-xsnap.js#L340-L373 :

  1. Start an xsnap worker
  2. Allocate a large array that causes garbage collection and then let it go out of scope.
  3. Take a snapshot
  4. Start a clone from the snapshot. (Its heap is cleaned up.)
  5. Track GC in the worker and the clone; note that they happen at different times.

Expected behavior

No observable differences between the worker and its clone.

Additional context

Original diagnosis: #3428 (comment)

Moddable has a fix in agoric-labs/xsnap-pub@94a7a88 :

we reworked the chunk allocator to always use a single memory block for chunk storage.

But integrating it is non-trivial because

  1. We have not integrated the platform vs. application refactor agoric-labs/xsnap-pub@c2cebe7
  2. The xsnap-ava.c code with our I/O loop has fallen behind.

cc @phoddie
@dckc dckc added bug Something isn't working xsnap the XS execution tool moddable-P2 Moddable collaboration: future priority labels Aug 2, 2021
@dckc dckc added this to the Testnet: Metering Phase milestone Aug 2, 2021
@dckc dckc self-assigned this Aug 2, 2021
@dckc
Copy link
Member Author

dckc commented Aug 5, 2021

Integrating the work from Moddable (378b66f) is going pretty well.

I'm getting 2 test failures:

✖ xs-perf › meter details
✖ xs-limits › heap exhaustion: orderly fail-stop

The first is about mapSetAddCount, which we're not sure we want/need (#3139). I can deal with that one way or another.

The 2nd is a crash in mprotect, called from fxAllocateChunks at xsnapPlatform.c:212.
https://gist.github.com/dckc/b3bf3e28946b1e6b42a67a9bfcdab827
I expect to get help from Moddable on that one.

@dckc dckc mentioned this issue Aug 5, 2021
Merged
4 tasks
dckc added a commit that referenced this issue Aug 14, 2021
@dckc
fix(xsnap): take snapshot without impact on GC #3607
85b252c 
Moddable reworked the chunk allocator to always use a single memory block for chunk storage.

The change is implemented in `xsnapPlatform.c`. The implementation uses `mmap`/`mprotect` (`VirtualAlloc` on Windows) to reserve and to commit memory pages. 

To get this fix, we add an `xsnap-native` submodule that replaces `xsnap.c` and friends.
The submodule includes a few tweaks: https://github.com/agoric-labs/xsnap-pub/tree/endo-submodule

fixes #3577
fixes #3139 , bringing us back to stock XS.
fixes #2469 - Moddable reviewed the code well enough to substantially re-work it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dckc dckc

Labels
bug Something isn't working moddable-P2 Moddable collaboration: future priority xsnap the XS execution tool
Projects
None yet
Milestone
Testnet: Metering Phase
Development

Successfully merging a pull request may close this issue.

fix(xsnap): take snapshot without impact on GC Agoric/agoric-sdk
1 participant
@dckc