Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@christyuj
Copy link

Hi, everyone. I am a member of alibaba xianyu team. At the invitation of xster, I would like to present some patches that have been repaired by the team for reference.

For the current online 1.5.4 version engine, we have sorted out some internal patches. I know these codes are not a perfect solution, but I hope to share them and explain some specific problems and demands in some scenarios.

Here are some stability and some extended capabilities of texture. As xianyu has 300 million users at present, we have some requirements on online stability and performance

I focus on the reasons for the two commit and the ideas for the modifications, which I hope will help the stability of the engine in the future.
Some of the other such as commit aa40f57 has been handled by my colleague kangwang1988 illustrated in #6145, I will not repeat.

Commit 3755a5d

Problem description: Flutter will crash when rendered in the background

Exception Type: SIGSEGV
Exception Codes: SEGV_ACCERR at 0x1
Triggered by Thread: 24

Thread 24 Crashed:
0 libGPUSupportMercury.dylib 0x000000018b1d1f90 _gpus_ReturnNotPermittedKillClient :12 (in libGPUSupportMercury.dylib)
1 GLEngine 0x0000000185d691f4 0x0000000185c78000 + 987636
2 GLEngine 0x0000000185d690f8 0x0000000185c78000 + 987384
3 OpenGLES 0x0000000185d77c58 -[EAGLContext presentRenderbuffer:] :72 (in OpenGLES)
4 Flutter 0x0000000103a75de8 0x0000000103a54000 + 138728
5 Flutter 0x0000000103a7880c 0x0000000103a54000 + 149516
6 Flutter 0x0000000103aa7744 0x0000000103a54000 + 341828
7 Flutter 0x0000000103aa7c34 0x0000000103a54000 + 343092
8 Flutter 0x0000000103aa7fd0 0x0000000103a54000 + 344016
9 Flutter 0x0000000103aa7acc 0x0000000103a54000 + 342732
10 Flutter 0x0000000103aab4a0 0x0000000103a54000 + 357536
11 Flutter 0x0000000103a82968 0x0000000103a54000 + 190824
12 Flutter 0x0000000103a83884 0x0000000103a54000 + 194692

Problem analysis: the Flutter Engine instance corresponds to four taskrunners :Platform, UI, GPU and IO. GPU and IO are involved in GPU operation. The current Message Loop cannot guarantee that there is no GPU/IO operation when the App goes back to the background.

Solutions:

  1. Control the size of tasks in each message Loop to ensure that they do not accumulate too many.
  2. Before switching to the background, check whether the GPU and IO TaskRunner have any unfinished tasks, if so, block the main thread until the task is completed.

Commit dfe3d98

Problem description: xianyu hopes to reuse Native image library and video library in the App. After it is implemented directly through the Flutter Texture mechanism, it is found that the memory is large and the whole rendering process is long, which cannot meet the online requirements.

Problem analysis:
The specific scheme is shown in the figure below
image.png

Solutions:
If you want to implement the scenario in the figure above, you need to expose some of the specified interfaces on the engine side.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@xster
Copy link
Member

xster commented Jul 9, 2019

Thanks! Can you make a different PR for each commit/bug/feature? Since each entire PR needs to be reviewed/merged, it makes it easier for the review process.

#if TARGET_IPHONE_SIMULATOR
return [CALayer class];
#else // TARGET_IPHONE_SIMULATOR
// #if TARGET_IPHONE_SIMULATOR
Copy link
Member

Choose a reason for hiding this comment

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

Please don't check-in commented code. If there are annotations needed to explain the context of code, please add some text comments.


- (void)disableGPUOperation {
[[_engine.get() lifecycleChannel] sendMessage:@"AppLifecycleState.paused"];
//暂时通过延时来等待GL操作结束(否则进入后台后的GL操作会闪退)
Copy link
Member

Choose a reason for hiding this comment

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

Can you guys change these comments to english? :D

Copy link
Author

Choose a reason for hiding this comment

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

sure,we will change it later

@cbracken
Copy link
Member

cbracken commented Aug 5, 2019

@xster @christyuj it looks like the this PR is for the purpose of discussion around a couple of changes to the engine. Since I assume the eventual plan is to break out a few orthogonal patches, I'd propose creating a fork and discussing those patches there, or opening a doc/email thread.

@xster who on the team should be involved in reviewing this? @christyuj do you plan to make the changes suggested by @xster (e.g. translate comments to English), which would help reviewers?

@cbracken cbracken closed this Aug 5, 2019
@xster
Copy link
Member

xster commented Aug 5, 2019

They're practical and useful fixes but I think the next step is to break this PR down to smaller per feature/issue PRs.

@cbracken, these follow up PRs should likely be reviewed by the engine team.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants