Skip to content

Conversation

@felipepiovezan
Copy link

This test occasionally fails on two of the busiest CI bots (asan and matrix), and we can't reproduce it locally. This leads to the hypothesis that the test is timing out (in the sense of the number of "join attempts" performed by this test's driver).

This commit doubles the number of iterations performed and also does an NFC refactor of the main test loop so that it can be more easily understood.

This test occasionally fails on two of the busiest CI bots (asan and matrix),
and we can't reproduce it locally. This leads to the hypothesis that the test is
timing out (in the sense of the number of "join attempts" performed by this
test's driver).

This commit doubles the number of iterations performed and also does an NFC
refactor of the main test loop so that it can be more easily understood.
Copy link

@adrian-prantl adrian-prantl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the bots you're mentioning, did you mean to land this on llvm/llvm-project main instead?

int count_completed_threads(int num_threads) {
int num_completed_threads = 0;
for (int i = 0; i < num_threads; i++)
if (completed_threads_array[i] == true)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (completed_threads_array[i] == true)
if (completed_threads_array[i])

int iter = 0;
while (1)
{
int max_time_to_wait = 40; // 40 iterations, or 120 seconds

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually multiply all timeouts by a factor 10 when running under ASAN. A reasonably proxy for that would be if getenv("ASAN_OPTIONS") returns a nonempty string, because lit sets that environment variable.

@adrian-prantl
Copy link

Forgot to say — this looks very promising!

@adrian-prantl
Copy link

I landed a variant of this in 869f551.

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.

2 participants