-
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
Initial work porting native diagnostic event pipe library to C library potentially shared between runtimes. #34600
Merged
lateralusX
merged 1 commit into
dotnet:master
from
lateralusX:lateralusX/eventpipe-native-mono-port
May 26, 2020
Merged
Initial work porting native diagnostic event pipe library to C library potentially shared between runtimes. #34600
lateralusX
merged 1 commit into
dotnet:master
from
lateralusX:lateralusX/eventpipe-native-mono-port
May 26, 2020
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lateralusX
added
area-Tracing-mono
NO-MERGE
The PR is not ready for merge yet (see discussion for detailed reasons)
NO-REVIEW
Experimental/testing PR, do NOT review it
and removed
area-Tracing-coreclr
labels
Apr 6, 2020
lateralusX
force-pushed
the
lateralusX/eventpipe-native-mono-port
branch
from
April 8, 2020 07:00
8fc14f8
to
061b40b
Compare
Why not just use the C++? |
It has been decided to do a C port of this library since it has been decided that Mono won't take dependencies on C++ at the moment. Also the library is currently tied closely to CoreCLR artefacts so in order to share it, we need to make it less clr specific. |
noahfalk
reviewed
Apr 9, 2020
lateralusX
force-pushed
the
lateralusX/eventpipe-native-mono-port
branch
2 times, most recently
from
April 22, 2020 09:19
f907c50
to
43c5ae5
Compare
lambdageek
reviewed
May 13, 2020
lateralusX
force-pushed
the
lateralusX/eventpipe-native-mono-port
branch
2 times, most recently
from
May 19, 2020 18:22
1aa75b8
to
e784dd5
Compare
lateralusX
changed the title
WIP: Port native diagnostic event pipe library to C library potentially shared between runtimes.
Port native diagnostic event pipe library to C library potentially shared between runtimes.
May 19, 2020
lateralusX
removed
the
NO-MERGE
The PR is not ready for merge yet (see discussion for detailed reasons)
label
May 19, 2020
lambdageek
requested review from
akoeplinger,
alexischr,
CoffeeFlux,
marek-safar,
SamMonoRT,
thaystg and
vargaz
as code owners
May 19, 2020 19:40
lambdageek
reviewed
May 19, 2020
monojenkins
pushed a commit
to monojenkins/mono
that referenced
this pull request
Jun 29, 2020
…Mono. Continuation of dotnet/runtime#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.
lateralusX
added a commit
to mono/mono
that referenced
this pull request
Jun 30, 2020
…Mono. (#19958) Continuation of dotnet/runtime#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. Co-authored-by: lateralusX <[email protected]>
lateralusX
added a commit
to lateralusX/runtime
that referenced
this pull request
Sep 4, 2020
library that can be shared between Mono as well as CoreCLR runtime. Port follows same guidelines setup for event pipe port dotnet#34600. Diagnostic server library is currently hosted as part of event pipe library but hosting its own runtime shim as well as source files (so could be split into separate library if ever make sense). Diagnostic Server have dependencies on event pipe library (and reuse part of event pipe runtime shim from its how shim). This is the first PR getting the code from diagnostic server codebase over to C library. Once that is done there will be follow up PR's starting to enabling more of the CoreCLR tests suites over event pipe currently depending on diagnostic server and connection between diagnostic server and event pipe library. Current state of this PR is port of most diagnostic server code + Windows PAL, but currently POSIX PAL is still WIP.
5 tasks
5 tasks
lateralusX
added a commit
to lateralusX/runtime
that referenced
this pull request
Oct 2, 2020
library that can be shared between Mono as well as CoreCLR runtime. Port follows same guidelines setup for event pipe port dotnet#34600. Diagnostic server library is currently hosted as part of event pipe library but hosting its own runtime shim as well as source files (so could be split into separate library if ever make sense). Diagnostic Server have dependencies on event pipe library (and reuse part of event pipe runtime shim from its how shim). This is the first PR getting the code from diagnostic server codebase over to C library.
monojenkins
pushed a commit
to monojenkins/mono
that referenced
this pull request
Oct 2, 2020
…tentially shared between runtimes. PR handling port of Diagnostic Server C++ library from CoreCLR into a C library that can be shared between Mono as well as CoreCLR runtime. Port follows same guidelines setup for event pipe port dotnet/runtime#34600. Diagnostic server library is currently hosted as part of event pipe library but hosting its own runtime shim as well as source files (so could be split into separate library if ever make sense). Diagnostic Server have dependencies on event pipe library (and reuse part of event pipe runtime shim from its own shim). This is the first PR getting the code from diagnostic server codebase over to C library. Once that is done there will be follow up PR's starting to enabling more of the CoreCLR tests suites over event pipe currently depending on diagnostic server and connection between diagnostic server and event pipe library. Diagnostic server processinfo, https://github.com/dotnet/runtime/tree/master/src/tests/tracing/eventpipe/processinfo, test pass running on Windows/Linux Mono. eventpipe buffersize, https://github.com/dotnet/runtime/tree/master/src/tests/tracing/eventpipe/buffersize, test pass running on Windows/Linux Mono, connecting diagnostic server with eventpipe and IPC test clients and parsers. TODO's before this PR can be merged: - [x] Fix all builds (currently only validated on Windows). - [x] Port POSIX PAL. - [x] Get pass on process info on Linux/Mac enabling the test as part of PR. - [x] Implement WriteEvent icall as well as starting IPC streaming thread in event pipe library. - [x] Get pass on one test consuming event pipe data over IPC.
lateralusX
added a commit
to mono/mono
that referenced
this pull request
Oct 5, 2020
…tentially shared between runtimes. (#20351) PR handling port of Diagnostic Server C++ library from CoreCLR into a C library that can be shared between Mono as well as CoreCLR runtime. Port follows same guidelines setup for event pipe port dotnet/runtime#34600. Diagnostic server library is currently hosted as part of event pipe library but hosting its own runtime shim as well as source files (so could be split into separate library if ever make sense). Diagnostic Server have dependencies on event pipe library (and reuse part of event pipe runtime shim from its own shim). This is the first PR getting the code from diagnostic server codebase over to C library. Once that is done there will be follow up PR's starting to enabling more of the CoreCLR tests suites over event pipe currently depending on diagnostic server and connection between diagnostic server and event pipe library. Diagnostic server processinfo, https://github.com/dotnet/runtime/tree/master/src/tests/tracing/eventpipe/processinfo, test pass running on Windows/Linux Mono. eventpipe buffersize, https://github.com/dotnet/runtime/tree/master/src/tests/tracing/eventpipe/buffersize, test pass running on Windows/Linux Mono, connecting diagnostic server with eventpipe and IPC test clients and parsers. TODO's before this PR can be merged: - [x] Fix all builds (currently only validated on Windows). - [x] Port POSIX PAL. - [x] Get pass on process info on Linux/Mac enabling the test as part of PR. - [x] Implement WriteEvent icall as well as starting IPC streaming thread in event pipe library. - [x] Get pass on one test consuming event pipe data over IPC. Co-authored-by: lateralusX <[email protected]>
lateralusX
added a commit
that referenced
this pull request
Oct 5, 2020
… C (#41872) library that can be shared between Mono as well as CoreCLR runtime. Port follows same guidelines setup for event pipe port #34600. Diagnostic server library is currently hosted as part of event pipe library but hosting its own runtime shim as well as source files (so could be split into separate library if ever make sense). Diagnostic Server have dependencies on event pipe library (and reuse part of event pipe runtime shim from its how shim). This is the first PR getting the code from diagnostic server codebase over to C library.
ghost
locked as resolved and limited conversation to collaborators
Dec 9, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR handling port of EventPipe C++ library from CoreCLR into a C library that can be shared between Mono as well as CoreCLR runtime.
Due to dependencies and current time line it has been decided to initially target Mono runtime (since it doesn't have event pipe functionality at the moment), but library is still designed to integrate into other runtimes, replacing current C++ library implementation, when time permits.
Until that happens, we will need to make sure changes done in C++ code base gets ported over to C library. The amount of changes going into the native library should however be small and if major changes needs to be done, we should first make sure to move over to use the shared version of event pipe C-library in CoreCLR and then only do changes into shared version of the library.
Port is currently mapping existing C++ code base, no focus on rewrite or change current implementation (unless needed to port code over to C).
Library source files are currently placed into mono/eventpipe folder and build as a separate library, linked into runtime, but will be moved when ready to be shared between runtimes. We might also move all code into a loadable library and p/invokes, depending on size, at least for Mono runtime targeting mobile devices.
There is also a unit test runner under mono/eventpipe/tests using eglib test runner, running through a set of unit tests covering the eventpipe library implementation.
Library is split up over a couple of source files closely mapping corresponding coreclr source files. Most code could be shared between runtimes and artifacts that are runtime specific, like locks/lists/maps/strings/pal layer etc, a shim API is defined in
ep-rt.h
andep-rt-types.h
that gets implemented by the targeted runtime consuming the event pipe C library, likeep-rt-mono.h
andep-rt-types-mono.h
. All function in the shim API is small and should be inlinable. For example, locking the configuration lock is declared like this inep-rt.h
:and in
ep-rt-mono.h
it is implemented like this:and CoreCLR should implement the function using its corresponding locking primitives. NOTE, since we primarily target Mono in initial port there might smaller adjustments needed to the shim API once bringing it over to CoreCLR, but those changes/adjustments can be done when needed.
Since library could be shared, it tries to standardize on common C99 data types when available. If there are runtime specific types needed, they are declared in
ep-rt-types.h
and implemented by each runtime in corresponding file. Most of library is currently following Mono coding guidelines.For shared types naming is done like this,
EventPipe[Type]
, likeEventPipeProvider
. This naming follow naming of other types in Mono as well as CoreCLR. There is one exception to this rule, types that are runtime specific implementation, naming is following function naming with a _t prefix, likeep_rt_session_provider_list_t
, that is a list specific type, implemented by each runtime. This is done to make it easier to see difference of types implemented in the runtime specific shim and types implemented in the shared library API surface.Since library shim implemented by targeted runtime will be called by C code, it can't throw exceptions, the C library should however compile under C++, meaning that CoreCLR's shim should be able to use C++ constructions as long as it don't throw any exceptions.
Library function naming follows the following pattern, for "public" functions included in
ep-*.h
, ep_ + object + action, so for example,ep_config_create_provider
, corresponds toEventPipe::Configuration::CreateProvider
in C++ library. For "private/internal" functions not, ep_ prefix is dropped, but object + action is still preserved, for exampleconfig_create_provider
and function is marked as being static when possible. If a function assumes a lock is held when getting called, it is using _lock_held (similar to SAL annotations) postfix to indicate that function doesn't claim lock and if function needs lock to be held by caller, it will assert that condition on enter (similar toPRECONDITION(EventPipe::IsLockOwnedByCurrentThread())
).Library uses asserts internally to validate state and conditions. This is a debug/checked build assert that should be excluded in release builds. It is defined in
EP_ASSERT
and should be redefined by targeted runtime. Private/internal functions asserts incorrect input parameters and state, while "public" functions return back error to caller when detecting incorrect input parameters. Incorrect state can still be assert in all kinds of functions.Getters/Setters are setup for accessing type members. All functions except a type's own alloc/init/fini/free implemented in
ep-*.c
are not allowed to use direct type member access and should go through available inlinable getter/setter functions. There is also a way to build the library that catches incorrect use of members at compile time (not inep-*-internals.c
since it include alloc/init/fini/free implementations, but in other source files).Types that can be stack allocated offers a init/fini function pairs. Types that should only be heap allocated, alloc/free, and types supporting both stack/heap, implements alloc/init/fini/free. Since C can't prevent incorrect usage patterns, the implementation needs to follow correct life time management patterns. Types allocated using alloc should be freed with corresponding free function. For example,
ep_event_alloc
should be freed byep_event_free
, the init/fini methods are then called by the alloc/free implementations. For types supporting stack allocation and allocated on stack or aggregated within other types, init must be called before first use and fini before going out of scope.Ownership of allocated objects is only considered complete on successful function calls. If a function fails, caller still owns resource and need to take actions.
Library uses UTF8 strings internally, mainly since Mono runtime already uses that and it is more portable. Number of strings in the library is limited, and in cases where stream protocol uses UTF16 strings, a prealloc string is stored on the type, reducing need to do multiple UTF8 -> UTF16 conversions over the lifetime of the type.
NOTE, this is first PR setting up most of the direction of the library and (as part of doing that) ported a great deal of eventpipe code and added ~40 native unittests. It is however not complete, or fully working, so there will be several follow up PR's building on top of this work in order to get complete library. Next component to integrate into the library, once this PR is merged, is BufferManager.