[web] Profile text layout and forward data to macrobenchmarks #17276
Conversation
| /// | ||
| /// To use the [Profiler]: | ||
| /// | ||
| /// 1. Enable the `FLUTTER_WEB_ENABLE_PROFILING` flag. |
There was a problem hiding this comment.
nit: technically it's not a flag, but an environment variable that can be set to "true".
| /// | ||
| /// 1. Enable the `FLUTTER_WEB_ENABLE_PROFILING` flag. | ||
| /// | ||
| /// 2. Using js interop, assign a listener function to |
| if (Profiler._instance == null) { | ||
| Profiler._instance = Profiler._(); | ||
| } | ||
| return Profiler._instance; |
There was a problem hiding this comment.
The above 4 lines can be reduced to return Profiler._instance ??= Profiler._()
|
|
||
| /// Used to send benchmark data to whoever is listening to them. | ||
| void benchmark(String name, num value) { | ||
| if (!isBenchmarkMode) { |
There was a problem hiding this comment.
Move into a shared function?
There was a problem hiding this comment.
I thought about doing this initially, but decided it may not tree-shake well when I move the throw into a function. But now that tree-shaking is out of the picture anyway, I think it's reasonable to put this logic in a shared function.
| _measurementResult = _measurementService.measure(this, constraints); | ||
| if (Profiler.isBenchmarkMode) { | ||
| Profiler.instance.benchmark('text_layout', stopwatch.elapsedMicroseconds); | ||
| stopwatch.stop(); |
There was a problem hiding this comment.
For better accuracy I think you want to stop before sending the number.
There was a problem hiding this comment.
I think stopwatch.elapsedMicroseconds is being read before calling Profiler.instance.benchmark(). Nevertheless, I'll move the stop to be above :)
There was a problem hiding this comment.
Ah, you're right. I think you can just not call stop at all, although somehow seeing the call to stop gives me more confidence about what's being measured :)
| if (!isBenchmarkMode) { | ||
| throw Exception( | ||
| 'Cannot use Profiler unless benchmark mode is enabled. ' | ||
| 'You can enable it by using the FLUTTER_WEB_ENABLE_PROFILING flag.', |
There was a problem hiding this comment.
In this message I would also refer to it as "environment variable" ("flag" reads too much like a command-line option)
|
Still needs dartfmt and see canvaskit failure. |
|
Mac and Fuchsia failures are unrelated. |
…flutter#17276)" This reverts commit f0caace.
This will allow web benchmarks to capture granular performance metrics. For example, one benchmark could run the gallery app, then receive and accumulate all the "text_layout" numbers.