-
Notifications
You must be signed in to change notification settings - Fork 690
testing/sched/rwsem_test: add comprehensive test suite for rwsem opti… #3397
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
| @@ -0,0 +1,320 @@ | |||
| # RWSem Test Coverage Summary | |||
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.
I haven't reviewed your code in detail but this documentation in its entirety is AI-generated slop. It's my opinion that it has no place in this PR or in the NuttX project.
…mization VELAPLATFO-51602 Add a comprehensive test suite to validate the reader-writer semaphore optimization that reduces unnecessary context switches. The optimization ensures up_wait() is only called when the lock is actually released (writer/reader count reaches 0), avoiding premature wake-ups. Test Coverage: - Core Correctness Tests (6 tests): * Multiple readers with no writers (concurrent read access) * Multiple writers exclusive access verification * Mixed reader-writer access patterns * Waiter wake-up correctness * Lock holder tracking (recursive write lock support) * Context switch reduction verification - Performance Tests (3 tests): * Recursive write lock performance * Converted lock (read->write) performance * High contention multi-threaded performance (8 threads) - Basic Functionality Tests (1 test): * Basic read/write/recursive lock operations Key APIs Tested: - init_rwsem() / destroy_rwsem() - down_read() / up_read() - down_write() / up_write() - Recursive locking behavior - Lock holder field tracking This test suite validates commit Id3b49dda0309b098f04e8ab499c28c94fe1f77ce which optimizes rwsem to reduce context switches in NuttX. Change-Id: Icabbc9a65e879d6689529d0c8e3aecbdbd9bd6be Signed-off-by: Yongrong Wang <wangyongrong@xiaomi.com>
raiden00pl
left a comment
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.
AI garbage
|
Why this testcase is AI garbage ? My view is that the API being tested is a common, POSIX-standard API. Therefore, it’s perfectly acceptable to use AI-generated tests or to reuse/adapt existing external test cases. There’s no need to reinvent the wheel by writing another brand-new test suite from scratch. |
|
@GUIDINGLI Did you even check the content of this commit?
Here's another example of AI garbage from Xiaomi: #3381 If you're going to use AI, at least do it right, and don't produce a wall of worthless text that no one reads. |
Yes, I have seen the external file, you can request the author to delete the intermediate, but can NOT say all the files are garbage. |
|
@wyr-7 |
|
@GUIDINGLI The PR was closed by the author. Another question: what license is this AI-generated code? Can you assure us that this is Apache compatible? |
Why not?
Closed by the author, not us. |
If a developer cannot take the time to write documentation for their tests, or even to write the tests, why should we bother reviewing it? |
|
@GUIDINGLI @raiden00pl @linguini1 @linguini1 wants rwsem test data. I used an ai tool to generate a relatively complete test case for testing. I think there's no problem with this. And @linguini1 wants me to share the test case with him, so I uploaded it here to give him a link. I didn't want to incorporate this pr into nuttx-apps. I think there's nothing wrong with using ai appropriately. What's wrong is that I shouldn't have pushed it here. It should only be pushed to my own homepage,so I closed this PR. |
It's time to learn how AI can help you development before you said the code generated by AI is garbage.
All my team member require to use AI to assist him writing code/test, analysis bug and write document. |
@raiden00pl You need ask the lawyer, but ALL Xiaomi employer(include my team) do the development with AI everyday. At least, I don't hear any licence issue from Xiaomi legel team. |
@wyr-7 BTW, please refuse the unreasonable requirement from @linguini1 in the future. If you want the new test, please develop it by yourself @linguini1 . |
|
@wyr-7 If this PR wasn't supposed to be merged, then fine. But it would be worth mentioning that in the content :)
@xiaoxiang781216 Can you ensure that AI-generated code doesn't contain copyleft licenses code like GPL?. It's up to humans to ensure that the code being generated has the correct license. Sorry, but I'm a little skeptical of your legal team statement, especially since the rest of the world hasn't really solved this problem yet, and copyright infringement by AI is a fact.
Then you use this AI tools in a wrong way, because AI-generated content like in this PR #3381 is obvious garbage and shouldn't be merged in the first place. At least read if what the AI has produced for you makes sense and add "Be concise" to your prompts |
You're right, thank you for providing the code you used to test. I did request on another PR (don't have the link off-hand) that you share the tests case you used to verify your change. I appreciate you sharing that information with me, AI-generated or not, because it gives me more information about your testing. The reason I reviewed this negatively is as you said, it shouldn't be merged into NuttX. I appreciate you wanting to upstream your testing, but there is currently an ongoing discussion about admitting AI code into NuttX. Currently my stance is no. If you have a different opinion, please share it in the discussion on the mailing list so we can make an informed decision when/if we vote on it!
Appropriate PR testing information is a requirement for contributions to NuttX, in the contributing guidelines and voted for in the mailing least. I don't know why this is an unreasonable request. I will not be writing test cases for everyone's PRs: if you want it merged, prove that it works. |
Let's AI generate the right code isn't a simple task. You need tell him many detail steps and verify the generated code carefully(this proccess need iterate many times before getting the final result), so it's the developer and AI cowork generate the code. Anyway, this is out of topic, let's see what's happen in the next couple years. |
|
@xiaoxiang781216 I'm not against AI. This is just tool like any other. I've been using AI for years for various tasks and I know it sucks for osdev tasks. Most important, anything AI produces must be checked and verified by a human so we can avoid what I call "AI garbage". |
First, Agree with your point: Then, But not all of them. Let's check the real code to say whether it is "garbage". The cases generated by AI are more rigorous than those written by engineers in some scenarios. |
The copyright ownership of AI-generated code lacks a uniform global legal ruling and is a core contentious issue in the global intellectual property field. I can't assure AI-generated code suit with Apache. |
|
The main difference between us is our stance on AI tools. |
@GUIDINGLI Generated file is not even close to nuttx standard and should not be merged.
I agree, but we have to be defensive when it comes to licensing. Copyleft licences are "viral" in nature and they can completely ruin your product. Keep in mind that the approach to copyright varies in different parts of the world.
As I said earlier, AI is a tool like any other. It can be used in the right way or it can be used in the wrong way. You can boost your productivity, or you can destroy it (and introduce bugs and security holes). Just remember that by boosting your team's productivity with AI, someone else must later review your changes. If you don't review your AI output, you're shifting that responsibility onto the community, which is not OK. You're boosting your team productivity at the expense of the community. I certainly won't advocate for a complete AI ban, but some rules are essential. Besides, a complete AI ban is practically impossible, because how can it be verified? However, this isn't nuttx-specific problem, but a global one. |


Add a comprehensive test suite to validate the reader-writer semaphore optimization that reduces unnecessary context switches. The optimization ensures up_wait() is only called when the lock is actually released (writer/reader count reaches 0), avoiding premature wake-ups.
Test Coverage:
Core Correctness Tests (6 tests):
Performance Tests (3 tests):
Basic Functionality Tests (1 test):
Key APIs Tested:
This test suite validates commit Id3b49dda0309b098f04e8ab499c28c94fe1f77ce which optimizes rwsem to reduce context switches in NuttX.