Skip to content

fix(restore): replace scratch zero with fresh allocation on Windows - #1765

Merged
danbugs merged 1 commit into
hyperlight-dev:mainfrom
danbugs:danbugs/fix-restore-rss-v0.16
Aug 26, 2026
Merged

fix(restore): replace scratch zero with fresh allocation on Windows#1765
danbugs merged 1 commit into
hyperlight-dev:mainfrom
danbugs:danbugs/fix-restore-rss-v0.16

Conversation

@danbugs

@danbugs danbugs commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

When restoring a snapshot, restore_snapshot() can fall into two paths:

  1. Scratch size unchanged → call zero() on the existing region
  2. Scratch size changed → allocate a fresh new scratch region

Unless you exclude default features and use KVM only, zero() falls to a codepath that memsets the entire scratch region with zeroes. With big scratch regions like is often the case with Hyperlight+Unikraft guests (e.g., 448 MiB for Node.js), this causes a massive RSS spike and significantly increases restore time.

Peak RSS Restore time
Before (Windows) ~453 MiB ~138 ms
After (Windows) ~12 MiB ~11 ms

With this change, the default path on Windows uses the same fresh-allocation strategy as when the scratch sizes differ — the OS provides demand-zero pages (page-file-backed), so physical memory is consumed only as the guest touches pages.

This is gated behind Windows only. The MSHV path on Linux is left unchanged for now to avoid accidentally regressing small MSHV workloads, and may be updated in the future after more thorough benchmarking.

Copilot AI lite review requested due to automatic review settings August 25, 2026 18:03
@danbugs danbugs added area/performance Addresses performance kind/refactor For PRs that restructure or remove code without adding new functionality. ready-for-review PR is ready for (re-)review labels Aug 25, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes snapshot restore behavior in hyperlight-host by avoiding full zeroing of large scratch regions on Windows, instead relying on fresh demand-zero allocations to reduce RSS spikes and restore latency.

Changes:

  • On Windows, restore_snapshot() allocates a new scratch region even when the scratch size is unchanged, avoiding a full-region fill(0)/memset.
  • Adds an in-code rationale explaining the Windows-specific performance motivation and why Linux behavior is unchanged.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/hyperlight_host/src/mem/mgr.rs Outdated
@danbugs
danbugs force-pushed the danbugs/fix-restore-rss-v0.16 branch 3 times, most recently from 0a18a36 to 4565fd5 Compare August 25, 2026 18:29
jsturtevant
jsturtevant previously approved these changes Aug 25, 2026
syntactically
syntactically previously approved these changes Aug 25, 2026

@syntactically syntactically left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like the functionally correct change to unblock the Windows unikraft performance issues. I do have a couple of minor nits below, especially around communicating why we are using (and that we should replace) this fairly odd heuristic.

Comment thread src/hyperlight_host/src/mem/mgr.rs Outdated
Comment thread src/hyperlight_host/src/mem/mgr.rs Outdated
@github-actions github-actions Bot removed the ready-for-review PR is ready for (re-)review label Aug 25, 2026
@danbugs
danbugs dismissed stale reviews from syntactically and jsturtevant via ded51f4 August 25, 2026 20:04
@danbugs
danbugs force-pushed the danbugs/fix-restore-rss-v0.16 branch from 4565fd5 to ded51f4 Compare August 25, 2026 20:04
Comment thread src/hyperlight_host/src/mem/shared_mem.rs Outdated
Comment thread src/hyperlight_host/src/mem/shared_mem.rs Outdated
Comment thread src/hyperlight_host/src/mem/shared_mem.rs Outdated
Comment thread src/hyperlight_host/src/mem/shared_mem.rs
syntactically
syntactically previously approved these changes Aug 25, 2026
@danbugs
danbugs force-pushed the danbugs/fix-restore-rss-v0.16 branch from 89c24b2 to 7c879b4 Compare August 25, 2026 23:21
@danbugs
danbugs enabled auto-merge (squash) August 26, 2026 00:26
restore_snapshot() zeroed scratch memory via scratch_mem.zero() when
the size was unchanged. On Linux, KVM-only builds use MADV_DONTNEED
(lazy zero) and mshv3 builds fall through to fill(0). On Windows
there was no lazy path at all — zeroing memsets the entire region.

For a Node.js guest with 448 MiB scratch, this caused:
- RSS spike from ~9 MiB to ~453 MiB on each restore
- Rewind time of ~138 ms (Windows)

Gate the fresh-allocation path behind Windows only. On Windows,
ExclusiveSharedMemory::new provides demand-zero pages (page-file-
backed), so physical memory is consumed only as the guest touches
pages. On Linux, keep the existing zero() path (KVM: MADV_DONTNEED,
mshv3: fill(0)).

Signed-off-by: danbugs <danilochiarlone@gmail.com>
@danbugs
danbugs force-pushed the danbugs/fix-restore-rss-v0.16 branch from 7c879b4 to 2a76cbc Compare August 26, 2026 01:04
@danbugs
danbugs merged commit 5e4b97b into hyperlight-dev:main Aug 26, 2026
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/performance Addresses performance kind/refactor For PRs that restructure or remove code without adding new functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants