Skip to content
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

benchmarks: support tinymembench #2789

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

Fix-Point
Copy link
Contributor

Summary

This patch adds tinymembench for memory bandwidth and latency measuring.

Impact

None.

Testing

Tested on QEMU/x86.

Copy link

@cederom cederom left a comment

Choose a reason for hiding this comment

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

Hey there @Fix-Point :-) Do I miss the C files? :D

@nuttxpr
Copy link

nuttxpr commented Oct 28, 2024

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX Requirements.

Here's why:

  • Summary: While it mentions the change (adding tinymembench), it lacks details.
    • Why is this change necessary? What problem does it solve or feature does it add?
    • How does tinymembench work? What does it measure and how?
  • Impact: Even if the impact is minimal, you should explicitly state it. For example:
    • Impact on user: "Users can now use the tinymembench tool to benchmark memory performance."
    • Impact on documentation: "Documentation for tinymembench has been added."
  • Testing:
    • Insufficient Detail: "Tested on QEMU/x86" is too vague. Specify the QEMU version, the exact x86 target (e.g., qemuarm, qemux86-64), and the NuttX configuration used.
    • Missing Logs: You need to provide actual testing logs before and after the change. This demonstrates the tool's functionality and proves the patch works as intended.

To improve this PR:

  1. Expand the Summary: Explain the rationale and functionality of tinymembench.
  2. Clarify Impact: Even if minimal, state the impact on users, documentation, etc.
  3. Provide Detailed Testing Information: Include QEMU versions, target configurations, and actual testing logs demonstrating the tool's usage and results.

This patch adds tinymembench for memory bandwidth and latency measuring.

Signed-off-by: ouyangxiangzhen <[email protected]>
@Fix-Point
Copy link
Contributor Author

Hey there @Fix-Point :-) Do I miss the C files? :D

Code downloading added. Problems should be fixed. =)

Copy link

@cederom cederom left a comment

Choose a reason for hiding this comment

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

Thank you @Fix-Point :-)

@xiaoxiang781216 xiaoxiang781216 merged commit 85094f3 into apache:master Oct 29, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants