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.

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

Merged
merged 5 commits into from
Dec 19, 2020
Merged

Test suite tweaks for rr #35539

merged 5 commits into from
Dec 19, 2020

Conversation

Keno
Copy link
Member

@Keno Keno commented Apr 21, 2020

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

@Keno Keno changed the title Two small tweaks to stdlib tests Test suite tweaks for rr Apr 22, 2020
@Keno
Copy link
Member Author

Keno commented Apr 22, 2020

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.

@staticfloat staticfloat force-pushed the kf/stdlibtests branch 2 times, most recently from 2058749 to 4d9e13b Compare May 8, 2020 03:25
base/download.jl Outdated Show resolved Hide resolved
base/download.jl Outdated Show resolved Hide resolved
@Keno
Copy link
Member Author

Keno commented Sep 29, 2020

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.

@Keno
Copy link
Member Author

Keno commented Nov 29, 2020

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.
@Keno
Copy link
Member Author

Keno commented Dec 19, 2020

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.

@Keno Keno merged commit 742a4a0 into master Dec 19, 2020
@Keno Keno deleted the kf/stdlibtests branch December 19, 2020 13:48
@Keno Keno added the backport 1.6 Change should be backported to release-1.6 label Dec 19, 2020
@KristofferC KristofferC mentioned this pull request Jan 5, 2021
26 tasks
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants