Skip to content

Conversation

@uweigand
Copy link
Contributor

  • Add missing clause setting the _ep_arch_info variable

* Add missing clause setting the _ep_arch_info variable
@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-Tracing-coreclr and removed community-contribution Indicates that the PR has been added by a community member labels Jul 27, 2021
@tommcdon
Copy link
Member

@josalem @lateralusX

@lateralusX
Copy link
Member

lateralusX commented Aug 30, 2021

Question, that CPU arch, what endianness does it support? Code in EventPipe (or at least consumers of nettrace file format), assumes data is emitted as little endian, so if some data will start to be emitted as big endian, they will not be correctly represented in parsing tools.

@uweigand
Copy link
Contributor Author

Question, that CPU arch, what endianness does it support? Code in EventPipe (or at least consumers of nettrace file format), assumes data is emitted as little endian, so if some data will start to be emitted as big endian, they will not be correctly represented in parsing tools.

s390x is big-endian. I see that code in eventpipe writes binary data in native byte order, so if the consumers expect this to always be little-endian, then this is a problem.

I haven't seen any issues in practice so far, however, so I'm wondering if I'm running all test cases I should be running here. What's the best way to test this subsystem? (Note that on s390x we're currently only supporting the Mono runtime.)

@lateralusX
Copy link
Member

lateralusX commented Aug 30, 2021

Question, that CPU arch, what endianness does it support? Code in EventPipe (or at least consumers of nettrace file format), assumes data is emitted as little endian, so if some data will start to be emitted as big endian, they will not be correctly represented in parsing tools.

s390x is big-endian. I see that code in eventpipe writes binary data in native byte order, so if the consumers expect this to always be little-endian, then this is a problem.

I haven't seen any issues in practice so far, however, so I'm wondering if I'm running all test cases I should be running here. What's the best way to test this subsystem? (Note that on s390x we're currently only supporting the Mono runtime.)

There are low level tests under tests/tracing/eventpipe that runs several produce/consumer scenarios as well as different EventPipe scenarios, but they all run on the same machine, so data will be read back exactly as serialized into nettrace binary format. I guess issues will manifest once you try to parse a nettrace file on a different arch using tools like perfview or TraceEvent library. I don't think the tools make specific assumptions about endianess, but since all just consume the binary data as serialize, currently the produce and consumer needs to run on the same endiness, so in order to get Windows tools like perfview/VS to work, all data serialized into a nettrace file needs to be in little endian format.

@lateralusX
Copy link
Member

Also note that we have had some issue running EventPipe tests on Mono CI, due to test marked as sensitive to tiered JIT has not been executed. I discovered this last week, so working on #58106 to make sure we get them up and running on platforms where we currently can run EventPipe automation.

@lateralusX
Copy link
Member

lateralusX commented Aug 30, 2021

In order to include diagnostics_tracing you need to include that component in build, but maybe you already build using static component support? If so all components (tracing, debugger, hot_reload) will be part of runtime build. If you do dynamic component build, then needed components needs to be deployed during test in order to be loaded and used by runtime. You can find some more details around component support, https://github.com/dotnet/runtime/blob/main/docs/design/mono/components.md and how for example tracing is used by Android/iOS, https://github.com/dotnet/runtime/blob/main/docs/design/mono/diagnostics-tracing.md.

@uweigand
Copy link
Contributor Author

Thanks for the pointers. I'll see if I can get the testsuite going, and if I can find a way to run some cross-platform testing.

@akoeplinger
Copy link
Member

Should we merge this PR in the meantime? We'll need this change regardless of the endianess topic.

@lateralusX
Copy link
Member

lateralusX commented Aug 30, 2021

Yes I think we could do that, it didn't prevent from running EventPipe code in the past on this platform, it is just info that goes into nettrace file as well as used in process info IPC commands.

@lateralusX lateralusX self-requested a review August 30, 2021 13:59
@akoeplinger akoeplinger merged commit 6a77a42 into dotnet:main Aug 30, 2021
@uweigand uweigand deleted the s390x-src-native branch August 30, 2021 15:14
@ghost ghost locked as resolved and limited conversation to collaborators Sep 29, 2021
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