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

Port EventPipeBuffer, EventPipeBufferList, EventPipeBufferManager to Mono. #37756

Merged

Conversation

lateralusX
Copy link
Member

Continuation of #34600, adding BufferManager into the library including unit tests, merging source files and additional library restructuring.

  • Port EventPipeBuffer.
  • Port EventPipeBufferList.
  • Port EventPipeManager.
  • Merge internals source files. Changed GETTER_SETTER to only mandate use of inlined functions when calling betweeen source files for different types. Upgraded all use within each source file for direct struct access.
  • Changed function entry error checkin strategy. Only apply active checks on outer library API, ep.h, and use EP_ASSERT for all other functions.
  • Dropped requires_lock_held in function naming, switch to comment in header or in forward declare for functions that validates that lock is held.
  • Removed currently unused GETTER/SETTERS.
  • Moved more comments into sources from corresponding CoreCLR EventPipe sources.
  • Add EventPipeBuffer/EventPipeBufferManager unit tests.

Next step after this PR will be to enable a file session during runtime and add more unit tests and then start getting diagnostic server component over to Mono.

@josalem
Copy link
Contributor

josalem commented Jun 11, 2020

CC - @noahfalk @sywhang

@lateralusX lateralusX force-pushed the lateralusX/eventpipe-buffer-manager branch 2 times, most recently from 70dc7b0 to 946f9e0 Compare June 12, 2020 14:58
@lateralusX lateralusX force-pushed the lateralusX/eventpipe-buffer-manager branch from e55477d to 46233c6 Compare June 19, 2020 10:15
@lateralusX lateralusX changed the title WIP: Port EventPipeBuffer, EventPipeBufferList, EventPipeBufferManager to Mono. Port EventPipeBuffer, EventPipeBufferList, EventPipeBufferManager to Mono. Jun 22, 2020
Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

LGTM, although I did have a couple of questions.

src/mono/mono/eventpipe/ep.c Outdated Show resolved Hide resolved
src/mono/mono/eventpipe/ep.c Outdated Show resolved Hide resolved
src/mono/mono/mini/mini-runtime.c Show resolved Hide resolved
src/mono/mono/eventpipe/ep-event-source.c Outdated Show resolved Hide resolved
src/mono/mono/eventpipe/ep-event-source.c Outdated Show resolved Hide resolved
src/mono/mono/eventpipe/ep-rt-mono.h Outdated Show resolved Hide resolved
src/mono/mono/eventpipe/ep-rt-mono.h Show resolved Hide resolved
src/mono/mono/eventpipe/ep-config.h Outdated Show resolved Hide resolved
@lateralusX lateralusX force-pushed the lateralusX/eventpipe-buffer-manager branch from 46233c6 to db9feef Compare June 29, 2020 11:05
@lateralusX lateralusX marked this pull request as ready for review June 29, 2020 11:21
@lambdageek
Copy link
Member

lambdageek commented Jun 29, 2020

(now disabled) WASM failure is due to #38573

…Mono.

* Port EventPipeBuffer.
* Port EventPipeBufferList.
* Port EventPipeManager.
* Merge internals source files. Changed GETTER_SETTER to only mandate use
of inlined functions when calling betweeen source files for different types.
Upgraded all use within each source file for direct struct access.
* Changed function entry error checkin strategy. Only apply active checks
on outer library API, ep.h, and use EP_ASSERT for all other functions.
* Dropped requires_lock_held in function naming, switch to comment in header
or in forward declare for functions that validates that lock is held.
* Removed currently unused GETTER/SETTERS.
* Moved more comments into sources from corresponding CoreCLR EventPipe
sources.
* Add EventPipeBuffer/EventPipeBufferManager unit tests.
* Align to changes in native CoreCLR eventpipe* sources.
* Add support starting default file session using env variables.
* Disable eventpipe callback on WASM due to platform issues with delegates.
@lateralusX lateralusX force-pushed the lateralusX/eventpipe-buffer-manager branch from 8df489a to 998e2ad Compare June 29, 2020 20:27
@lateralusX lateralusX merged commit 3e0a169 into dotnet:master Jun 30, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants