Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1221433002:
cc: narrow ChromeOS perf workaround to ARM (Closed)
Descriptioncc: narrow ChromeOS perf workaround to ARM
GL_COMMANDS_COMPLETED_CHROMIUM makes EGL_SYNC_PRIOR_COMMANDS_IMPLICIT_EXTERNAL_ARM
fence slow down significantly. It's ARM GPU driver issue,
which chromium cannot workaround it.
BUG=522903, 580166
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
Committed: https://crrev.com/b482d9409f829c4c5c3c5030d251399b97cecea8
Cr-Commit-Position: refs/heads/master@{#370935}
Patch Set 1 #Patch Set 2 : NotForReview: nasty baseline patch #Patch Set 3 : rebase to ToT #Patch Set 4 : narrow ChromeOS perf workaround to ARM #Messages
Total messages: 24 (6 generated)
dongseong.hwang@intel.com changed reviewers: + reveman@chromium.org
reveman, could you review? I extract it from https://codereview.chromium.org/1134993003/ as you requested.
On 2015/06/26 15:21:21, dshwang wrote: > reveman, could you review? I extract it from > https://codereview.chromium.org/1134993003/ as you requested. hi reveman, could you review?
On 2015/07/07 08:59:10, dshwang wrote: > On 2015/06/26 15:21:21, dshwang wrote: > > reveman, could you review? I extract it from > > https://codereview.chromium.org/1134993003/ as you requested. > > hi reveman, could you review? reveman, could you review?
We might need some changes to the polling interval on the service side to avoid a performance regression. Have you tried this on an ARM device?
On 2015/08/05 14:03:09, reveman wrote: > We might need some changes to the polling interval on the service side to avoid > a performance regression. Have you tried this on an ARM device? no, I don't have any ARM chrome book. It may take time that I get ARM chrome book. you can take over this CL.
> I'd rather not make this nastier than it already is. Let's try to fix the > polling interval on the service side so this problem goes away. All platforms > will benefit from that. On Acer C720 (Intel Haswell), I didn't see any perf difference using http://webglsamples.org/aquarium/aquarium.html What's mean "fix the polling interval" currently, GpuCommandBufferStub::PollWork() kick every 1ms if there are pending query. Do you think we need to kick more frequently?
On 2015/08/05 at 17:02:29, dongseong.hwang wrote: > > I'd rather not make this nastier than it already is. Let's try to fix the > > polling interval on the service side so this problem goes away. All platforms > > will benefit from that. > > On Acer C720 (Intel Haswell), I didn't see any perf difference using http://webglsamples.org/aquarium/aquarium.html > > What's mean "fix the polling interval" > currently, GpuCommandBufferStub::PollWork() kick every 1ms if there are pending query. Do you think we need to kick more frequently? I think it's the context switches that are the biggest problem. Right now we schedule a call to PollWork independent of when was the last time we processed pending queries and that can result in a lot of unnecessary context switches. Simply making sure to not call MakeCurrent and process pending work more than every 1ms might be enough.
On 2015/08/05 17:57:45, reveman wrote: > On 2015/08/05 at 17:02:29, dongseong.hwang wrote: > > > I'd rather not make this nastier than it already is. Let's try to fix the > > > polling interval on the service side so this problem goes away. All > platforms > > > will benefit from that. > > > > On Acer C720 (Intel Haswell), I didn't see any perf difference using > http://webglsamples.org/aquarium/aquarium.html > > > > What's mean "fix the polling interval" > > currently, GpuCommandBufferStub::PollWork() kick every 1ms if there are > pending query. Do you think we need to kick more frequently? > > I think it's the context switches that are the biggest problem. Right now we > schedule a call to PollWork independent of when was the last time we processed > pending queries and that can result in a lot of unnecessary context switches. > Simply making sure to not call MakeCurrent and process pending work more than > every 1ms might be enough. I filed https://code.google.com/p/chromium/issues/detail?id=522903 I checked Patch Set 2 regress performance in daisy_spring. We need to fix it to enable native GMB for ChromeOS. I'm debugging gpu scheduler but I'm new at it so maybe it takes time.
On 2015/08/20 at 11:35:10, dongseong.hwang wrote: > On 2015/08/05 17:57:45, reveman wrote: > > On 2015/08/05 at 17:02:29, dongseong.hwang wrote: > > > > I'd rather not make this nastier than it already is. Let's try to fix the > > > > polling interval on the service side so this problem goes away. All > > platforms > > > > will benefit from that. > > > > > > On Acer C720 (Intel Haswell), I didn't see any perf difference using > > http://webglsamples.org/aquarium/aquarium.html > > > > > > What's mean "fix the polling interval" > > > currently, GpuCommandBufferStub::PollWork() kick every 1ms if there are > > pending query. Do you think we need to kick more frequently? > > > > I think it's the context switches that are the biggest problem. Right now we > > schedule a call to PollWork independent of when was the last time we processed > > pending queries and that can result in a lot of unnecessary context switches. > > Simply making sure to not call MakeCurrent and process pending work more than > > every 1ms might be enough. > > I filed https://code.google.com/p/chromium/issues/detail?id=522903 > I checked Patch Set 2 regress performance in daisy_spring. > We need to fix it to enable native GMB for ChromeOS. > I'm debugging gpu scheduler but I'm new at it so maybe it takes time. https://codereview.chromium.org/1315713007 is supposed to avoid some context switches just to check queries. I think we can give this patch a try after that has landed.
On 2015/08/28 15:19:36, reveman wrote: > https://codereview.chromium.org/1315713007 is supposed to avoid some context > switches just to check queries. I think we can give this patch a try after that > has landed. Thx for the effort. Let's resolve the root issue together :)
On 2015/09/01 12:31:47, dshwang wrote: > On 2015/08/28 15:19:36, reveman wrote: > > https://codereview.chromium.org/1315713007 is supposed to avoid some context > > switches just to check queries. I think we can give this patch a try after > that > > has landed. > > Thx for the effort. Let's resolve the root issue together :) Looks like that landed, time to try this then?
On 2015/11/04 19:11:07, danakj wrote: > On 2015/09/01 12:31:47, dshwang wrote: > > On 2015/08/28 15:19:36, reveman wrote: > > > https://codereview.chromium.org/1315713007 is supposed to avoid some context > > > switches just to check queries. I think we can give this patch a try after > > that > > > has landed. > > > > Thx for the effort. Let's resolve the root issue together :) > > Looks like that landed, time to try this then? Even if it was landed, this issue is not resolved as I report current status in BUG=522903
Description was changed from ========== cc: remove ChromeOS perf workaround. We should just remove this. If there are still performance issues with using fences then we need to solve them. BUG=522903 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== cc: narrow ChromeOS perf workaround to ARM GL_COMMANDS_COMPLETED_CHROMIUM makes EGL_SYNC_PRIOR_COMMANDS_IMPLICIT_EXTERNAL_ARM fence slow down significantly. It's ARM GPU driver issue, which chromium cannot workaround it. BUG=522903, 580166 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
reveman, could you review it again? I explain what is the root cause in issue 580166. It's due to ARM driver bug, and no way to workaround. Currently, native GpuMemoryBuffer is supported for Intel ChromeOS, so this CL is fine. ARM needs to fix the fence issue to enable native GpuMemoryBuffer.
Patchset #4 (id:60001) has been deleted
lgtm
On 2016/01/21 21:28:36, reveman wrote: > lgtm thank you for reviewing! after this land, native zero/one-copy is ready to enable
The CQ bit was checked by dongseong.hwang@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1221433002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1221433002/80001
Message was sent while issue was closed.
Description was changed from ========== cc: narrow ChromeOS perf workaround to ARM GL_COMMANDS_COMPLETED_CHROMIUM makes EGL_SYNC_PRIOR_COMMANDS_IMPLICIT_EXTERNAL_ARM fence slow down significantly. It's ARM GPU driver issue, which chromium cannot workaround it. BUG=522903, 580166 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== cc: narrow ChromeOS perf workaround to ARM GL_COMMANDS_COMPLETED_CHROMIUM makes EGL_SYNC_PRIOR_COMMANDS_IMPLICIT_EXTERNAL_ARM fence slow down significantly. It's ARM GPU driver issue, which chromium cannot workaround it. BUG=522903, 580166 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
Message was sent while issue was closed.
Committed patchset #4 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== cc: narrow ChromeOS perf workaround to ARM GL_COMMANDS_COMPLETED_CHROMIUM makes EGL_SYNC_PRIOR_COMMANDS_IMPLICIT_EXTERNAL_ARM fence slow down significantly. It's ARM GPU driver issue, which chromium cannot workaround it. BUG=522903, 580166 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== cc: narrow ChromeOS perf workaround to ARM GL_COMMANDS_COMPLETED_CHROMIUM makes EGL_SYNC_PRIOR_COMMANDS_IMPLICIT_EXTERNAL_ARM fence slow down significantly. It's ARM GPU driver issue, which chromium cannot workaround it. BUG=522903, 580166 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/b482d9409f829c4c5c3c5030d251399b97cecea8 Cr-Commit-Position: refs/heads/master@{#370935} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/b482d9409f829c4c5c3c5030d251399b97cecea8 Cr-Commit-Position: refs/heads/master@{#370935} |
