Skip to content

Conversation

@jstarks
Copy link
Member

@jstarks jstarks commented Oct 29, 2025

openhcl_vmm has a lot of code that depends on memcpy being fast, but musl's memcpy on x86_64 is often slow. Write a generic memcpy in Rust and rely on LLVM to do a good job optimizing it.

openhcl_vmm has a lot of code that depends on memcpy being fast, but
musl's memcpy on x86_64 is slow. Write a generic memcpy in Rust and
rely on LLVM to do a good job optimizing it.
Copilot AI review requested due to automatic review settings October 29, 2025 05:58
@jstarks jstarks requested review from a team as code owners October 29, 2025 05:58
Copy link
Contributor

Copilot AI left a comment

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 introduces an optimized memcpy and memmove implementation to address performance issues with musl's implementation on x86_64. The implementation handles various copy sizes with specialized strategies and correctly handles overlapping memory regions.

  • Adds a new fast_memcpy crate with optimized memory copy functions
  • Integrates the fast_memcpy into OpenHCL's underhill_entry for x86_64 targets
  • Configures dev profile optimizations to ensure fast_memcpy is always optimized

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
support/fast_memcpy/src/lib.rs Core implementation of optimized memcpy/memmove with size-specific strategies and overlap handling
support/fast_memcpy/Cargo.toml Package manifest for the new fast_memcpy crate
openhcl/underhill_entry/src/lib.rs Imports fast_memcpy for x86_64 to replace musl's memcpy
openhcl/underhill_entry/Cargo.toml Adds fast_memcpy dependency
Cargo.toml Registers fast_memcpy workspace member and configures dev profile optimizations
Cargo.lock Lock file updates for new dependency

@github-actions
Copy link

@github-actions github-actions bot added the unsafe Related to unsafe code label Oct 29, 2025
@github-actions
Copy link

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

@smalis-msft smalis-msft added the release-ci-required Add to a PR to trigger PR gates in release mode label Oct 30, 2025
@jstarks jstarks changed the title do-not-merge: fast_memcpy: add memcpy implementation for openhcl_vmm fast_memcpy: add memcpy implementation for openhcl_vmm Oct 31, 2025
0 => {}
1 => copy_one::<u8>(dest, src),
2 => copy_one::<u16>(dest.cast(), src.cast()),
3 => copy_one::<U8x3>(dest.cast(), src.cast()),
Copy link
Contributor

Choose a reason for hiding this comment

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

How frequently does 3 come up?

Copy link
Member Author

Choose a reason for hiding this comment

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

I dunno.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth having this extra type and wiring? It seems like an odd len to come in. How often is memcpy called with such small sizes in general? I'd expect small sizes to just be optimized out.

@github-actions
Copy link

@github-actions
Copy link

@github-actions
Copy link

@jstarks jstarks merged commit cb0d7ce into microsoft:main Nov 3, 2025
101 of 105 checks passed
@jstarks jstarks deleted the memcpy branch November 3, 2025 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-ci-required Add to a PR to trigger PR gates in release mode unsafe Related to unsafe code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants