ProfileInstallerTranscodeBenchmark increased for unknown reason [261998144]

Verified

Bug

Status Update

No update yet.

Description

mo...@google.com

created issue #1

Dec 9, 2022 05:50PM

Alert on dashboard: https://androidx-perf.skia.org/t/?begin=1670529681&end=1670529682&subset=all

CLs in build: https://android-build.googleplex.com/builds/branch-dashboard/aosp-androidx-main?build_id=9385668

The commit points to a CL that has no real change (just reformatting), so something else is going on.

Sean, can you look and see why this benchmark is going crazy?

Comments

ph...@gmail.com <ph...@gmail.com> #2Dec 9, 2022 06:32PM

> The Transition Kotlin extensions target the Framework Transition class which is a bit confusing

All extensions in 'core-ktx' target either 'core' or the framework types. This is working as expected. It sounds like this is a feature request for a transition-ktx with equivalent extensions for the androidx.transition types.

> as most framework classes are being deprecated in favor of their AndroidX counterparts, Fragments for example.

"Most"? This *dramatically* overstates what is actually happening. Fragments (and loaders) were added to the framework in a time where the Android team did not have external libraries for shipping features. Transition was added 5 years later.

ph...@gmail.com <ph...@gmail.com> #3Dec 9, 2022 06:34PM

Pardon my misunderstanding, I was under the belief that where possible, new features that could be developed outside the framework, would be under Androidx. Also it's a common refrain to use the Androidx libraries over the framework libraries where possible, as those ship update independently of platform releases. I wasn't attempting to be dramatic.

If the Transition api is unlikely to change, there is no need for a transition-ktx for androidx.transition types. If developers should use Androidx transitions over framework transitions where possible however, then it would be much appreciated.

I am well aware of the limitations that existed when Fragments and Loaders were created, and when the Transition api was introduced.

se...@google.com <se...@google.com> #4Dec 9, 2022 07:30PM

Reassigned to ra...@google.com.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit ae42e311ecfbac18cedc90e489d1fdde956a4557
Author: Jake Wharton <jakew@google.com>
Date: Fri Nov 22 11:13:30 2019

Introduce transition-ktx for standalone transition library

These extensions are the same as androidx.core.transition for the platform types, just adapted to the standalone library.

Bug: 138870873
Test: gw :transition:transition-ktx:build :transition:transition-ktx:connectedCheck
Change-Id: Ie80022cc60ed1f482b7593db5ef1ac1c5ea33d8d

M buildSrc/src/main/kotlin/androidx/build/PublishDocsRules.kt
M jetifier/jetifier/migration.config
M settings.gradle
A transition/transition-ktx/OWNERS
A transition/transition-ktx/api/1.4.0-alpha01.txt
A transition/transition-ktx/api/current.txt
A transition/transition-ktx/api/public_plus_experimental_1.4.0-alpha01.txt
A transition/transition-ktx/api/public_plus_experimental_current.txt
A transition/transition-ktx/api/res-1.4.0-alpha01.txt
A transition/transition-ktx/api/restricted_1.4.0-alpha01.txt
A transition/transition-ktx/api/restricted_current.txt
A transition/transition-ktx/build.gradle
A transition/transition-ktx/src/androidTest/AndroidManifest.xml
A transition/transition-ktx/src/androidTest/java/androidx/transition/TestActivity.kt
A transition/transition-ktx/src/androidTest/java/androidx/transition/TransitionTest.kt
A transition/transition-ktx/src/androidTest/res/layout/test_activity.xml
A transition/transition-ktx/src/main/AndroidManifest.xml
A transition/transition-ktx/src/main/java/androidx/transition/Transition.kt

https://android-review.googlesource.com/1172715

ca...@gmail.com <ca...@gmail.com> #5Dec 10, 2022 02:16PM

Hello!! be warned, most of these so called hackers here are impostors, I know how real hackers work, they never advertise themselves in such a credulous manner and they are always discrete. I’ve been ripped off so many times out of desperation trying to find urgent help until my friend finally introduced me to a reliable hacker who’s services are cheap and affordable, discretion and delivers, he does all sorts of hacks but he helped me Hacked my cheating boyfriend email/facebook, whatsapp, instagram, with snapchat, I have made him my permanent hacker and you can as well enjoy his services. You can contact him at: (TECHCROWNHACKER @ GMAIL COM) and after his work also endeavor to spread the good news on his work and how he helped you.

ra...@google.com <ra...@google.com> #6Dec 10, 2022 04:25PM

Reassigned to ma...@google.com.

Hi Marcelo, can you please take a look at this when you get a chance ? If not, I can come and take a look when I get back.

ma...@google.com <ma...@google.com> #7Dec 12, 2022 08:26PM

I'm pretty sure the offending PR here is aosp/2333568 - that is the only PR touching the profile transcoding.

It fixes an important bug with compressed profiles but it seems that the new code is slower than before.

Message last modified on Dec 12, 2022 08:27PM

cc...@google.com <cc...@google.com> #8Dec 12, 2022 09:10PM

If you look in the dashboard, the commit link is wrong, since it doesn't encompass all builds since the previous (9385400). Correct build range link is here, which includes Ben's CL: https://android-build.googleplex.com/builds/9385668/branches/aosp-androidx-main/targets/_/cls?end=9385400&inclusiveStart=1&inclusiveEnd=0

(If a build goes missing for any reason, it corrupts the commit link. In general, you can always go to dashboard, and create range link like the above)

Message last modified on Dec 12, 2022 09:11PM

be...@google.com <be...@google.com> #9Dec 13, 2022 02:56PM

Yes, it looks like my change introduced this regression. If I read this dashboard correctly, before the change it took a minimum of 0.0012 ms to install a profile and now it takes ~ 1.3 ms.
That would mean the change makes opening the file slower by about 1000 times.

Chris: Given this runs on a background thread and is not blocking app startup, is it urgent to address this or can it wait for the next release cycle?

cc...@google.com <cc...@google.com> #10Dec 13, 2022 06:10PM

The number in the triage page is synthetic, you need to click through to the dashboard here to see the real nanos. It went from 0.2ms to 46ms, which is a huge slowdown. We should fix this before cherry-picking / releasing this change.

be...@google.com <be...@google.com> #11Dec 14, 2022 04:20PM

Accepted by be...@google.com.

Uploaded aosp/2355662 to address the problem.

be...@google.com <be...@google.com> #12Jan 18, 2023 06:09PM

Status: Assigned (reopened)

The CL mentioned in comment#11 will land shortly and should fix this issue.

mo...@google.com <mo...@google.com> Jan 18, 2023 06:51PM

Reassigned to be...@google.com.

ap...@google.com <ap...@google.com> #13Jan 20, 2023 06:22PM

Project: platform/frameworks/support
Branch: androidx-main

commit b7fc1b0247a4f4883b37c1998e3f12b4f0c0b7b0
Author: Ben Weiss <benweiss@google.com>
Date: Wed Dec 14 15:49:17 2022

More detailed compressed profile handling

Just replacing openFd with open dramatically decreased the performance
of opening a profile. This new handling goes back to openFd and
introduces a diagnostic code to hint at updating bundletool.

Bug: 261998144
Test: :profileinstaller:profileinstaller:test
RelNote: Add diagnostics for compressed profiles

Change-Id: I8641387ad6073dde588d0d764da4a3b24b1c8ee4

M profileinstaller/profileinstaller/api/current.txt
M profileinstaller/profileinstaller/api/public_plus_experimental_current.txt
M profileinstaller/profileinstaller/api/restricted_current.txt
M profileinstaller/profileinstaller/src/main/java/androidx/profileinstaller/DeviceProfileWriter.java
M profileinstaller/profileinstaller/src/main/java/androidx/profileinstaller/ProfileInstaller.java

https://android-review.googlesource.com/2355662

be...@google.com <be...@google.com> #14Jan 23, 2023 12:34PM

The landed CL has moved the benchmark down to normal levels again.

be...@google.com <be...@google.com> #15Jan 23, 2023 12:35PM

Verified by be...@google.com.

See dashboard

pr...@google.com <pr...@google.com> #16Feb 8, 2023 06:21PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.profileinstaller:profileinstaller:1.3.0-beta01