Skip to content

Conversation

directhex
Copy link
Contributor

This is partially cursed, as runtime.git's packaging infra is opinionated about e.g. overriding PackageID

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

Does this PR produce the package? Was going to download and look at it.

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

So I manually tested the build artifact, and I'm seeing:

02-18 16:56:20.751 20237 20237 W monodroid: Initializing profiler with options: aot:port=9999,output=/data/user/0/com.mono.profiler.hellomaui/files/.__override__/profile.aotprofile
02-18 16:56:20.752 20237 20237 W monodroid: The 'aot' profiler wasn't found in the main executable nor could it be loaded from 'libmono-profiler-aot.so'.

It is inside the .apk:

lib\arm64-v8a\libmono-profiler-aot.so

I reverted to the old files I built myself, and those seem to work. It can load:

02-18 17:04:21.449 26961 26961 W monodroid: Looking for profiler init symbol 'mono_profiler_init_aot'? 0x7000c7855c

Is there enough changes in main, where a build from this PR wouldn't work alongside release/6.0?

@steveisok
Copy link
Member

@jonathanpeppers is what you're trying to build easy so that we can test the same thing?

@jonathanpeppers
Copy link
Member

jonathanpeppers commented Feb 22, 2022

It looks like the symbol for mono_profiler_init is there:

$ nm -D -C libmono-profiler-aot.so | grep mono_profiler_init
0000d7cd T mono_profiler_init_aot

The binary I have that works:

$ nm -D -C libmono-profiler-aot.so | grep mono_profiler_init
00015f11 T mono_profiler_init_aot

So then I looked at all symbols with mono, and there is a new one:

$ nm -D -C libmono-profiler-aot.so | grep mono_profiler_set_inline_method_callback
         U mono_profiler_set_inline_method_callback

This symbol wasn't there before, would this cause an issue?

@steveisok
Copy link
Member

This symbol wasn't there before, would this cause an issue?

Yes, you're going to need a lib that is 1-1 with the runtime version you're using. I can generate one from release/6.0 that you could try.

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

I manually tested a build based on release/6.0 and it seems to work:

https://dev.azure.com/dnceng/internal/_build/results?buildId=1627363&view=results

  Read total 380759 bytes...
  Summary:
        Modules:         30
        Types:          966
        Methods:      3,969
  Going to write the profile to 'custom.aprof'

@steveisok
Copy link
Member

Since the transport package is most useful in release/6.0, we shouldn't merge this to main just yet.

@steveisok steveisok added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Feb 24, 2022
@ghost ghost closed this Mar 26, 2022
@ghost
Copy link

ghost commented Mar 26, 2022

Draft Pull Request was automatically closed for inactivity. Please let us know if you'd like to reopen it.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 25, 2022
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Build-mono NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants