-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[RISC-V] Increase timeout for tracing/eventcounter/runtimecounters #104461
[RISC-V] Increase timeout for tracing/eventcounter/runtimecounters #104461
Conversation
@@ -104,7 +104,7 @@ public static int TestEntryPoint() | |||
// Create an EventListener. | |||
using (RuntimeCounterListener myListener = new RuntimeCounterListener()) | |||
{ | |||
Thread.Sleep(3000); | |||
Thread.Sleep(60000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A 1 minute sleep in an inner loop test seems like too much. Is there some to wait exactly until the counters are ready?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about smth like this?
// Create an EventListener.
using (RuntimeCounterListener myListener = new RuntimeCounterListener())
{
// Wait max 60 seconds
for (int i = 0; i < 60; i++)
{
Thread.Sleep(1000);
if (myListener.Verify())
{
Console.WriteLine("Test passed");
return 100;
}
}
Console.WriteLine($"Test Failed - did not see one or more of the expected runtime counters.");
return 1;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked version from #104461 (comment) with checked build on riscv64 VisionFive2:
Did not see cpu-usage
Saw cpu-usage
Saw working-set
Saw gc-heap-size
Saw gen-0-gc-count
Saw gen-1-gc-count
Saw gen-2-gc-count
Saw threadpool-thread-count
Saw monitor-lock-contention-count
Saw threadpool-queue-length
Saw threadpool-completed-items-count
Saw alloc-rate
Saw active-timer-count
Saw gc-fragmentation
Saw gc-committed
Saw exception-count
Saw time-in-gc
Saw gen-0-size
Saw gen-1-size
Saw gen-2-size
Saw loh-size
Saw poh-size
Saw assembly-count
Saw il-bytes-jitted
Saw methods-jitted-count
Saw time-in-jit
Saw gen-0-gc-budget
Saw total-pause-time-by-gc
Test passed
If you think that approach from #104461 (comment) is ok, I'll update PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with this part of the runtime... we'll probably have to wait for @tommcdon once the US holiday is over.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not have objections to the approach listed in #104461 (comment). Adding @noahfalk @hoyosjs for additional thoughts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I've updated PR
RISC-V qemu-prio1-checked: 9343 / 9346 (99.97%)
Failed
Killed
Skipped
|
RISC-V starfive-prio1-checked: 9327 / 9346 (99.80%)
Failed
Killed
Skipped
|
RISC-V qemu-prio1-checked: 9343 / 9346 (99.97%)
Failed
Killed
Skipped
|
RISC-V starfive-prio1-checked: 9327 / 9346 (99.80%)
Failed
Killed
Skipped
|
This test expects that all runtime counters are already prepared when timeout has passed, however, it might be untrue for debug/checked builds and for VisionFive2 (riscv64).
f507f2f
to
06b308b
Compare
Can this be merged? |
This test expects that all runtime counters are already prepared when timeout has passed, however, it might be untrue for debug/checked builds (specifically, debug/checked build on VisionFive2 riscv64).
Part of #84834, cc @dotnet/samsung