Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c5718eaf authored by Mark White's avatar Mark White
Browse files

Resolve compat perf regression via @NoLogging annotation 

This annotation allows marking specific compat changes so that they are not logged by the compat framework. This is expected to be used in code that is called in performance critical paths.

Note this change also means that non-loggable changes (including targeting older sdk) are no longer going to stats log.

Change-Id: I3df7d0a3575534fc5be7911624237b57c4deb8f7
Flag: EXEMPT bugfix
Test: PlatformCompatFrameworkTests CtsAppCompatHostTestCases FrameworksServicesTests_android_server_compat
Bug: 424051595
parent 7af0bfa6

core/java/android/app/AppCompatCallbacks.java

+10 −5
Original line number Diff line number Diff line
@@ -86,24 +86,29 @@ public final class AppCompatCallbacks implements Compatibility.BehaviorChangeDel 
    }



    public boolean isChangeEnabled(long changeId) {

        boolean isLoggable = mLogChangeChecksToStatsD ||

            changeIdInChangeList(mLoggableChanges, changeId);

        if (!isLoggable) {

            boolean isEnabled = !changeIdInChangeList(mDisabledChanges, changeId);

            return isEnabled;

        }

        return isChangeEnabledAndReport(changeId, mLogChangeChecksToStatsD);

    }



    private boolean isChangeEnabledAndReport(long changeId, boolean doStatsLog) {

        boolean isEnabled = !changeIdInChangeList(mDisabledChanges, changeId);

        boolean isLoggable = changeIdInChangeList(mLoggableChanges, changeId);

        if (isEnabled) {

            // Not present in the disabled changeId array

            reportChange(changeId, ChangeReporter.STATE_ENABLED, isLoggable, doStatsLog);

            reportChange(changeId, ChangeReporter.STATE_ENABLED, doStatsLog);

            return true;

        }

        reportChange(changeId, ChangeReporter.STATE_DISABLED, isLoggable, doStatsLog);

        reportChange(changeId, ChangeReporter.STATE_DISABLED, doStatsLog);

        return false;

    }



    private void reportChange(long changeId, int state, boolean isLoggable, boolean doStatsLog) {

    private void reportChange(long changeId, int state, boolean doStatsLog) {

        int uid = Process.myUid();

        mChangeReporter.reportChange(uid, changeId, state, false, isLoggable, doStatsLog);

        mChangeReporter.reportChange(uid, changeId, state, false, doStatsLog);

    }



}

core/java/android/widget/TextView.java

+4 −0
Original line number Diff line number Diff line
@@ -60,6 +60,7 @@ import android.app.assist.AssistStructure; 
import android.app.compat.CompatChanges;

import android.compat.annotation.ChangeId;

import android.compat.annotation.EnabledSince;

import android.compat.annotation.NoLogging;

import android.compat.annotation.UnsupportedAppUsage;

import android.content.ClipData;

import android.content.ClipDescription;
@@ -537,6 +538,7 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener 
     * @hide

     */

    @ChangeId

    @NoLogging

    @EnabledSince(targetSdkVersion = Build.VERSION_CODES.TIRAMISU)

    public static final long BORINGLAYOUT_FALLBACK_LINESPACING = 210923482L; // buganizer id









@@ -545,6 +547,7 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener













     * @hide















     */















    @ChangeId













    @NoLogging















    @EnabledSince(targetSdkVersion = Build.VERSION_CODES.P)















    public static final long STATICLAYOUT_FALLBACK_LINESPACING = 37756858; // buganizer id























@@ -554,6 +557,7 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener













     * @hide















     */















    @ChangeId













    @NoLogging















    @EnabledSince(targetSdkVersion = VERSION_CODES.VANILLA_ICE_CREAM)















    public static final long USE_BOUNDS_FOR_WIDTH = 63938206;  // buganizer id















































core/java/com/android/internal/compat/ChangeReporter.java



+10
−34




























Original line number
Diff line number
Diff line








@@ -94,20 +94,17 @@ public class ChangeReporter {













     * @param state            of the reported change - enabled/disabled/only logged















     * @param isKnownSystemApp do we know that the affected app is a system app? (if true is















     *                         definitely a system app, if false it may or may not be a system app)













     * @param isLoggableBySdk  whether debug logging is allowed for this change based on target













     *                         SDK version. This is combined with other logic to determine whether













     *                         to actually log. If the sdk version does not matter, should be true.















     * @param doStatsLog       whether to log to stats log.















     */















    public void reportChange(int uid, long changeId, int state, boolean isKnownSystemApp,













            boolean isLoggableBySdk, boolean doStatsLog) {













            boolean doStatsLog) {















        boolean isAlreadyReported =















                checkAndSetIsAlreadyReported(uid, new ChangeReport(changeId, state));















        if (doStatsLog && shouldWriteToStatsLog(isKnownSystemApp, isAlreadyReported)) {















            FrameworkStatsLog.write(FrameworkStatsLog.APP_COMPATIBILITY_CHANGE_REPORTED, uid,















                    changeId, state, mSource);















        }













        if (shouldWriteToDebug(isAlreadyReported, state, isLoggableBySdk)) {













        if (shouldWriteToDebug(isAlreadyReported, state)) {















            debugLog(uid, changeId, state);















        }















    }









@@ -122,7 +119,7 @@ public class ChangeReporter {













     * @param doStatsLog whether to log to stats log.















     */















    public void reportChange(int uid, long changeId, int state, boolean doStatsLog) {













        reportChange(uid, changeId, state, false, true, doStatsLog);













        reportChange(uid, changeId, state, false, doStatsLog);















    }































    /**









@@ -148,7 +145,7 @@ public class ChangeReporter {













     * @return true if the report should be logged















     */















    @VisibleForTesting













    boolean shouldWriteToStatsLog(boolean isKnownSystemApp, boolean isAlreadyReported) {













    public boolean shouldWriteToStatsLog(boolean isKnownSystemApp, boolean isAlreadyReported) {















        // We don't log where we know the source is a system app or is already reported















        return !isKnownSystemApp && !isAlreadyReported;















    }









@@ -158,13 +155,9 @@ public class ChangeReporter {













     *















     * @param isAlreadyReported is the change already reported















     * @param state             of the reported change - enabled/disabled/only logged













     * @param isLoggableBySdk   whether debug logging is allowed for this change based on target SDK













     *                          version. This is combined with other logic to determine whether to













     *                          actually log. If the sdk version does not matter, should be true.















     * @return true if the report should be logged















     */













    private boolean shouldWriteToDebug(













            boolean isAlreadyReported, int state, boolean isLoggableBySdk) {













    private boolean shouldWriteToDebug(boolean isAlreadyReported, int state) {















        // If log all bit is on, always return true.















        if (mDebugLogAll) return true;















        // If the change has already been reported, do not write.









@@ -176,8 +169,8 @@ public class ChangeReporter {













        boolean skipLoggingFlag = Flags.skipOldAndDisabledCompatLogging();















        if (!skipLoggingFlag || Log.isLoggable(TAG, Log.DEBUG)) return true;





























        // Log if the change is enabled and targets the latest sdk version.













        return isLoggableBySdk && state != STATE_DISABLED;













        // Log if the change is enabled













        return state != STATE_DISABLED;















    }































    /**









@@ -186,29 +179,12 @@ public class ChangeReporter {













     * @param uid               affected by the change















     * @param changeId          the reported change id















     * @param state             of the reported change - enabled/disabled/only logged













     *













     * @return true if the report should be logged













     */













    @VisibleForTesting













    boolean shouldWriteToDebug(int uid, long changeId, int state) {













        return shouldWriteToDebug(uid, changeId, state, true);













    }



























    /**













     * Returns whether the next report should be logged to logcat.













     *













     * @param uid               affected by the change













     * @param changeId          the reported change id













     * @param state             of the reported change - enabled/disabled/only logged













     * @param isLoggableBySdk   whether debug logging is allowed for this change based on target SDK













     *                          version. This is combined with other logic to determine whether to













     *                          actually log. If the sdk version does not matter, should be true.















     * @return true if the report should be logged















     */















    @VisibleForTesting













    boolean shouldWriteToDebug(int uid, long changeId, int state, boolean isLoggableBySdk) {













    public boolean shouldWriteToDebug(int uid, long changeId, int state) {















        return shouldWriteToDebug(













                isAlreadyReported(uid, new ChangeReport(changeId, state)), state, isLoggableBySdk);













                isAlreadyReported(uid, new ChangeReport(changeId, state)), state);















    }































    /**










@@ -240,7 +216,7 @@ public class ChangeReporter {













     * @return true if the report should be logged















     */















    @VisibleForTesting













    boolean isAlreadyReported(int uid, long changeId, int state) {













    public boolean isAlreadyReported(int uid, long changeId, int state) {















        return isAlreadyReported(uid, new ChangeReport(changeId, state));















    }

















































core/java/com/android/internal/compat/CompatibilityChangeInfo.java



+15
−1




























Original line number
Diff line number
Diff line








@@ -32,6 +32,7 @@ public class CompatibilityChangeInfo implements Parcelable {













    private final int mEnableSinceTargetSdk;















    private final boolean mDisabled;















    private final boolean mLoggingOnly;













    private final boolean mNoLogging;















    private final @Nullable String mDescription;















    private final boolean mOverridable;

























@@ -56,6 +57,10 @@ public class CompatibilityChangeInfo implements Parcelable {













        return mLoggingOnly;















    }





























    public boolean getNoLogging() {













        return mNoLogging;













    }





























    public String getDescription()  {















        return mDescription;















    }









@@ -66,7 +71,8 @@ public class CompatibilityChangeInfo implements Parcelable {





























    public CompatibilityChangeInfo(















            Long changeId, String name, int enableAfterTargetSdk, int enableSinceTargetSdk,













            boolean disabled, boolean loggingOnly, String description, boolean overridable) {













            boolean disabled, boolean loggingOnly, boolean noLogging, String description,













            boolean overridable) {















        this.mChangeId = changeId;















        this.mName = name;















        if (enableAfterTargetSdk > 0) {









@@ -80,6 +86,7 @@ public class CompatibilityChangeInfo implements Parcelable {













        }















        this.mDisabled = disabled;















        this.mLoggingOnly = loggingOnly;













        this.mNoLogging = noLogging;















        this.mDescription = description;















        this.mOverridable = overridable;















    }









@@ -90,6 +97,7 @@ public class CompatibilityChangeInfo implements Parcelable {













        this.mEnableSinceTargetSdk = other.mEnableSinceTargetSdk;















        this.mDisabled = other.mDisabled;















        this.mLoggingOnly = other.mLoggingOnly;













        this.mNoLogging = other.mNoLogging;















        this.mDescription = other.mDescription;















        this.mOverridable = other.mOverridable;















    }









@@ -100,6 +108,7 @@ public class CompatibilityChangeInfo implements Parcelable {













        mEnableSinceTargetSdk = in.readInt();















        mDisabled = in.readBoolean();















        mLoggingOnly = in.readBoolean();













        mNoLogging = in.readBoolean();















        mDescription = in.readString();















        mOverridable = in.readBoolean();















    }









@@ -116,6 +125,7 @@ public class CompatibilityChangeInfo implements Parcelable {













        dest.writeInt(mEnableSinceTargetSdk);















        dest.writeBoolean(mDisabled);















        dest.writeBoolean(mLoggingOnly);













        dest.writeBoolean(mNoLogging);















        dest.writeString(mDescription);















        dest.writeBoolean(mOverridable);















    }









@@ -136,6 +146,9 @@ public class CompatibilityChangeInfo implements Parcelable {













        if (getLoggingOnly()) {















            sb.append("; loggingOnly");















        }













        if (getNoLogging()) {













            sb.append("; noLogging");













        }















        if (getOverridable()) {















            sb.append("; overridable");















        }









@@ -156,6 +169,7 @@ public class CompatibilityChangeInfo implements Parcelable {













                && this.mEnableSinceTargetSdk == that.mEnableSinceTargetSdk















                && this.mDisabled == that.mDisabled















                && this.mLoggingOnly == that.mLoggingOnly













                && this.mNoLogging == that.mNoLogging















                && this.mDescription.equals(that.mDescription)















                && this.mOverridable == that.mOverridable;















    }

































core/java/com/android/internal/compat/TEST_MAPPING
0 → 100644



+8
−0




























Original line number
Diff line number
Diff line













{













  "presubmit": [













      // Unit tests













      {













          "name": "PlatformCompatFrameworkTests"













      }













  ]













}










 No newline at end of file