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

sof_perf_analyzer: add skip-to-first-trace option #1100

Merged
merged 3 commits into from
Sep 7, 2023

Conversation

aiChaoSONG
Copy link

In CI test, some traces from previous test case will apprear in the mtrace of current test case, this is a know issue.

Previously, we always skip to the first valid trace of this test case, this could raise trouble for developer use of this script.

This patch adds skip-to-first-trace option, for CI test, this option should be set (True), and for developer use, this option should be left as it is (False).

@aiChaoSONG aiChaoSONG requested a review from singalsu September 6, 2023 06:18
singalsu
singalsu previously approved these changes Sep 6, 2023
Copy link
Contributor

@singalsu singalsu left a comment

Choose a reason for hiding this comment

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

This patch fixes the problem that I had. Thank you very much!

Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

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

The pylint warning may go away if you drop the too broad except?

tools/sof_perf_analyzer.py Outdated Show resolved Hide resolved
tools/sof_perf_analyzer.py Outdated Show resolved Hide resolved
tools/sof_perf_analyzer.py Outdated Show resolved Hide resolved
In CI test, some traces from previous test case will
apprear in the mtrace of current test case, this is
a know issue.

Previously, we always skip to the first valid trace
of this test case, this could raise trouble for developer
use of this script.

This patch adds skip-to-first-trace option, for CI
test, this option should be set (True), and for developer
use, this option should be left as it is (False).

Signed-off-by: Chao Song <[email protected]>
marc-hb
marc-hb previously approved these changes Sep 7, 2023
Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

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

Nit: could you add a blank line before 190?

Note: still a Github Draft

case-lib/lib.sh Outdated Show resolved Hide resolved
Chao Song added 2 commits September 7, 2023 12:20
Due to a known issue in CI test, some traces from
previous test case will apprear in the mtrace of
current test case. Set skip-to-first-trace option
to skip those traces.

Signed-off-by: Chao Song <[email protected]>
The skip_to_first_trace() function either return a value
or raise an exception. Currently there is no good way
to do type annotation for such function, so remove its
type annotation.

Signed-off-by: Chao Song <[email protected]>
@aiChaoSONG aiChaoSONG changed the title [skip ci]sof_perf_analyzer: add skip-to-first-trace option sof_perf_analyzer: add skip-to-first-trace option Sep 7, 2023
@aiChaoSONG aiChaoSONG marked this pull request as ready for review September 7, 2023 04:25
@aiChaoSONG aiChaoSONG requested a review from a team as a code owner September 7, 2023 04:25
@aiChaoSONG
Copy link
Author

Thanks Marc, the test is good, see internal result 31419, merge

@aiChaoSONG aiChaoSONG merged commit 471a1da into thesofproject:main Sep 7, 2023
3 checks passed
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.

3 participants