Regression in diff-min test flutter_gallery__memory_nav

[kf6gpe]

See https://flutter-flutter-perf.skia.org/e/?begin=1603897001&end=1604032912&keys=Xb89e35c986a650a0bf9142de23b3f1bb&requestType=0&xbaroffset=22270

Looks like commit #69264, relanding the staging of web tests.

cc @jonahwilliams, author of the reland commit.

[kf6gpe]

Or could be #69307, maybe? This is a test change.

[jonahwilliams]

It was definitely #69307 - I think my question is how would that negatively impact memory usage? Since we don't spin up an additional isolate, I would think memory usage would be lower generally - unless of course we're measuring wrong.

FYI @aam

[jonahwilliams]

This test is using adb to measure memory so its not caused by something like only looking at one isolate's memory

[jonahwilliams]

Looking at https://github.com/flutter/flutter/blame/bb495ba90c63ba026450d37175d5c279c3b8a632/dev/devicelab/lib/tasks/perf_tests.dart#L1220

It looks this is the difference between memory at the end and memory at the start. If we use less memory at the start (due to not spinning up an isolate) then this would increase.

This seems to be confirmed by https://flutter-flutter-perf.skia.org/e/?queries=sub_result%3Dstart-median%26sub_result%3Dstart-min%26test%3Dflutter_gallery__memory_nav

[jonahwilliams]

This is actually a positive change

[jonahwilliams]

Leaving open for future discussion

[jonahwilliams]

FYI @liyuqian , this benchmark can be pretty confusing if we decrease start up memory. Though might be worth talking about as a perf improvement, if I'm reading this correctly we're using 10 MB less memory at startup

[liyuqian]

Agree it's confusing as the metric isn't monotone when the performance improves. We've had similar situations before with missed frames metrics which triggered us to replace them with 90th, 99th percentiles (#18727).

I'm not sure what's a good replacement though. One question is: if we reduce 10 MB memory at startup, why didn't we also reduce 10 MB memory in the end? Is it that we just defer the memory allocation to a later point?

I believe the original intention of measuring the startup memory is to measure the non-Flutter memory (those overhead memory by unrelated Android/iOS stuff) and make our metrics immune to those OS changes. If the startup memory actually includes something that Flutter allocates, maybe we should measure:

1. baseline_memory: memory use before Flutter starts to allocate anything.
2. startup memory: memory use when the app fully starts as we measured now.
3. end_memory: memory use when the app test run ends as we measured now.

We then calculate start_diff = start_memory - baseline_memory and end_diff = end_memory - baseline_memory, and only report regressions on start_diff and end_diff? CC @dnfield for more thoughts.

[jonahwilliams]

I'm not sure what's a good replacement though. One question is: if we reduce 10 MB memory at startup, why didn't we also reduce 10 MB memory in the end? Is it that we just defer the memory allocation to a later point?

Because it was a temporary memory allocation from the isolate used to parse the asset manifest

[liyuqian]

I'm not sure what's a good replacement though. One question is: if we reduce 10 MB memory at startup, why didn't we also reduce 10 MB memory in the end? Is it that we just defer the memory allocation to a later point?

Because it was a temporary memory allocation from the isolate used to parse the asset manifest

If it's temp, shouldn't it be GCed by the end of the test run?

[jonahwilliams]

It was: The end memory stayed the same, the start memory decreased. So the diff did increase, but none of the component measurements got worse

[liyuqian]

I see. So previously the 10MB gets GCed (a -10MB allocation) between start and end, while now that 10MB does not exist anymore. If there's a baseline_memory that can measure the memory before such temp memory can ever be allocated by Flutter, we could avoid this kind of confusion.