Add benchmarks to measure dart -> native time #28492
Conversation
dnfield
left a comment
dnfield
left a comment
There was a problem hiding this comment.
Lgtm once license issue is resolved. Should just need to add the new files to the golden copy
|
|
||
| void TearDown(const ::benchmark::State& state) {} | ||
|
|
||
| void Wait() { latch_.Wait(); } |
There was a problem hiding this comment.
I am not sure a latch needs to be part of the fixture. Can the benchmark just not create one on its own as needed? That it is an auto reset latch is not immediately obvious from using the fixture. The other setup as part of inheriting from FixturesBase is fine.
| }))); | ||
| AddNativeCallback( | ||
| "NotifyNative", | ||
| CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) { Signal(); }))); |
There was a problem hiding this comment.
This is just removal of unrelated log spam right?
testing/fixtures_base.h
Outdated
|
|
||
| namespace flutter::testing { | ||
|
|
||
| class FixturesBase { |
There was a problem hiding this comment.
Can we call this something like DartFixture or something? Since this fixture seems to be primarily dealing with setting up the VM and isolates in various modes? Perhaps also add a docstring.
| #include "flutter/testing/testing.h" | ||
| #include "flutter/testing/thread_test.h" | ||
|
|
||
| namespace flutter::testing { |
There was a problem hiding this comment.
lol, I am only now realizing you can nest namespaces like this. Pretty cool.
| FML_DISALLOW_COPY_AND_ASSIGN(DartNativeBenchmarks); | ||
| }; | ||
|
|
||
| BENCHMARK_F(DartNativeBenchmarks, DartNativeMessages)(benchmark::State& st) { |
There was a problem hiding this comment.
For new benchmarks in this harness, it may be a good idea to add a docstring that explains what the benchmark is trying to measure as well as a more elaborate name. Right now, "DartNativeMessages" is pretty vague. I expected this case to measure the time it takes to make a call from Dart code to native code repeatedly. Instead, this benchmark measures time to first Dart message from a newly created isolate in a brand new VM.
There was a problem hiding this comment.
So, something like, "TimeToFirstNativeMessageFromIsolateInNewVM". I guess its verbose but gets the job done.
There was a problem hiding this comment.
Done. I also want to measure sending multiple messages from dart to native. I've added another test for that as well.
iskakaushik
added
the
waiting for tree to go green
This reverts commit 3155b00.
|
Potential cause of the tree being red. Investigating. |
This reverts commit 3155b00.
This reverts commit 79818c6.
This reverts commit 2bed429.
4 participants
No description provided.