-
Notifications
You must be signed in to change notification settings - Fork 57
CVS-174585: Memory map shared weights when possible #829
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
CVS-174585: Memory map shared weights when possible #829
Conversation
There was a problem hiding this 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 memory mapping functionality for shared weights in the OpenVINO execution provider to improve performance by avoiding unnecessary memory copies. The changes enable direct memory mapping of external weight files to device tensors when possible.
- Added version check macro for OpenVINO to conditionally enable features based on version
- Implemented memory mapping support for weight files with device-specific optimizations
- Refactored tensor creation to use memory-mapped weights when available, falling back to traditional copying
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| onnxruntime/core/providers/openvino/ov_interface.h | Added version check macro for OpenVINO compatibility |
| onnxruntime/core/providers/openvino/contexts.h | Extended WeightsFile class with device mapping functionality |
| onnxruntime/core/providers/openvino/backend_utils.cc | Implemented memory mapping logic and refactored tensor creation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // CPU/virtual device case, create a CPU tensor memory mapped from file | ||
| auto&& mmaped_tensor = ov::read_tensor_data(file_path_); | ||
| it->second = MappingContainer{.ptr_ = mmaped_tensor.data(), .tensor_ = mmaped_tensor}; |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'mmaped' to 'mmapped' in variable name and comment.
| // CPU/virtual device case, create a CPU tensor memory mapped from file | |
| auto&& mmaped_tensor = ov::read_tensor_data(file_path_); | |
| it->second = MappingContainer{.ptr_ = mmaped_tensor.data(), .tensor_ = mmaped_tensor}; | |
| // CPU/virtual device case, create a CPU tensor memory mmapped from file | |
| auto&& mmapped_tensor = ov::read_tensor_data(file_path_); | |
| it->second = MappingContainer{.ptr_ = mmapped_tensor.data(), .tensor_ = mmapped_tensor}; |
| uint8_t* mmaped_weights = static_cast<uint8_t*>(weights.TryGetOrCreateDeviceMapping(opt_remote_ctx)); | ||
|
|
||
| if (mmaped_weights) { | ||
| // We have memory mapped weights. Create a Tensor view into it for this value. | ||
| ORT_ENFORCE(value.data_offset < weights.Size() && | ||
| value.size <= weights.Size() && | ||
| (value.data_offset <= weights.Size() - value.size), | ||
| "File offset + size outside of external initializer file"); | ||
| void* mmapped_offset = static_cast<void*>(mmaped_weights + value.data_offset); |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'mmaped' to 'mmapped' in variable name.
| uint8_t* mmaped_weights = static_cast<uint8_t*>(weights.TryGetOrCreateDeviceMapping(opt_remote_ctx)); | |
| if (mmaped_weights) { | |
| // We have memory mapped weights. Create a Tensor view into it for this value. | |
| ORT_ENFORCE(value.data_offset < weights.Size() && | |
| value.size <= weights.Size() && | |
| (value.data_offset <= weights.Size() - value.size), | |
| "File offset + size outside of external initializer file"); | |
| void* mmapped_offset = static_cast<void*>(mmaped_weights + value.data_offset); | |
| uint8_t* mmapped_weights = static_cast<uint8_t*>(weights.TryGetOrCreateDeviceMapping(opt_remote_ctx)); | |
| if (mmapped_weights) { | |
| // We have memory mapped weights. Create a Tensor view into it for this value. | |
| ORT_ENFORCE(value.data_offset < weights.Size() && | |
| value.size <= weights.Size() && | |
| (value.data_offset <= weights.Size() - value.size), | |
| "File offset + size outside of external initializer file"); | |
| void* mmapped_offset = static_cast<void*>(mmapped_weights + value.data_offset); |
| uint8_t* mmaped_weights = static_cast<uint8_t*>(weights.TryGetOrCreateDeviceMapping(opt_remote_ctx)); | ||
|
|
||
| if (mmaped_weights) { | ||
| // We have memory mapped weights. Create a Tensor view into it for this value. | ||
| ORT_ENFORCE(value.data_offset < weights.Size() && | ||
| value.size <= weights.Size() && | ||
| (value.data_offset <= weights.Size() - value.size), | ||
| "File offset + size outside of external initializer file"); | ||
| void* mmapped_offset = static_cast<void*>(mmaped_weights + value.data_offset); |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'mmaped' to 'mmapped' in variable name.
| uint8_t* mmaped_weights = static_cast<uint8_t*>(weights.TryGetOrCreateDeviceMapping(opt_remote_ctx)); | |
| if (mmaped_weights) { | |
| // We have memory mapped weights. Create a Tensor view into it for this value. | |
| ORT_ENFORCE(value.data_offset < weights.Size() && | |
| value.size <= weights.Size() && | |
| (value.data_offset <= weights.Size() - value.size), | |
| "File offset + size outside of external initializer file"); | |
| void* mmapped_offset = static_cast<void*>(mmaped_weights + value.data_offset); | |
| uint8_t* mmapped_weights = static_cast<uint8_t*>(weights.TryGetOrCreateDeviceMapping(opt_remote_ctx)); | |
| if (mmapped_weights) { | |
| // We have memory mapped weights. Create a Tensor view into it for this value. | |
| ORT_ENFORCE(value.data_offset < weights.Size() && | |
| value.size <= weights.Size() && | |
| (value.data_offset <= weights.Size() - value.size), | |
| "File offset + size outside of external initializer file"); | |
| void* mmapped_offset = static_cast<void*>(mmapped_weights + value.data_offset); |
| uint8_t* mmaped_weights = static_cast<uint8_t*>(weights.TryGetOrCreateDeviceMapping(opt_remote_ctx)); | ||
|
|
||
| if (mmaped_weights) { | ||
| // We have memory mapped weights. Create a Tensor view into it for this value. | ||
| ORT_ENFORCE(value.data_offset < weights.Size() && | ||
| value.size <= weights.Size() && | ||
| (value.data_offset <= weights.Size() - value.size), | ||
| "File offset + size outside of external initializer file"); | ||
| void* mmapped_offset = static_cast<void*>(mmaped_weights + value.data_offset); |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'mmaped' to 'mmapped' in variable name (note: mmapped_offset is already correct).
| uint8_t* mmaped_weights = static_cast<uint8_t*>(weights.TryGetOrCreateDeviceMapping(opt_remote_ctx)); | |
| if (mmaped_weights) { | |
| // We have memory mapped weights. Create a Tensor view into it for this value. | |
| ORT_ENFORCE(value.data_offset < weights.Size() && | |
| value.size <= weights.Size() && | |
| (value.data_offset <= weights.Size() - value.size), | |
| "File offset + size outside of external initializer file"); | |
| void* mmapped_offset = static_cast<void*>(mmaped_weights + value.data_offset); | |
| uint8_t* mmapped_weights = static_cast<uint8_t*>(weights.TryGetOrCreateDeviceMapping(opt_remote_ctx)); | |
| if (mmapped_weights) { | |
| // We have memory mapped weights. Create a Tensor view into it for this value. | |
| ORT_ENFORCE(value.data_offset < weights.Size() && | |
| value.size <= weights.Size() && | |
| (value.data_offset <= weights.Size() - value.size), | |
| "File offset + size outside of external initializer file"); | |
| void* mmapped_offset = static_cast<void*>(mmapped_weights + value.data_offset); |
https://jira.devtools.intel.com/browse/CVS-174585