Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename GPU thread to raster thread #29443

Closed
liyuqian opened this issue Mar 15, 2019 · 45 comments
Closed

Rename GPU thread to raster thread #29443

liyuqian opened this issue Mar 15, 2019 · 45 comments
Assignees
Labels
customer: crowd engine P3 severe: performance team

Comments

@liyuqian
Copy link
Contributor

@liyuqian liyuqian commented Mar 15, 2019

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 liyuqian added team severe: performance labels Mar 15, 2019
@liyuqian
Copy link
Contributor Author

@liyuqian liyuqian commented Mar 15, 2019

@kenzieschmoll
Copy link
Member

@kenzieschmoll kenzieschmoll commented Mar 19, 2019

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
Copy link
Member

@chinmaygarde chinmaygarde commented Mar 19, 2019

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

@InMatrix
Copy link

@InMatrix InMatrix commented Mar 20, 2019

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
Copy link
Contributor Author

@liyuqian liyuqian commented Mar 20, 2019

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
Copy link
Contributor Author

@liyuqian liyuqian commented Mar 20, 2019

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
Copy link
Contributor

@jacob314 jacob314 commented Mar 20, 2019

@dnfield
Copy link
Member

@dnfield dnfield commented Jun 4, 2019

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
Copy link
Contributor Author

@liyuqian liyuqian commented Jun 4, 2019

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
Copy link
Member

@timsneath timsneath commented Jan 3, 2020

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
Copy link
Contributor

@csells csells commented Jan 3, 2020

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

@iapicca iapicca added customer: crowd engine labels Jan 14, 2020
@filiph
Copy link
Contributor

@filiph filiph commented Mar 4, 2020

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. 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)?

@timsneath
Copy link
Member

@timsneath timsneath commented Mar 4, 2020

@liyuqian
Copy link
Contributor Author

@liyuqian liyuqian commented Mar 4, 2020

"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
Copy link
Member

@cbracken cbracken commented Mar 4, 2020

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
Copy link
Member

@dnfield dnfield commented Mar 4, 2020

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
Copy link
Contributor Author

@liyuqian liyuqian commented Mar 4, 2020

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

  • For the current UI thread we have proposed
    • Dart thread
    • Main thread
    • Build thread
  • For the current GPU thread we have proposed
    • Render thread
    • Rasterization thread
    • Raster thread

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

@filiph
Copy link
Contributor

@filiph filiph commented Mar 4, 2020

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:

  • main thread
  • build thread
  • app thread
  • widgets thread
  • prime thread
  • master thread
  • dart thread
  • UI thread

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

@jacob314
Copy link
Contributor

@jacob314 jacob314 commented Mar 4, 2020

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
Copy link
Member

@cbracken cbracken commented Mar 4, 2020

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
Copy link
Member

@chinmaygarde chinmaygarde commented Mar 4, 2020

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
Copy link
Member

@dnfield dnfield commented Mar 4, 2020

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

  • The name is not easily confused with the actual main thread running the application, which is different from the thread running Dart. This means we should not call it "main", "UI" (since Android calls it the UiThread already), "master", "prime". IMO this means we should also not call it the "App" thread.
  • The name makes it clear that this is the thread the engine uses to interact with the Dart VM
  • The name is not specific to a Framework-only concept like Widget, RenderObject, or Build.

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

@chinmaygarde
Copy link
Member

@chinmaygarde chinmaygarde commented Mar 4, 2020

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
Copy link
Member

@cbracken cbracken commented Mar 4, 2020

"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
Copy link
Member

@chinmaygarde chinmaygarde commented Mar 4, 2020

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.

@cobblest
Copy link

@cobblest cobblest commented Mar 5, 2020

+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
Copy link
Contributor

@filiph filiph commented Mar 5, 2020

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
Copy link
Member

@cbracken cbracken commented Mar 5, 2020

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
Copy link
Contributor Author

@liyuqian liyuqian commented Mar 5, 2020

@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
Copy link
Contributor Author

@liyuqian liyuqian commented Mar 5, 2020

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 filiph changed the title Rename GPU thread to render thread? Rename GPU thread to render thread Mar 16, 2020
@filiph filiph changed the title Rename GPU thread to render thread Rename GPU thread to raster thread Mar 16, 2020
@filiph
Copy link
Contributor

@filiph filiph commented Mar 23, 2020

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".

filiph added a commit to filiph/website that referenced this issue Mar 23, 2020
This implements the “transition period” phase of the change proposed in flutter/flutter#29443 (comment).
sfshaza2 pushed a commit to flutter/website that referenced this issue Mar 24, 2020
This implements the “transition period” phase of the change proposed in flutter/flutter#29443 (comment).
filiph added a commit to filiph/flutter that referenced this issue Mar 27, 2020
As per flutter#29443, we are renaming the GPU thread to “raster thread”. The main reason is that most developers assume the GPU thread is running on the GPU, which leads the astray.

DevTools, the engine, and the website are already updated. This PR will complete the necessary code changes. See flutter#29443 (comment) for the whole picture.

I purposefully didn’t touch the tracing event name constant `GPURasterizer::Draw`. Not only does it already convey what it does well even after the GPU->Raster rename, but changing the constant would break folks who depend on that name in their perf benchmarks.

This is purely a documentation change.
@kf6gpe
Copy link
Contributor

@kf6gpe kf6gpe commented Apr 13, 2020

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

@liyuqian
Copy link
Contributor Author

@liyuqian liyuqian commented Apr 13, 2020

@kf6gpe : all engine commits have landed and I just updated #29443 (comment) . @filiph : did I miss any work that's blocking you?

@filiph
Copy link
Contributor

@filiph filiph commented Apr 14, 2020

@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
Copy link
Contributor Author

@liyuqian liyuqian commented Apr 14, 2020

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

@filiph
Copy link
Contributor

@filiph filiph commented Apr 14, 2020

Yes, stable.

@kf6gpe kf6gpe added the P3 label Jun 1, 2020
@liyuqian
Copy link
Contributor Author

@liyuqian liyuqian commented Jul 27, 2020

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

@filiph
Copy link
Contributor

@filiph filiph commented Jul 27, 2020

Ah! Thanks for the nudge.

I can have a look at the outstanding checkboxes (#29443 (comment)) this week. Then I'll close.

@filiph
Copy link
Contributor

@filiph filiph commented Jul 28, 2020

Today:

  • Updated top 2 SERPs of results of internal search for "GPU thread" to use "raster thread". It seems that more or less all the popular internal docs are migrated now (for some of them I didn't have edit access, so I suggested).
  • Went through the 109 current flutter.dev/go design docs and updated the ones that used "GPU thread".
  • Asking for a tweet to be published this or next week.

Closing.

@filiph filiph closed this as completed Jul 28, 2020
@github-actions
Copy link

@github-actions github-actions bot commented Aug 18, 2021

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
customer: crowd engine P3 severe: performance team
Projects
None yet
Development

No branches or pull requests

16 participants