[coap] optimize retransmission timer scheduling - #12366
Conversation
Summary of ChangesHello @abtink, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors and optimizes the CoAP retransmission timer mechanism. By integrating the timer scheduling logic directly into the retransmission handling process and leveraging proactive timer updates, it reduces redundant iterations and improves the efficiency of managing pending CoAP messages. The changes also enhance code organization by encapsulating metadata-related operations within the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request provides a well-executed optimization for the CoAP retransmission timer logic. By removing the ScheduleRetransmissionTimer() method and integrating its logic into HandleRetransmissionTimer(), you've successfully eliminated a redundant iteration over the pending requests list. The use of Timer::FireAtIfEarlier() in CopyAndEnqueueMessage() and the removal of the timer update from DequeueMessage() are also efficient changes that contribute to the overall optimization. The related refactoring, such as introducing new methods on the Metadata struct, significantly improves code readability and encapsulation. The changes are logically sound and enhance both performance and maintainability. Great work!
Library files
|
ce1c6f4 to
e691cae
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request provides a solid optimization for the CoAP retransmission timer scheduling. By removing the separate ScheduleRetransmissionTimer() method and integrating its logic into HandleRetransmissionTimer(), a redundant iteration over the pending requests is avoided. The use of Timer::FireAtIfEarlier() when enqueuing a new message and the removal of the immediate timer reschedule from DequeueMessage() are effective optimizations that improve efficiency. The implementation is clean, correct, and the logic is sound. I approve of these changes.
e691cae to
4048e8e
Compare
This commit optimizes the CoAP retransmission timer logic by removing the `ScheduleRetransmissionTimer()` method, which iterated over all pending requests to determine the next fire time. The logic is updated as follows: - `HandleRetransmissionTimer()` now determines the next fire time while iterating over the `mPendingRequests` list to process retransmissions. This avoids a redundant second pass over the list. - `NextFireTime` is used to track the earliest fire time. - `CopyAndEnqueueMessage()` uses `Timer::FireAtIfEarlier()` to update the timer only if the new message's fire time is earlier than the current schedule. - `DequeueMessage()` no longer triggers a schedule update. If the dequeued message was the next to expire, the timer will fire, perform no actions, and then reschedule itself.
4048e8e to
5884e75
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12366 +/- ##
==========================================
+ Coverage 75.45% 77.17% +1.72%
==========================================
Files 686 688 +2
Lines 93857 91453 -2404
==========================================
- Hits 70817 70580 -237
+ Misses 23040 20873 -2167
🚀 New features to boost your workflow:
|
github-project-automation
Bot
moved this from Under review
to Merged
in Improving CoAP/TMF
3 participants
This commit optimizes the CoAP retransmission timer logic by removing the
ScheduleRetransmissionTimer()method, which iterated over all pending requests to determine the next fire time.The logic is updated as follows:
HandleRetransmissionTimer()now determines the next fire time while iterating over themPendingRequestslist to process retransmissions. This avoids a redundant second pass over the list.NextFireTimeis used to track the earliest fire time.CopyAndEnqueueMessage()usesTimer::FireAtIfEarlier()to update the timer only if the new message's fire time is earlier than the current schedule.DequeueMessage()no longer triggers a schedule update. If the dequeued message was the next to expire, the timer will fire, perform no actions, and then reschedule itself.