-
Notifications
You must be signed in to change notification settings - Fork 4.4k
[feat](benchmark): replace load_param with load_param_mem #6286
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
Conversation
|
The binary size change of libncnn.so (bytes)
|
|
run cd build
cmake -DCMAKE_BUILD_TYPE=Debug -DNCNN_VULKAN=ON -DNCNN_BUILD_EXAMPLES=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON ..
make -j$(nproc)
cd benchmark
./benchncnn 4 8 0 -1 1result before after |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6286 +/- ##
========================================
Coverage 93.28% 93.29%
========================================
Files 845 845
Lines 265939 266146 +207
========================================
+ Hits 248086 248305 +219
+ Misses 17853 17841 -12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 replaces file-based model loading with memory-based loading in the benchncnn tool by embedding model parameter files directly into the executable as C arrays. This eliminates the dependency on external .param files and allows the benchmark to run from any directory.
- Introduces a CMake macro to convert .param files into C header files containing hex data arrays
- Updates benchncnn.cpp to use
load_param_mem()instead ofload_param()with embedded data - Normalizes model names by replacing hyphens with underscores to ensure valid C identifiers
Reviewed Changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmake/ncnn_generate_model_data_header.cmake | Adds CMake macro to convert .param files to C arrays with validation |
| benchmark/model_param_spv_data.h.in | Template for including generated model parameter headers |
| benchmark/model_param_registry.h.in | Template for model parameter registry data |
| benchmark/benchncnn.cpp | Updates to use embedded model data with registry lookup function |
| benchmark/README.md | Updates documentation to reflect new param usage without .param suffix |
| benchmark/CMakeLists.txt | Integrates model data generation into build process |
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.
💡 Add Copilot custom instructions for smarter, more guided reviews. 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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
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.
💡 Add Copilot custom instructions for smarter, more guided reviews. 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 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
This reverts commit 77e57d7.
|
System: macos run |
|
|
|
Thanks for your contribution ! |
Previous implementation required
benchncnnto be run from the directory containing the model files. This often caused failures and inconvenience when executed from an arbitrary working directory.This change embeds the model files directly into the executable by compiling them as C arrays. Consequently,
benchncnnnow becomes fully self-contained and can be run from any location without dependency on external model files.