It's not pretty, but it makes the test run a lot faster.
Details
- Reviewers
ldionne Mordante philnik - Group Reviewers
Restricted Project - Commits
- rGc1c29eb9f7be: Fix failed libcxx test build on the Windows to Linux cross builders. NFC.
rGbb588519c5b5: [libc++] Run modules_include.sh.cpp compiles in parallel
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I wonder whether bash and wait will work across different platforms but otherwise looks better than my solution.
Probably not, but it's already XFAILed for AIX, FreeBSD, Windows and Android, so it probably wasn't really portable before.
Some numbers:
Run | Test old time | Test new time | all old time | all new time |
C++2b | 853s | 55s | 15m 33s | 8m 58s |
Modular build | 763s | 51s | 14m 7s | 13m 6s |
C++03 | 227s | 14s | 4m 54s | 4m 7s |
I have no idea what's going on on the windows runners. That's probably unrelated, since on windows the test is marked UNSUPPORTED. On Apple the test has a timeout now, probably because of resource contention, since this test starts ~140 processes in parallel now and they are much weaker than the Linux runners. I'll try to change the script to run at most 16 processes in parallel to avoid the contention. In a perfect world we'd use the lit thread pool for this, but that would require teaching lit about multiple independent processes from a single test file, which would take a lot longer than changing the script here. Let's fix the test first and then make it better.
I'll land this now, since it fixes the CI and actually lets a lot of the runs be a lot faster. Feel free to review post-commit and I'll create a patch to address any comments.
Thanks for this. On Arm 32 bit the test is down to 5 minutes now (could be faster I expect but we have a limited core count).
Hi @philnik,
here is a problem with the test during the remote runs on the windows cross builders. A lot of those messages because of broken path:
clang++: error: no such file or directory: 'C:buildbotas-builder-1x-armv7lllvm-projectlibcxxtestlibcxxmodules_include.sh.cpp' clang++: error: no input files ...
- https://lab.llvm.org/buildbot/#/builders/60/builds/10870/steps/14/logs/FAIL__llvm-libc__-static_cfg_in__modules_include_s
- https://lab.llvm.org/buildbot/#/builders/119/builds/12143/steps/14/logs/FAIL__llvm-libc__-static_cfg_in__modules_include_s
This is the windows to arm-linux cross builders, which run the tests on the arm/aarch64 boards.
Would you fix that or revert the changes?
Since this is isn't a libc++-supported configuration AFAICT and reverting the patch would break our own CI, I don't think that's an option. I'm not actually sure why the test runs at all, since it looks like you are running a windows-arm build, which should be disabled by the // UNSUPPORTED: windows. It might be enough to drop the bash -c to avoid making the backslashes escape sequences. I can't remember why I used the in the first place, since I just use echo directly in other places. I'll make a patch to simplify this.
libcxx/test/libcxx/modules_include.sh.cpp | ||
---|---|---|
40 | I'm not sure how I would do that without building a thread pool. Just the number of cores doesn't really help. At that point it's probably easier to integrate with lit. |
@vvereschaka I think your issue with escaping should indeed be solved by https://reviews.llvm.org/D144825. I suspect you ran into this issue because your executor (which must be some flavor of libcxx/utils/ssh.py must treat escapes differently.
Could you work with us on transitioning your build bot over to BuildKite so we run it as pre-commit? This is our supported way of doing CI now and it has the benefit that we see problems before the happen, not after.
Since this is isn't a libc++-supported configuration AFAICT and reverting the patch would break our own CI, I don't think that's an option.
This is supported libc++ configuration.
I'm not actually sure why the test runs at all, since it looks like you are running a windows-arm build, which should be disabled by the // UNSUPPORTED: windows.
Because the target is Arm/Aarch64 Linux. This is Window-to-ARM-Linux cross builders. // UNSUPPORTED: windows works for the Windows targets.
So, the test became broken on the windows cross builders. I'm going to disable it there for awhile.
@vvereschaka I think your issue with escaping should indeed be solved by https://reviews.llvm.org/D144825.
Unfortunately it wasn't solved.
I suspect you ran into this issue because your executor (which must be some flavor of libcxx/utils/ssh.py must treat escapes differently.
I use libcxx's (libcxx/utils/ssh.py) executor to run the remote tests. The problem occurs on the build host side during the test preparations, before invoking ssh.py.
Could you work with us on transitioning your build bot over to BuildKite so we run it as pre-commit?
I'll think, I'll try to find some hardware for that purpose. But currently there is no possibility to convert the existing cross builders into the buildkite agents.
Ah, that makes sense. I thought it was compiling for windows-arm, which we don't support.
So, the test became broken on the windows cross builders. I'm going to disable it there for awhile.
@vvereschaka I think your issue with escaping should indeed be solved by https://reviews.llvm.org/D144825.
Unfortunately it wasn't solved.
I suspect you ran into this issue because your executor (which must be some flavor of libcxx/utils/ssh.py must treat escapes differently.
I use libcxx's (libcxx/utils/ssh.py) executor to run the remote tests. The problem occurs on the build host side during the test preparations, before invoking ssh.py.
Could you work with us on transitioning your build bot over to BuildKite so we run it as pre-commit?
I'll think, I'll try to find some hardware for that purpose. But currently there is no possibility to convert the existing cross builders into the buildkite agents.
Unfortunately I don't think I can really try to fix it without having access to some hardware. If you could give me access to a machine I'd be happy to work on fixing this.
Unfortunately I don't think I can really try to fix it without having access to some hardware. If you could give me access to a machine I'd be happy to work on fixing this.
Sorry, I can't give an access currently. But the problem is clear - this is a handling of the windows-style pathes by the current test. lit and clang handle them correctly, but when you switch to the unix tool (such as bash) on Windows, these tools do not work properly with that kind of pathes. They process the \x characters within the windows-style path as the control sequences and break the original path. The test became too bash specific/depended and now it needs a proper processing/converting the pathes (\ -> /) to work correctly on Windows hosts. sed -e s/\\/\//g on %s may help may be, but anyway it doesn't look portable.
libcxx/test/libcxx/modules_include.sh.cpp | ||
---|---|---|
40 | Then please use a way with lit, this test now uses a lot of resources on machines with less than 16 cores. |
Can you limit this to the number of cores on the system, for example, by using nproc?