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.

Sign up for GitHub

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

Jump to bottom

Disable LTTNG in runtime tests by default #67937

Merged
merged 1 commit into from
Apr 13, 2022
Merged

Disable LTTNG in runtime tests by default #67937

merged 1 commit into from
Apr 13, 2022

Conversation

hoyosjs
Copy link
Member

@hoyosjs hoyosjs commented Apr 13, 2022

Disables LTTNG support on tests by default. A lot of extra work is done with is - it is not pay for play and all VM events should be available through eventpipe + eventlistener.

cc: @trylek

@hoyosjs hoyosjs requested a review from a team April 13, 2022 00:14
@ghost ghost assigned hoyosjs Apr 13, 2022
@ghost
Copy link

ghost commented Apr 13, 2022

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

Disables LTTNG support on tests by default. A lot of extra work is done with is - it is not pay for play and all VM events should be available through eventpipe + eventlistener.

cc: @trylek

Author: hoyosjs
Assignees: hoyosjs
Labels:

area-Infrastructure-coreclr
Milestone: -

josalem
josalem approved these changes Apr 13, 2022
View reviewed changes
Copy link
Contributor

@josalem josalem left a comment

Choose a reason for hiding this comment

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

Have we been running our tests with COMPlus_EnableEventLog on by default this whole time? That can be a pretty serious perf hit.

trylek
trylek approved these changes Apr 13, 2022
View reviewed changes
Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

LGTM, thank you! I'm not yet completely clear on what happens in the lab because the run.sh script is not used there; yet the symptoms I've been seeing in the r2r-extra pipeline w.r.t. the JIT/Methodical test merging change lead me to think whether we might be somehow setting the option there too. I have launched an instrumented r2r-extra run with added runtime logging that should shed more light on this.

@hoyosjs hoyosjs merged commit 7eb5d7b into main Apr 13, 2022
@hoyosjs hoyosjs deleted the juhoyosa/disable-lttng-for-test branch April 13, 2022 06:45
@ghost ghost locked as resolved and limited conversation to collaborators May 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jkoritzinsky jkoritzinsky jkoritzinsky approved these changes

@stephentoub stephentoub stephentoub approved these changes

@davmason davmason davmason approved these changes

@trylek trylek trylek approved these changes

@josalem josalem josalem approved these changes

Assignees

@hoyosjs hoyosjs

Labels
area-Infrastructure-coreclr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@hoyosjs @jkoritzinsky @stephentoub @davmason @trylek @josalem