-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Test suite tweaks for rr #35539
Test suite tweaks for rr #35539
Conversation
I've pushed two additional test suite tweaks that are needed for the test suite to pass under rr and retitled the PR. With this merged, the test suite passes for me under rr. |
2058749
to
4d9e13b
Compare
4d9e13b
to
71f5094
Compare
71f5094
to
1ed0e00
Compare
Given the recent CI instability, it seems worth pushing this over the finish line. I've rebased this. Let's see what rr has to say. |
Alright, starting to work through the rr-specific test failures. The Profile test failure should be fixed by rr-debugger/rr#2753. I believe other than that we have issues in Distributed, Downloads and Threads. The Threads test failure just looks like a timeout caused by rr overhead, but I'm considering just not running the threads test under rr anyway, since it can hide things like memory model issues, which that test definitely needs to catch. I haven't looked into the Distributed and Downloads test failures yet. |
We want to start running rr on CI, but at the moment the threads test is clearing ENV, causing it to lose the LD_PRELOAD that improves rr performance. Fix that by instead passing through the environment and setting the relevant JULIA_NUM_THREADS flag on top of it.
If the FDs we allocated are too high, we leaked fds somewhere. On linux it's fairly easy to dump out information about all active FDs, so do that in the failure case, so the logs can help in debugging.
By default, I'm having the test suite set up to put any workers spawned with addprocs into their own rr sessions. This doesn't work with SharedArrays, because those workers share memory with the main process. Add a new JULIA_RR environment variable that specifies which rr to use for the test suite and a flag to the testsuite's addprocs command to determine whether or not to detach the workers from the current rr session.
RR changes the affinity, so 0,1 may not be available in the affinity mask. It's a bit complicated to fix this test and would require parsing the existing affinity mask, so just disable it for now.
Alright, the rr tracing seems to finally work. Will merge this PR when regular CI Is green and then we'll turn on rr by default in a few days. |
threads: Pass through environment to thread_exec (for LD_PRELOAD for rr)
FileWatching: If the fds we allocated were to high, dump out what all those other fds were up to