search

flutter / flutter

Flutter makes it easy and fast to build beautiful apps for mobile and beyond
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
165.36k stars 27.28k forks source link

Rename GPU thread to raster thread #29443

Closed liyuqian closed 4 years ago

liyuqian commented 5 years ago

GPU thread seems to be a confusing name as it sounds like everything happens on the GPU but it's actually not. Maybe it's better to fix it sooner (when we're still relatively small) than later.

Specifically,

  1. Even if we're using GPU, a lot of work on the "GPU" thread currently happens on the CPU. For example, shader compilations.
  2. Sometimes this thread involves no GPU at all (e.g., when --enable-software-rendering is given, or when Flutter is running on the iOS simulator)

One rename suggestion is to follow Android's naming of RenderThread so Android developers will immediately feel familiar.

liyuqian commented 5 years ago

CC @chinmaygarde @cbracken @kenzieschmoll @Hixie

kenzieschmoll commented 5 years ago

Looping in @sfshaza2 and @InMatrix as this naming will affect our docs and tooling. We use "UI" and "GPU" to describe the threads in current documentation - https://flutter.dev/docs/testing/ui-performance.

CC @jacob314

chinmaygarde commented 5 years ago

Sounds good to me. I have been referring to it as the render thread in presentations for similar reasons.

InMatrix commented 5 years ago

I'm supportive of finding a more accurate name for the GPU thread. I have a couple of questions:

One rename suggestion is to follow Android's naming of RenderThread so Android developers will immediately feel familiar.

Will this analogy bring incorrect assumption about how Flutter's "render" thread works?

Related, I found Chrome shows a "GPU" thread in its profiler: image

Is this "GPU" thread in the same nature as Flutter's "GPU" thread?

liyuqian commented 5 years ago

I see that "render thread" could also be a little misleading since Flutter also has both a Dart rendering layer, and an engine rendering layer. Maybe let's call it "rasterize thread" since it's mainly about Rasterizer::Draw and we haven't used "rasterize" anywhere else yet.

I'm not very familiar with the Chrome GPU thread. I'm asking someone who's working on Chrome and I'll let you know once I heard back.

liyuqian commented 5 years ago

Just heard back:

So far, Chrome's GPU thread only deals with GL commands so it's different from our "GPU thread". https://www.chromium.org/developers/design-documents/gpu-accelerated-compositing-in-chrome https://www.chromium.org/developers/design-documents/gpu-command-buffer

jacob314 commented 5 years ago

I think we'll need to verify that whatever name we pick is familiar enough to flutter users. It would be nice if the name we pick is accurate but a slightly misleading name could be alright if it points users in the right general direction. Our users don't need to land changes to the flutter engine, just get an approximate idea of where the slowdown their app is experiencing may be coming from.

On Wed, Mar 20, 2019 at 2:57 PM liyuqian notifications@github.com wrote:

Just heard back:

So far, Chrome's GPU thread only deals with GL commands so it's different from our "GPU thread".

https://www.chromium.org/developers/design-documents/gpu-accelerated-compositing-in-chrome https://www.chromium.org/developers/design-documents/gpu-command-buffer

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/flutter/flutter/issues/29443#issuecomment-475044339, or mute the thread https://github.com/notifications/unsubscribe-auth/ABK4PMDvhQShQ6S3VtMxMzJygnCa2oEVks5vYq7DgaJpZM4b3CE0 .

dnfield commented 5 years ago

While we're at it, should we rename the UI thread to the Dart thread? Or something indicating that it's the thread used for Dart code? It gets confusing for Android, where @UiThread means what we call the Platform thread.

liyuqian commented 5 years ago

Dart thread sounds like a good idea to me. But I didn't touch that thread often so maybe it's more important to know what the framework team thinks about this name.

timsneath commented 4 years ago

I agree that any change should be holistic. Adding @csells to brainstorm ideas here since he can both canvass others and has priors here from previous projects.

csells commented 4 years ago

I agree that we should use a more precise name. Adding @Hixie for his thoughts.

filiph commented 4 years ago

My thoughts on this:

However, I don't have strong opinion about this. I do want to get to a decision quite soon, though, since I have some video recording scheduled, and would like to have the terminology right.

What's the next step to get this terminology change approved (or rejected)?

timsneath commented 4 years ago

I have no issues with either of these (a slight preference for "raster" over "rasterization", but I'm speaking as a pundit rather than in any capacity worth paying attention to!). Your explanations are clear to me and preferable to "GPU thread" and "Dart thread".

On Wed, Mar 4, 2020 at 2:23 PM Filip Hracek notifications@github.com wrote:

My thoughts on this:

  • As an alternative name for "GPU thread", I like "rasterization thread". It's almost completely accurate, has no naming conflicts (that I know of), and is still short enough. For people who haven't heard the term rasterization, it's easily searchable https://www.google.com/search?q=rasterization. It can be abbreviated to "raster thread", for example when we need to fit it in a devtools UI.
  • As an alternative name for "CPU thread", I like "main thread". It's what people are using, and it's also quite accurate (it's where the main() function gets called, and it's also the "main" thread of the app).
    • I like "main thread" more than "Dart thread", because the existence of "Dart thread" seems to suggest that there is only one Dart thread, which is not true (and kind of a dangerous idea to proliferate).

However, I don't have strong opinion about this. I do want to get to a decision quite soon, though, since I have some video recording scheduled, and would like to have the terminology right.

What's the next step to get this terminology change approved (or rejected)?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/flutter/flutter/issues/29443?email_source=notifications&email_token=AARWL6Z3VBT7SIDZRQILD5TRF3IGDA5CNFSM4G64EE2KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEN2VR2I#issuecomment-594893033, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARWL64JO3C5EVIZ5KDS2VLRF3IGDANCNFSM4G64EE2A .

liyuqian commented 4 years ago

"raster/rasterization thread" and "main thread" sound good to me. Please feel free to organize a meeting to see if most people feel good about those names, and drive the effort of making the renaming happen. I feel that the majority of work would be on documentation and tools. In the engine, @chinmaygarde and I should be able to modify the performance overlay, and GPUTaskRunner/UITaskRunner without too much overhead.

As mentioned earlier, this will have a long lasting and wide impact that's harder to make as time goes by, so doing it early is definitely preferable.

cbracken commented 4 years ago

One big concern I have with renaming what we currently call our 'UI thread' to the 'main thread' is the iOS also calls something the 'main thread' and that thread is the thread we call our 'platform thread'.

dnfield commented 4 years ago

Raster thread is good.

I don't like "main thread". Main thread should normally mean the thread that was used for invoking the main method of your native application - e.g. the Platform task runner in our current lingo, and the UiThread in Android lingo. The current UI thread is very different from that.

liyuqian commented 4 years ago

Let me also propose "Build thread" for the current UI thread. To summarize,

So far, "raster thread" seem to be preferable by many, while "Dart thread" vs "Main thread" is a little bit unclear?

filiph commented 4 years ago

Thanks for the quick response, everyone!

Unless someone tells me otherwise, I'll consider "raster thread" a winner for the future name of "GPU thread".

For "CPU thread", here are some alternatives:

Can you all please reply with your favorites and with your "no go" lists?

jacob314 commented 4 years ago

My preferences in order:

  1. main thread
  2. event loop thread <-- extra alternative
  3. widgets thread
  4. app thread

No gos: prime thread master thread <-- seems like a more confusing version of main. There isn't something overly special about the initial isolate. build thread. <-- fine if users understand it is widget build but confusing if they think of a different type of build

cbracken commented 4 years ago

My preferences for the UI thread:

  1. App thread
  2. UI thread (are we renaming it because of confusion with the Android UI thread which is our platform thread? If so, then put this on my no-go list for the exact same reason as main thread)

No go:

  1. main thread (This has a very different meaning on iOS -- specifically it's what we call our platform thread)
  2. prime thread
  3. master thread
  4. build/widgets thread: these are both specific to the widgets layer of the framework -- I think most people would get it... but it's a bit misleading since you could write an app that only uses dart:ui or just the render layer etc.
chinmaygarde commented 4 years ago

GPU Thread Preferences:

  1. Render Thread: Engine components already use this terminology but it clashes with Androids thread names.
  2. Raster Thread: Not completely accurate but close enough.

CPU Thread Preferences:

  1. Application/App Thread.
  2. Root Isolate Thread.
  3. UI Thread: Engine components use this terminology but it clashes with platform thread naming conventions.
  4. Dart Thread.

None of the current suggestions are completely accurate as the workloads on these threads are quite disparate (and will likely change as the engine evolves). So, let's just pick one that is the least confusing.

dnfield commented 4 years ago

I think that whatever name we come up with, it should have these properties:

These all point me back to "Dart" thread, or perhaps "Root Isolate Thread".

chinmaygarde commented 4 years ago

These all point me back to "Dart" thread, or perhaps "Root Isolate Thread".

I think of all suggestions, "Root Isolate Thread" is the most accurate but I am not sure how obvious that is to the user. That term is mostly used in the engine.

cbracken commented 4 years ago

"Root isolate thread" is wordy but agreed -- it's the most accurate by far. I prefer it to "Dart thread" mostly because you can legitimately have multiple isolates and they're all Dart threads. I suspect the sort of people who are likely to care about what's running on which thread are likely to be fine with that name. And needless to say, we should have clear docs on this either way.

On the fence about "App thread" -- of all the names so far it's my favourite of the short ones, but I think I object to it for the same reason I object to "Dart thread" -- the Dart bits of your app can run on multiple theads (or should be able to barring current bugs :P).

chinmaygarde commented 4 years ago

We should open this up to the community for a vote. I'd be delighted to call it "Thready McThreadface".

On the fence about "App thread" -- of all the names so far it's my favourite of the short ones...

Agreed.

liyuqian commented 4 years ago

BTW, I've also added "Dart thread" and the original "UI thread" to the list. (I guess we might end up not renaming UI thread at all if we disagree too much on the new name.) If you haven't considered them, please feel free to modify your vote.

For "build thread", it could also be referenced to layer tree build, or scene build. I think that's something necessary even if one just uses dart:ui instead of widgets. I picked "build thread" because "frame build time" and "frame raster time" are already frequently used in performance testing to differentiate the current "UI thread" and "GPU thread".

@InMatrix @jayoung-lee : the votes here are so diverse that we might need some rigorous treatment of data collection and decision making. (BTW, if we're really serious about voting, there are 11 voting rules in this book's page 17-22 to consider.)

johnpryan commented 4 years ago

As a user, I would call it the "main thread" if I am building/debugging a Flutter app. If you need something more precise, you could call it the "Dart main thread" to contrast it with the "platform thread" "iOS main thread" or "Android UI thread".

timsneath commented 4 years ago

"root thread" feels plausible as an alternative to main thread, following on from Chinmay's comments. Distinguishes from the platform "main thread", but also has the same connotations.

On Wed, Mar 4, 2020 at 4:04 PM John Ryan notifications@github.com wrote:

As a user, I would call it the "main thread" if I am building/debugging a Flutter app. If you need something more precise, you could call it the "Dart main thread" to contrast it with the "platform thread" "iOS main thread" or "Android UI thread".

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/flutter/flutter/issues/29443?email_source=notifications&email_token=AARWL64TZIFTDIIHNSZDZHTRF3UBFA5CNFSM4G64EE2KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEN3D2TQ#issuecomment-594951502, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARWL67FGUSJCFKONIIFZ5DRF3UBFANCNFSM4G64EE2A .

jayoung-lee commented 4 years ago

Adding @cobblest, who is working on the tooling side UX.

InMatrix commented 4 years ago

Thinking about the context where users will run into those names, I feel the term we pick needs to suggest potential actions to take. One of such contexts is examining the timeline graphs and seeing some red bars. We should help users establish simple associations between red bars on the [insert name] thread and a set of performance diagnosis heuristics they can try. And for the vast majority of our users, the actions they can take are within the limits of their own project rather than in Flutter engine/framework. So I'd favor a name that has concrete meaning to them over a name more accurate to an engine hacker.

As far as voting on the names, I don't think having users vote on those names will work, since that would be quite out of context. We should at least try plugging in those names in some snippets of our existing performance docs and then ask users to rate how well they feel they understand what's being described.

cobblest commented 4 years ago

+1 to what @InMatrix said. This could be a good research question for our next performance profiler study, where we ask users to use devtools to solve a real performance problem. @liyuqian @jacob314 maybe we could work with you to generate a list for the study.

filiph commented 4 years ago

I'm a little sad that this seems less and less likely to be resolved quickly. But that's my problem to deal with. (I will use CPU / GPU thread in the video. We can hopefully add subtitles / annotations later.) I'm genuinely glad we're taking this seriously.

I echo @InMatrix and @cobblest: I'm less concerned about accuracy from the perspective of a Flutter SDK engineer, or even a plugin author, and more concerned about readability and actionability from the perspective of a regular Flutter app developer. For that reason, "main thread" (as in main()) and "raster thread" are still my favorites.

cbracken commented 4 years ago

As noted above I think "main thread" is likely to cause a tonne of confusion -- whenever we use that term we're going to have to clarify if we mean "the thing all iOS developers refer to as the main thread" or "the thing Flutter developers are calling the main thread which is not at all the same as the thread that all iOS developers call the main thread".

Using that term might be okay for developers who've never written code for iOS, but for iOS developers, it's likely to be hugely confusing.

For a bit more context -- I think we want to use these terms consistently throughout our documentation/codelabs. When we get to plugin development or add2app where some of the developer's code (ObjC/Swift bits) are running on what all Apple developer documentation refers to as the "main thread" (what we call the platform thread), I don't think there'd be a clear way to explain how Flutter code is running on our "main thread" (what we currently call the "UI thread", but which is not the iOS "main thread"). This seems like a recipe for confusion.

liyuqian commented 4 years ago

@filiph : it's currently "UI thread + GPU thread" instead of "CPU thread + GPU thread". So if nothing changes, please use "UI thread" instead of "CPU thread" in your video.

liyuqian commented 4 years ago

I believe that changing "GPU thread" to "raster thread" sounds good to most of us. Since that's the main confusion, I suggest we change "GPU thread" to "raster thread" first (probably before @filiph 's performance video), and then figure out how to name "UI thread" later. Please upvote or downvote this suggestion. @cbracken @chinmaygarde @timsneath @InMatrix @jayoung-lee @cobblest @johnpryan @dnfield @jacob314 @kenzieschmoll

filiph commented 4 years ago

This is a multi-step deal. Here's what we currently think needs to be done:

TODO list

Any others? Any volunteers to help Yuqian or Kenzie with the PRs?

Transition period

For one stable version of Flutter, we will need to say

raster thread (previously known as the "GPU thread")

in most documentation. Once "raster thread" has been adopted everywhere in the stable branch, we can start removing the parentheses and therefore all mentions of "raster thread".

kf6gpe commented 4 years ago

@liyuqian, where we with landing the engine changes for this? Looks like thats's blocking the rest of @filiph's work. Thanks!

liyuqian commented 4 years ago

@kf6gpe : all engine commits have landed and I just updated https://github.com/flutter/flutter/issues/29443#issuecomment-602791835 . @filiph : did I miss any work that's blocking you?

filiph commented 4 years ago

@liyuqian Not at all. The framework changes have landed. Unfortunately, I don't think I made it to the dev channel in time.

I'll keep this issue open for now, at least until we see the change in the performance overlay, and are able to actively socialize the renaming (e.g. by a tweet).

liyuqian commented 4 years ago

@filiph : by "until we see the change in the performance overlay", do you mean until https://github.com/flutter/engine/pull/17148 makes into either dev, beta, or stable channel?

filiph commented 4 years ago

Yes, stable.

liyuqian commented 4 years ago

@filiph : I wonder if this is now fully completed?

filiph commented 4 years ago

Ah! Thanks for the nudge.

I can have a look at the outstanding checkboxes (https://github.com/flutter/flutter/issues/29443#issuecomment-602791835) this week. Then I'll close.

filiph commented 4 years ago

Today:

Closing.

github-actions[bot] commented 3 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.