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.

Sign up for GitHub

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

Jump to bottom

clangd/unittests/TUSchedulerTests.cpp fails and timesout frequently #64964

Closed
hiraditya opened this issue Aug 24, 2023 · 6 comments
Closed

clangd/unittests/TUSchedulerTests.cpp fails and timesout frequently #64964

hiraditya opened this issue Aug 24, 2023 · 6 comments
Labels
bug Indicates an unexpected problem or unintended behavior clangd test-suite

Comments

@hiraditya
Copy link
Collaborator

For more logs see for example:

Test logs pasted here.

Driver produced command: cc1 -cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name no_cmd.h -mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=all -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -funwind-tables=2 -target-cpu x86-64 -tune-cpu generic -debugger-tuning=gdb -fcoverage-compilation-dir=/clangd-test -resource-dir lib/clang/17 -D MAIN -internal-isystem lib/clang/17/include -internal-isystem /usr/local/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -fdeprecated-macro -fdebug-compilation-dir=/clangd-test -ferror-limit 19 -fgnuc-version=4.2.1 -fcxx-exceptions -fexceptions -no-round-trip-args -faddrsig -D__GCC_HAVE_DWARF2_CFI_ASM=1 -x c++-header /clangd-test/no_cmd.h
Rebuilding invalidated preamble for /clangd-test/no_cmd.h version null (previous was version null)
out/llvm-project/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:1354: Failure
Value of: S.blockUntilIdle(timeoutSeconds(10))
  Actual: false
Expected: true
Built preamble of size 217192 for file /clangd-test/no_cmd.h version null in 11.22 seconds
ASTWorker building file /clangd-test/unreliable.h version null with command inferred from main.cpp
[/clangd-test]
clang -DMAIN -x c++-header -- /clangd-test/unreliable.h
Driver produced command: cc1 -cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name unreliable.h -mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=all -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -funwind-tables=2 -target-cpu x86-64 -tune-cpu generic -debugger-tuning=gdb -fcoverage-compilation-dir=/clangd-test -resource-dir lib/clang/17 -D MAIN -internal-isystem lib/clang/17/include -internal-isystem /usr/local/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -fdeprecated-macro -fdebug-compilation-dir=/clangd-test -ferror-limit 19 -fgnuc-version=4.2.1 -fcxx-exceptions -fexceptions -no-round-trip-args -faddrsig -D__GCC_HAVE_DWARF2_CFI_ASM=1 -x c++-header /clangd-test/unreliable.h
Building first preamble for /clangd-test/unreliable.h version null
out/llvm-project/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:1354: Failure
Value of: S.blockUntilIdle(timeoutSeconds(10))
  Actual: false
Expected: true
Built preamble of size 217204 for file /clangd-test/unreliable.h version null in 10.22 seconds
ASTWorker building file /clangd-test/ok.h version null with command 
[/clangd-test]
clang -xobjective-c++-header /clangd-test/ok.h
Driver produced command: cc1 -cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name ok.h -mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=all -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -funwind-tables=2 -target-cpu x86-64 -tune-cpu generic -debugger-tuning=gdb -fcoverage-compilation-dir=/clangd-test -resource-dir lib/clang/17 -internal-isystem lib/clang/17/include -internal-isystem /usr/local/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -fdeprecated-macro -fdebug-compilation-dir=/clangd-test -ferror-limit 19 -fgnuc-version=4.2.1 -fobjc-runtime=gcc -fobjc-encode-cxx-class-template-spec -fobjc-exceptions -fcxx-exceptions -fexceptions -no-round-trip-args -faddrsig -D__GCC_HAVE_DWARF2_CFI_ASM=1 -x objective-c++-header /clangd-test/ok.h
Building first preamble for /clangd-test/ok.h version null
Built preamble of size 220364 for file /clangd-test/ok.h version null in 0.06 seconds
out/llvm-project/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:1354: Failure
Value of: S.blockUntilIdle(timeoutSeconds(10))
  Actual: false
Expected: true
ASTWorker building file /clangd-test/not_included.h version null with command clangd fallback
[/clangd-test]

In another run:

out/llvm-project/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:1354: Failure
Value of: S.blockUntilIdle(timeoutSeconds(10))
  Actual: false
Expected: true

Built preamble of size 207320 for file /clangd-test/foo.cpp version 3 in 5.44 seconds
Reusing preamble version 3 for version 4 of /clangd-test/foo.cpp
out/llvm-project/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:1318: Failure
Value of: Collector.diagVersions().back()
Expected: has a first field that is equal to "3", and has a second field that is equal to "3"
  Actual: ("3", "4"), whose second field does not match

out/llvm-project/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:1311
Value of: Collector.diagVersions().back()
Expected: has a first field that is equal to "3", and has a second field that is equal to "3"
  Actual: ("2", "3"), whose first field does not match
out/llvm-project/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:1318
Value of: Collector.diagVersions().back()
Expected: has a first field that is equal to "3", and has a second field that is equal to "3"
  Actual: ("3", "4"), whose second field does not match
@hiraditya hiraditya added bug Indicates an unexpected problem or unintended behavior clangd labels Aug 24, 2023
@llvmbot
Copy link
Member

llvmbot commented Aug 24, 2023

@llvm/issue-subscribers-bug

@llvmbot
Copy link
Member

llvmbot commented Aug 24, 2023

@llvm/issue-subscribers-clangd

@hiraditya
Copy link
Collaborator Author

Created a patch to configure clangd tests apart from clang tools tests. https://reviews.llvm.org/D158566

@HighCommander4
Copy link
Collaborator

See also #59644, #50117, clangd/clangd#1712

@kadircet
Copy link
Member

kadircet commented Sep 1, 2023

Thanks for the report @hiraditya, the correctness issue is because we weren't asserting for executing all the work (hence we falsely, succeeded). Sent out https://reviews.llvm.org/D159337.

Regarding the timeouts, the machine seems to be really busy again, building trivial preambles take ~11 secs: Built preamble of size 217192 for file /clangd-test/no_cmd.h version null in 11.22 seconds. Bumping timeouts in https://reviews.llvm.org/D159338.

Considering these timeout issues are started happening recently, I wonder if we're spawning too many threads with the recent test execution strategy changes. Can you give some details of the machine you're running the tests/builds on @hiraditya ?

kadircet added a commit that referenced this issue Sep 1, 2023
@kadircet
[clangd][tests] Assert for idleness of scheduler
69feef0 
We could fail going idle in 5 seconds and get spurious errors
afterwards. See #64964.

Differential Revision: https://reviews.llvm.org/D159337
kadircet added a commit that referenced this issue Sep 1, 2023
@kadircet
[clangd][tests] Bump timeouts in TUSchedulerTests to 60 secs
16b8b3e 
There are some slow/congested bots, that can't go idle in 10 secs, see #64964

Differential Revision: https://reviews.llvm.org/D159338
@hiraditya
Copy link
Collaborator Author

Can you give some details of the machine you're running the tests/builds on @hiraditya ?

I dont have the details, these are MacOS ci machines. Maybe there is something in the artifact directory you find helpful: https://ci.android.com/builds/submitted/10685355/darwin_mac/latest ?

@kadircet kadircet mentioned this issue Sep 6, 2023
Closed
avillega pushed a commit to avillega/llvm-project that referenced this issue Sep 11, 2023
@kadircet
[clangd][tests] Assert for idleness of scheduler
aadb91e 
We could fail going idle in 5 seconds and get spurious errors
afterwards. See llvm#64964.

Differential Revision: https://reviews.llvm.org/D159337
avillega pushed a commit to avillega/llvm-project that referenced this issue Sep 11, 2023
@kadircet
[clangd][tests] Bump timeouts in TUSchedulerTests to 60 secs
be875b0 
There are some slow/congested bots, that can't go idle in 10 secs, see llvm#64964

Differential Revision: https://reviews.llvm.org/D159338
avillega pushed a commit to avillega/llvm-project that referenced this issue Sep 11, 2023
@kadircet
[clangd][unittests] Limit paralelism for clangd unittests
0de8b21 
We started seeing a lot of timeouts that align with the change in lit to
execute gtests in shards. The logic there assumes tests are
single-threaded, which is the case for most of the LLVM, hence they
pick #shards ~ #cores (by slightly overshooting).

There are enough unittests in clangd that rely on multi-threading, they
can create arbitrarily many threads but we limit amount of meaningful
work to ~4 thread per process.

This change ensures that we're accounting for that paralelism when
executing clangd tests and not overloading test executors.

In theory the change overestimates the requirements, not all tests are
multi-threaded, but it doesn't seem to be resulting in any regressions
on my local runs.

Fixes llvm#64964.
Fixes clangd/clangd#1712.
qihangkong pushed a commit to rvgpu/llvm that referenced this issue Apr 18, 2024
@kadircet @ZijunZhaoCCK
[clangd][tests] Assert for idleness of scheduler
2f30b26 
We could fail going idle in 5 seconds and get spurious errors
afterwards. See llvm/llvm-project#64964.

Differential Revision: https://reviews.llvm.org/D159337
qihangkong pushed a commit to rvgpu/llvm that referenced this issue Apr 18, 2024
@kadircet @ZijunZhaoCCK
[clangd][tests] Bump timeouts in TUSchedulerTests to 60 secs
27802c0 
There are some slow/congested bots, that can't go idle in 10 secs, see llvm/llvm-project#64964

Differential Revision: https://reviews.llvm.org/D159338
qihangkong pushed a commit to rvgpu/llvm that referenced this issue Apr 18, 2024
@kadircet @ZijunZhaoCCK
[clangd][unittests] Limit paralelism for clangd unittests
b487f8a 
We started seeing a lot of timeouts that align with the change in lit to
execute gtests in shards. The logic there assumes tests are
single-threaded, which is the case for most of the LLVM, hence they
pick #shards ~ #cores (by slightly overshooting).

There are enough unittests in clangd that rely on multi-threading, they
can create arbitrarily many threads but we limit amount of meaningful
work to ~4 thread per process.

This change ensures that we're accounting for that paralelism when
executing clangd tests and not overloading test executors.

In theory the change overestimates the requirements, not all tests are
multi-threaded, but it doesn't seem to be resulting in any regressions
on my local runs.

Fixes llvm/llvm-project#64964.
Fixes clangd/clangd#1712.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Indicates an unexpected problem or unintended behavior clangd test-suite
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[clangd][unittests] Limit paralelism for clangd unittests kadircet/llvm-project
5 participants
@kadircet @HighCommander4 @hiraditya @EugeneZelenko @llvmbot