Add track cost time #153
Add track cost time #153
Conversation
|
Automatic code review is enabled for this repository. Reviewing this PR now. |
![devloai[bot]](https://avatars.githubusercontent.com/in/886113?s=60&v=4){kind=small}
There was a problem hiding this comment.
I've reviewed the performance tracking additions. The implementation is well-structured but needs some robustness improvements (null checks) and better documentation. The MNN precision change requires explanation as it could affect accuracy. Please also consider the naming and configurability suggestions to improve maintainability.
| // cost spend | ||
| std::shared_ptr<inspirecv::TimeSpend> m_face_track_cost_; | ||
|
|
||
| bool m_enable_track_cost_spend_ = false; |
There was a problem hiding this comment.
Consider using a more descriptive name for the member variable. m_enable_track_cost_spend_ could be renamed to m_enable_performance_tracking_ or m_track_perf_enabled_ to better reflect its purpose.
| int32_t FaceSession::SetEnableTrackCostSpend(bool value) { |
There was a problem hiding this comment.
The SetEnableTrackCostSpend method should validate the TimeSpend object exists before using it. Add a null check for m_face_track_cost_ before calling Reset().
| } | ||
|
|
||
| void FaceSession::PrintTrackCostSpend() { | ||
| if (m_enable_track_cost_spend_) { |
There was a problem hiding this comment.
The PrintTrackCostSpend method should validate the TimeSpend object exists. Add a null check for m_face_track_cost_ before accessing it:
| if (m_enable_track_cost_spend_) { | |
| if (m_enable_track_cost_spend_ && m_face_track_cost_) { |
| return ret; | ||
| } | ||
|
|
||
| int loop = 100; |
There was a problem hiding this comment.
The benchmark loop count is hardcoded to 100. Consider making this configurable via command line argument to allow for different test scenarios.
| MNN::BackendConfig bnconfig; | ||
| bnconfig.power = MNN::BackendConfig::Power_High; | ||
| bnconfig.precision = MNN::BackendConfig::Precision_High; | ||
| bnconfig.precision = MNN::BackendConfig::Precision_Normal; |
There was a problem hiding this comment.
Changed precision from High to Normal without explanation. This could impact model accuracy. Please document the rationale for this change, especially if it's related to the performance tracking feature.
No description provided.