-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
[libcxx] [test] Add a test parameter for disabling memory intensive tests #68214
Conversation
…ests Specifically, the test std/input.output/string.streams/stringstream/stringstream.members/gcount.pass.cpp allocates a std::string with INT_MAX-1 elements, and then writes this to a std::stringstream. On Linux, running this test consumes around 5.0 GB of memory; on Windows, it ends up using up to 6.8 GB of memory. This limits whether such tests can run on e.g. GitHub Actions runners, where the free runners are limited to 8 GB of memory. This is somewhat similar to, but still notably different, from the existing test parameter long_tests.
@llvm/pr-subscribers-libcxx ChangesSpecifically, the test This limits whether such tests can run on e.g. GitHub Actions runners, where the free runners are limited to 8 GB of memory. This is somewhat similar to, but still notably different, from the existing test parameter long_tests. Full diff: https://github.com/llvm/llvm-project/pull/68214.diff 2 Files Affected:
diff --git a/libcxx/test/std/input.output/string.streams/stringstream/stringstream.members/gcount.pass.cpp b/libcxx/test/std/input.output/string.streams/stringstream/stringstream.members/gcount.pass.cpp
index 3a5edac6c58b4fe..8dc74421e789590 100644
--- a/libcxx/test/std/input.output/string.streams/stringstream/stringstream.members/gcount.pass.cpp
+++ b/libcxx/test/std/input.output/string.streams/stringstream/stringstream.members/gcount.pass.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
// UNSUPPORTED: 32-bit-pointer
+// REQUIRES: large_tests
// Test that tellp() does not break the stringstream after INT_MAX, due to use
// of pbump() that accept int.
diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index d4e4b722347d623..5911af85c1c8169 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -278,6 +278,14 @@ def getStdFlag(cfg, std):
help="Whether to enable tests that take longer to run. This can be useful when running on a very slow device.",
actions=lambda enabled: [] if not enabled else [AddFeature("long_tests")],
),
+ Parameter(
+ name="large_tests",
+ choices=[True, False],
+ type=bool,
+ default=True,
+ help="Whether to enable tests that use a lot of memory. This can be useful when running on a device with limited amounts of memory.",
+ actions=lambda enabled: [] if not enabled else [AddFeature("large_tests")],
+ ),
Parameter(
name="hardening_mode",
choices=["unchecked", "hardened", "safe", "debug"],
|
I have a slight problem with these kinds of Lit features, because what's considered "large" or "slow" is entirely subjective. This is also the reason why Instead, I think it would be better to have a way to exclude tests that time out after a certain amount of time, or that bypass some system limits like rlimits. WDYT? |
Yes, that's true. In absolute terms, whether something is large might depend on how much memory you have. But if you look at the whole testsuite, where most tests run "reasonably sized" things, and we have one test that uses 5-7 GB of memory, it's pretty clear that there's a distinction there.
At least for the memory intensive test, it's not workable to just try running it and seeing if it works and tolerate failures. Half of the time, this test itself fails due to OOM, but half of the time, another test that is being compiled at the same time gets the OOM error. So we need to proactively stop it from trying to run. If you mean that the tests (and the compiler itself) would be run within some wrapping that limits it from using more than x GB of memory etc, I guess that could work - but it'd be a fair amount of work - way out of scope for what I could take on right here. |
Thanks! @ldionne - how would you feel about backporting this to 17.x? It's not a regression fix in itself, but it would allow running the tests on 17.x successfully. |
I'd be OK with that -- this is not a risky change for the release, so I think that's acceptable. |
…ests (llvm#68214) Specifically, the test std/input.output/string.streams/stringstream/stringstream.members/gcount.pass.cpp allocates a std::string with INT_MAX-1 elements, and then writes this to a std::stringstream. On Linux, running this test consumes around 5.0 GB of memory; on Windows, it ends up using up to 6.8 GB of memory. This limits whether such tests can run on e.g. GitHub Actions runners, where the free runners are limited to 8 GB of memory. This is somewhat similar to, but still notably different, from the existing test parameter long_tests. (cherry picked from commit 122064a)
Local branch amd-gfx ba5461f Merged main:761c9dd92789 into amd-gfx:5dff4b923208 Remote branch main 122064a [libcxx] [test] Add a test parameter for disabling memory intensive tests (llvm#68214)
…ests (#68214) Specifically, the test std/input.output/string.streams/stringstream/stringstream.members/gcount.pass.cpp allocates a std::string with INT_MAX-1 elements, and then writes this to a std::stringstream. On Linux, running this test consumes around 5.0 GB of memory; on Windows, it ends up using up to 6.8 GB of memory. This limits whether such tests can run on e.g. GitHub Actions runners, where the free runners are limited to 8 GB of memory. This is somewhat similar to, but still notably different, from the existing test parameter long_tests. (cherry picked from commit 122064a)
Specifically, the test
std/input.output/string.streams/stringstream/stringstream.members/gcount.pass.cpp allocates a std::string with INT_MAX-1 elements, and then writes this to a std::stringstream. On Linux, running this test consumes around 5.0 GB of memory; on Windows, it ends up using up to 6.8 GB of memory.
This limits whether such tests can run on e.g. GitHub Actions runners, where the free runners are limited to 8 GB of memory.
This is somewhat similar to, but still notably different, from the existing test parameter long_tests.