ocaml-multicore / eio

Effects-based direct-style IO for multicore OCaml
Other
557 stars 72 forks source link

Add stress fd test #763

Closed RichardChukwu closed 3 hours ago

RichardChukwu commented 2 days ago

Title: Add Stress Test for Open File Descriptor (FD) Leaks

Description:

This pull request adds a new stress test (stress_fd.ml) to check for file descriptor (FD) leaks in Eio. The test ensures that the number of open FDs at the beginning of the test matches the count at the end, helping to identify resource leaks under high load.

Test Details:

Test Objective: Validate that Eio properly handles file descriptors during concurrent domain and fiber operations without leaks. Approach:

The test creates multiple domains and fibers, which perform various IO operations (e.g., reading/writing to a file). At the end of the test, the number of open file descriptors is checked to ensure no leaks occurred.

Rationale: This addresses a common source of performance degradation and resource exhaustion when managing IO operations in parallel.

Benchmark Improvements:

This test contributes to the ongoing effort to improve benchmarking and stress testing of Eio, particularly in areas related to OS integration, as discussed in issue #450.

Related Issues: related to #450: Add more stress tests and benchmarks

RichardChukwu commented 1 day ago

Hello @patricoferris I'd appreciate your feedback on this, thank you

talex5 commented 21 hours ago

This looks AI-generated to me. The code doesn't match the description. Also, https://github.com/ocaml-multicore/eio/issues/762#issuecomment-2394783534 looks AI-generated to me, though I'm not very familiar with this stuff.

@patricoferris do we need to add some rule somewhere about this? I see https://kernelnewbies.org/Outreachyfirstpatch says "Contributions that include content generated by AI tools that may rely on sources that are not compatible with GPLv2 thus cannot be accepted into the Linux kernel. This includes ChatGPT." Maybe we need something similar for Eio?

RichardChukwu commented 21 hours ago

Does it mean I can't work on this until issue #761 is resolved please? @patricoferris

@talex5 I looked at the other stress tests and wrote the code in the same manner, please I'd like to know if that applies?

patricoferris commented 3 hours ago

Does it mean I can't work on this until issue https://github.com/ocaml-multicore/eio/issues/761 is resolved please?

Yes, that's right. Let's keep our focus on a single issue and PR.

Contributions that include content generated by AI tools that may rely on sources that are not compatible with GPLv2 thus cannot be accepted into the Linux kernel. This includes ChatGPT.

I think we should have a rule, though I'm not sure what it is. I'm not a lawyer, and from the very light searching I did, it would seem that the licensing issue exists in a pretty grey area. However, that's not to say we can't have our own rule. I'll draft something for the CONTRIBUTING.md maybe?

@RichardChukwu -- we really appreciate you taking the time to engage and contribute to Eio. To be clear, I don't think what we're saying here is that contributions can't use AI. But it should be used as a tool to help write a PR, not as a tool to write the whole PR, your comments and PR description for you. There are a few reasons for this.

  1. Most importantly, it obfuscates for us how you think. Without being able to see how you think, it makes it much harder for us to help you learn and subsequently to land the PR.
  2. It is more work for us. This is related to (1), but reviewing code generated by AI at this point is inefficient and unproductive. We could have piped the issue into ChatGPT (or similar) ourselves, but the point of Outreachy is to help you contribute to open-source.

So I think we can maybe try this PR again, with your thoughts, descriptions, code and maybe even bugs :)) ? In which case, I'm going to close this PR. Looking forward to seeing a new one when you have the time.

RichardChukwu commented 2 hours ago

Duly noted @patricoferris thank you so much for this feedback, I understand you perfectly and will take corrections accordingly.

Let me figure out another issue to work on then. How about assigning #761 to me first, since that's needed for this isuse to be worked on and is more related to Windows?