Skip to content

Modify UFFD tests to run serve loop in a separate process - #1450

Merged
ValentaTomas merged 26 commits into
mainfrom
cross-process-uffd-tests
Nov 7, 2025
Merged

ValentaTomas merged 26 commits into
mainfrom
cross-process-uffd-tests

Conversation

@ValentaTomas

@ValentaTomas ValentaTomas commented Nov 6, 2025 •

Copy link
Copy Markdown
Member

This more accurately simulates the FC-orchestrator configuration and prevents the Go mmap bugs.


Note

Run the UFFD serve loop in a helper process with pipe/signal IPC, refactor tests to use cross-process flow, and update test utilities to new APIs.

  • Tests (UFFD):
    • Introduce cross-process test harness configureCrossProcessTest and helper TestHelperServingProcess to serve page faults in a separate process using pipes (content, offsets, ready) and signals (SIGUSR1/SIGUSR2).
    • Replace in-process server setup with cross-process flow across missing_test.go and missing_write_test.go; add t.Parallel() and adjust operation counts; change some hugepage suites to numberOfPages: 8.
    • Update assertions to fetch faulted page offsets via offsetsOnce().
  • New helper:
    • cross_process_helper_test.go: implements cross-process serving (crossProcessServe), offset collection (getAccessedOffsets), and IPC setup; uses fdexit, mapping, and zap logger.
  • Refactors (helpers_test.go):
    • Simplify testHandler: remove in-process missingRequests/mapping; add offsetsOnce and a write mutex; drop touchRead.
  • Test utils:
    • Export NewMemorySlicer and add Size()/Content() methods; update RandomPages to use it.
    • Change NewPageMmap API to accept *testing.T and manage cleanup internally; remove explicit unmap return value.

Written by Cursor Bugbot for commit d17d851. This will update automatically on new commits. Configure here.

Comment thread packages/orchestrator/internal/sandbox/uffd/cross_process_helper_test.go Outdated
Comment thread packages/orchestrator/internal/sandbox/uffd/cross_process_helper_test.go Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/orchestrator/internal/sandbox/uffd/cross_process_helper_test.go Outdated
Comment thread packages/orchestrator/internal/sandbox/uffd/cross_process_helper_test.go Outdated
@ValentaTomas
ValentaTomas marked this pull request as draft November 6, 2025 23:50
@ValentaTomas
ValentaTomas marked this pull request as ready for review November 7, 2025 01:09
Comment thread packages/orchestrator/internal/sandbox/uffd/cross_process_helper_test.go Outdated
Comment thread packages/orchestrator/internal/sandbox/uffd/serve.go Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/orchestrator/internal/sandbox/uffd/serve.go
Comment thread packages/orchestrator/internal/sandbox/uffd/cross_process_helper_test.go Outdated
@ValentaTomas
ValentaTomas enabled auto-merge (squash) November 7, 2025 02:12
Comment thread packages/orchestrator/internal/sandbox/uffd/cross_process_helper_test.go Outdated
@ValentaTomas
ValentaTomas requested a review from djeebus November 7, 2025 02:59
Comment thread packages/orchestrator/internal/sandbox/uffd/cross_process_helper_test.go Outdated
@ValentaTomas
ValentaTomas requested a review from djeebus November 7, 2025 06:23
@ValentaTomas
ValentaTomas merged commit 6ee2ebb into main Nov 7, 2025
28 checks passed
@ValentaTomas
ValentaTomas deleted the cross-process-uffd-tests branch November 7, 2025 16:13
ValentaTomas added a commit that referenced this pull request May 2, 2026
…MissingWriteWithPrefault loads to 1_000_000

Continues the regression sweep started in 4681ab2. The original
PR #1415 (88c3960, Nov 3 2025) introduced these tests at 1_000_000
operations. PR #1450 (6ee2ebb, Nov 7 2025, "Modify UFFD tests to
run serve loop in a separate process") cut them all to 10_000 in the
same commit that switched from in-process to cross-process tests, with
no commit message justification. The cross-process refactor itself
does not require lower iteration counts: the per-iteration work is
still in-process memory access; only the page-state snapshot RPC is
cross-process and is called once per test.

Reverts:
  - TestSerialMissing:                10_000 -> 1_000_000
  - TestSerialMissingWrite:           10_000 -> 1_000_000
  - TestParallelMissingWriteWithPrefault: 10_000 -> 1_000_000

Per-test wall time on this branch (sudo go test, no -race):
  - TestSerialMissing                  ~3s
  - TestSerialMissingWrite             ~3s
  - TestParallelMissingWriteWithPrefault ~2s

TestParallelMissingWithPrefault stays at 10_000: it was only ever
parallelOperations := 10 in #1415, then bumped to 10_000 in #1450,
so 10_000 is already above its historical baseline.
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.

2 participants