Set AppStartUp UserTag from engine

[kenzieschmoll]

This is the engine side of flutter/flutter#84884. This 'AppStartUp' user tag will be unset in the framework once app start up is complete (flutter/flutter#86516). Tools will pull the CPU profile tagged with 'AppStartUp' to provide better app start up profiling support.

CC @xster @bkonyi @iskakaushik

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[zanderso]

The presubmit test failures appear to be realted.

[zanderso]

@jason-simmons Do you know if there's any way to get the backtraces in the test logs to show file names and line numbers?

[iskakaushik]

LGTM

[jason-simmons]

@zanderso The Abseil symbolizer library that we recently adopted does not provide line numbers. Apparently that feature is not on their roadmap (abseil/abseil-cpp#382)

[iskakaushik]

I was able to get a backtrace:

Process 2519611 stopped
* thread #1, name = 'eu', stop reason = signal SIGSEGV: invalid address (fault address: 0x58)
    frame #0: 0x0000000003eb8f3e eu`::Dart_NewUserTag(label="AppStartUp") at dart_api_impl.cc:7030:3
   7027
   7028	DART_EXPORT Dart_Handle Dart_NewUserTag(const char* label) {
   7029	  Thread* thread = Thread::Current();
-> 7030	  CHECK_ISOLATE(thread->isolate());
   7031	  DARTSCOPE(thread);
   7032	  if (label == nullptr) {
   7033	    return Api::NewError(

Looks like we might need to Dart_EnterIsolate before being able to create a new user tag. cc: @bkonyi to confirm.

Instructions to get better backtraces:

1. Copy cp out/host_debug_unopt/exe.unstripped/embedder_unittests out/host_debug_unopt/eu. This is needed due to rpath inconsistencies. I will shortly file an issue for this.
2. lldb out/host_debug_unopt/eu
3. r -- --gtest_filter="*CanLaunchAndShutdownWithValidProjectArgs"

[kenzieschmoll]

Looks like we might need to Dart_EnterIsolate before being able to create a new user tag. cc: @bkonyi to confirm.

Are you suggesting that we make a call to Dart_EnterIsolate(<someIsolate>) directly before calling Dart_SetCurrentUserTag(Dart_NewUserTag("AppStartUp")); in the engine? Or that we need to make a change to the impl of the UserTag apis added to the SDK?

If it is the former, what isolate would we want to pass to Dart_EnterIsolate?

[iskakaushik]

@kenzieschmoll Given that you wish to set the start and end tags prior to the start of the app and after the end of the app respectively, I think the API shouldn't expect you to have an isolate started. Given this, I think the Dart impl should allow for setting and unsetting of user tags without having to call enter isolate.

[zanderso]

That might be difficult. I believe the user tag is stored on the current isolate: https://github.com/dart-lang/sdk/blob/master/runtime/vm/isolate.h#L1498. @a-siva or @rmacnak-google might know more.

[kenzieschmoll]

Given that you wish to set the start and end tags prior to the start of the app and after the end of the app respectively

Just a small correction on this - we wish to set the "AppStartUp" tag at the start of the app if possible. So whereever the main isolate is created / started, we should be setting the "AppStartUp" UserTag. And then we want to unset the "AppStartUp" UserTag once app start up has completed, which has already landed in the framework (flutter/flutter#86516).

[a-siva]

That might be difficult. I believe the user tag is stored on the current isolate: https://github.com/dart-lang/sdk/blob/master/runtime/vm/isolate.h#L1498. @a-siva or @rmacnak-google might know more.

Yes, UserTags are stored per isolate and the C API call Dart_NewUserTag requires an isolate to be present when it is invoked.
Isolates typically run independently and are also profiled independently, they could also be running different code so a UserTag defined in one isolate may not make sense in another isolate. The new isolate-group feature under development could potentially have a use case of Usertags being relevant across isolates in a group, but setting/unsetting of tags would still be specific to an isolate as they run independent of each other.

[chinmaygarde]

Kaushik's variant of the patch has landed. Closing this one.