Skip to content

Cleanup work #89976

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

Merged
merged 9 commits into from
Aug 8, 2023
Merged

Cleanup work #89976

merged 9 commits into from
Aug 8, 2023

Conversation

LakshanF
Copy link
Contributor

@LakshanF LakshanF commented Aug 3, 2023

Miscellaneous cleanup work: Implements ep_rt_aot_thread_has_started, RhEventPipeInternal_EventActivityIdControl that allows per-thread activity ID, generating a valid thread handle for streaming_thread in EventPipe (EventPipe calls ep_rt_aot_thread_has_started which will cause issues unless its a valid thread handle now), not handling events related to GCHeapDumpKeyword and TypeKeyword (until issue #88491 is fixed correctly), and expanding testing.

@LakshanF LakshanF added this to the 8.0.0 milestone Aug 3, 2023
@LakshanF LakshanF self-assigned this Aug 3, 2023
@ghost
Copy link

ghost commented Aug 3, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Miscellaneous cleanup work: Implementing ep_rt_aot_thread_has_started, generating a valid thread handle for streaming_thread in EventPipe (EventPipe calls ep_rt_aot_thread_has_started which will cause issues unless its a valid thread handle now), not handling events related to GCHeapDumpKeyword and TypeKeyword (until issue 88491 is fixed correctly), and expanding testing.

Author: LakshanF
Assignees: LakshanF
Labels:

area-NativeAOT-coreclr

Milestone: 8.0.0

@LakshanF LakshanF marked this pull request as draft August 3, 2023 21:49
@LakshanF LakshanF marked this pull request as ready for review August 4, 2023 18:31
@LakshanF LakshanF requested a review from elinor-fung August 4, 2023 18:31
@@ -249,7 +249,7 @@ extends:
extraStepsTemplate: /eng/pipelines/coreclr/nativeaot-post-build-steps.yml
extraStepsParameters:
creator: dotnet-bot
testBuildArgs: 'nativeaot tree ";nativeaot;Loader;Interop;tracing/eventpipe/config;tracing/eventpipe/diagnosticport;tracing/eventpipe/reverse;tracing/eventpipe/processenvironment;tracing/eventpipe/simpleruntimeeventvalidation;tracing/eventpipe/processinfo2;" test tracing/eventcounter/runtimecounters.csproj /p:BuildNativeAotFrameworkObjects=true'
testBuildArgs: 'nativeaot tree ";nativeaot;Loader;Interop;tracing;" /p:BuildNativeAotFrameworkObjects=true'
Copy link
Member

Choose a reason for hiding this comment

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

I recall we hit issues with PR legs timing out when all tracing tests were added to the build. Won't that be a problem again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The build seems faster now with around 50 mins or so. I would like to get fuller test coverage and can reduce the set if this becomes a problem again.

Copy link
Member

Choose a reason for hiding this comment

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

Okay - as long as you're aware / watching out for it. I thought 50 min test build was about what it was when we were timeouts (in combination with the time for running in helix), but may be misremembering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Increased the timeout to 180 minutes in line with some of the long running CI steps.

@elinor-fung
Copy link
Member

Closing and re-opening to pick up #90118.

@elinor-fung elinor-fung closed this Aug 8, 2023
@elinor-fung elinor-fung reopened this Aug 8, 2023
@LakshanF LakshanF merged commit 3c5ad6c into dotnet:main Aug 8, 2023
@LakshanF LakshanF deleted the EventPipeCleanup branch August 8, 2023 13:35
@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants