[Impeller] GPU Tracer for GLES. #47080

jonahwilliams · 2023-10-18T23:09:58Z

Trace GPU execution time on GLES using GL_EXT_disjoint_timer_query. This requires a per-app opt in from the Android Manifest with the key "io.flutter.embedding.android.EnableOpenGLGPUTracing set to true.

jason-simmons · 2023-10-19T21:47:50Z

impeller/renderer/backend/gles/gpu_tracer_gles.cc

+  has_started_frame_ = true;
+
+  FML_DCHECK(!active_frame_.has_value());
+  FML_DCHECK(query != 0);


this is redundant with the query == 0 check above

jason-simmons · 2023-10-19T21:50:43Z

impeller/renderer/backend/gles/gpu_tracer_gles.h

+  std::deque<uint32_t> pending_traces_;
+  std::optional<uint32_t> active_frame_ = std::nullopt;
+  std::thread::id raster_thread_;
+  bool has_started_frame_ = false;


has_started_frame_ can be removed. The code can instead check whether active_frame_ contains a value.

jason-simmons · 2023-10-19T21:54:07Z

impeller/renderer/backend/gles/gpu_tracer_gles.cc

+  }
+
+  // For reasons unknown to me, querying the state of more than
+  // on query object per frame causes crashes on a Pixel 6 pro.


typo: "on" -> "one"

jason-simmons · 2023-10-19T21:56:58Z

impeller/renderer/backend/gles/gpu_tracer_gles.cc

+  // For reasons unknown to me, querying the state of more than
+  // on query object per frame causes crashes on a Pixel 6 pro.
+  // It does not crash on an S10.
+  auto latest_query = pending_traces_.front();


I think latest_query implies that this is the most recent query, when it's actually the least recent.

I'd rename this to query

jason-simmons · 2023-10-19T22:22:51Z

impeller/renderer/backend/gles/gpu_tracer_gles.cc

+
+  // For reasons unknown to me, querying the state of more than
+  // on query object per frame causes crashes on a Pixel 6 pro.
+  // It does not crash on an S10.


Where exactly is the crash happening?

If the tracer can only consume one pending query per MarkFrameStart call, then that means the pending traces queue will grow unbounded each time a query result is unavailable at the next MarkFrameStart.

jonahwilliams · 2023-10-20T01:28:51Z

This actually works OK on the Pixel 7s we have in the device lab. On a Pixel 6 pro the crash is almost immediate and appears to be in the driver:

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0000000000000000
Cause: null pointer dereference
    x0  b40000797364acd0  x1  0000000000000000  x2  0000000000000000  x3  0000007999a60d98
    x4  0000000000000003  x5  0000000007ee634c  x6  71647164636d6471  x7  7f7f7f7f7f7f7f7f
    x8  0000000000000000  x9  0000000000000004  x10 0000000000000001  x11 0000000000000000
    x12 0000000000000008  x13 b400007b275cd128  x14 0000000000000007  x15 0000000000000028
    x16 0000000000000007  x17 0000000000000007  x18 000000798b636000  x19 b40000797364acd0
    x20 0000000000000000  x21 b4000079736b07b0  x22 b4000079735b4660  x23 b40000797360fef0
    x24 0000007999a65000  x25 000000799b889e6e  x26 0000007999a60d90  x27 0000000000008866
    x28 0000007999a60d98  x29 0000000000000001
    lr  00000079ece308a0  sp  0000007999a60a20  pc  00000079ece308ac  pst 0000000060001000
backtrace:
      #00 pc 000000000082d8ac  /vendor/lib64/egl/libGLES_mali.so (gles_fb_flush_compute_nx+92) (BuildId: 045075399765d011)
      #01 pc 00000000007e30ac  /vendor/lib64/egl/libGLES_mali.so (gles2_queryp_flush+108) (BuildId: 045075399765d011)
      #02 pc 00000000007e4050  /vendor/lib64/egl/libGLES_mali.so (gles_queryp_object_flush_for_result+64) (BuildId: 045075399765d011)
      #03 pc 00000000007e2760  /vendor/lib64/egl/libGLES_mali.so (gles2_queryp_get_query_object_getv+464) (BuildId: 045075399765d011)
      #04 pc 0000000002074c44  /data/app/~~KIK6odR8TVi-SXQaZ2msSQ==/com.gskinner.flutter.wonders-RurBsIIIDsouyhOiGFYG-Q==/base.apk!libflutter.so (offset 0xff23000) (auto impeller::GLProc<void (unsigned int, unsigned int, int*)>::operator()<unsigned int&, int, int*>(unsigned int&, int&&, int*&&) const+152) (BuildId: 46301208b8388191c9b7feb94f655a031d7f1eb7)
      #05 pc 000000000207803c  /data/app/~~KIK6odR8TVi-SXQaZ2msSQ==/com.gskinner.flutter.wonders-RurBsIIIDsouyhOiGFYG-Q==/ba

Based on some other reports, this seems like a fiddly API (https://bugs.chromium.org/p/chromium/issues/detail?id=453965) . I'm going to make usage opt in so we can use it on our benchmarks when necessary without worrying about broader device compatibility.

impeller/renderer/backend/gles/gpu_tracer_gles.cc

jason-simmons · 2023-10-20T16:39:55Z

impeller/renderer/backend/gles/gpu_tracer_gles.cc

+    return;
+  }
+
+  FML_DCHECK(!active_frame_.has_value());


This duplicates the active_frame_.has_value() check above

jason-simmons · 2023-10-20T16:41:24Z

impeller/renderer/backend/gles/gpu_tracer_gles.cc

+
+void GPUTracerGLES::MarkFrameEnd(const ProcTableGLES& gl) {
+  if (!enabled_ || std::this_thread::get_id() != raster_thread_ ||
+      !active_frame_.has_value() || !active_frame_.has_value()) {


Remove the second copy of !active_frame_.has_value()

Trace GPU execution time on GLES using GL_EXT_disjoint_timer_query. This requires a per-app opt in from the Android Manifest with the key `"io.flutter.embedding.android.EnableOpenGLGPUTracing` set to true.

Fixes breakages from: * flutter/engine#47080 * flutter/engine#47247 * flutter/engine#47278

bdero/impeller-cmake#20 Fixes breakages from: * #47080 * #47247 * #47278

jonahwilliams added 2 commits October 18, 2023 15:56

++

eb8b915

++

c46d3eb

github-actions bot added the e: impeller label Oct 18, 2023

jonahwilliams added 3 commits October 18, 2023 21:14

++

6034da3

++

c014eba

++

bf6f77b

jonahwilliams mentioned this pull request Oct 19, 2023

[Impeller] Implement GPU frame time tracing where feasible. flutter/flutter#136493

Closed

add 'test'

3eb2edd

jonahwilliams marked this pull request as ready for review October 19, 2023 17:31

licenses and cleanup.

12f86c3

jonahwilliams requested review from dnfield and jason-simmons October 19, 2023 17:51

chinmaygarde self-requested a review October 19, 2023 20:17

chinmaygarde assigned jonahwilliams Oct 19, 2023

chinmaygarde changed the title ~~[Impeller] GPU Tracer for GLES~~ [Impeller] GPU Tracer for GLES. Oct 19, 2023

jason-simmons reviewed Oct 19, 2023

View reviewed changes

Add option to enable GPU tracing for OpenGL from Android Manifest

51ad782

jonahwilliams commented Oct 20, 2023

View reviewed changes

impeller/renderer/backend/gles/gpu_tracer_gles.cc Outdated Show resolved Hide resolved

++

cd0c46d

github-actions bot added the platform-android label Oct 20, 2023

jonahwilliams requested a review from jason-simmons October 20, 2023 15:21

jason-simmons reviewed Oct 20, 2023

View reviewed changes

++

6a93525

jason-simmons approved these changes Oct 20, 2023

View reviewed changes

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 20, 2023

auto-submit bot merged commit 29168dd into flutter:main Oct 20, 2023

jonahwilliams deleted the gles_tracer branch October 20, 2023 17:52

engine-flutter-autoroll mentioned this pull request Oct 20, 2023

Roll Flutter Engine from d46933e6772e to b27e1b38375b (9 revisions) flutter/flutter#136982

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 20, 2023

29168dd75 [Impeller] GPU Tracer for GLES. (flutter/engine#47080)

f42405d

bdero added a commit to bdero/impeller-cmake that referenced this pull request Nov 1, 2023

Bump Flutter Engine to head.

2678b56

Fixes breakages from: * flutter/engine#47080 * flutter/engine#47247 * flutter/engine#47278

bdero mentioned this pull request Nov 1, 2023

Bump Flutter Engine to head. bdero/impeller-cmake#20

Merged

bdero added a commit to bdero/impeller-cmake that referenced this pull request Nov 1, 2023

Bump Flutter Engine to head.

4ea085d

Fixes breakages from: * flutter/engine#47080 * flutter/engine#47247 * flutter/engine#47278

bdero added a commit to bdero/impeller-cmake that referenced this pull request Nov 1, 2023

Bump Flutter Engine to head.

ffb5582

Fixes breakages from: * flutter/engine#47080 * flutter/engine#47247 * flutter/engine#47278

bdero mentioned this pull request Nov 1, 2023

Bump impeller-cmake to head #47563

Merged

bdero added a commit that referenced this pull request Nov 1, 2023

Bump impeller-cmake to head (#47563)

c896f5f

bdero/impeller-cmake#20 Fixes breakages from: * #47080 * #47247 * #47278

[Impeller] GPU Tracer for GLES. #47080

[Impeller] GPU Tracer for GLES. #47080

Uh oh!

Conversation

jonahwilliams commented Oct 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Oct 20, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jonahwilliams commented Oct 18, 2023 •

edited

Loading