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

Commit 0b8bee01 authored by Mark White's avatar Mark White
Browse files

Revert "Revert "Resolve compat perf regression via @NoLogging an..."

Revert submission 34235880-revert-34109140-compat-nologging-ULMDUCRDIO

Reason for revert: Reapplying with mainline fix 

Reverted changes: /q/submissionid:34235880-revert-34109140-compat-nologging-ULMDUCRDIO

Change-Id: Ic31ed07806ee096d2201da6080e36fa2dc6de680
parent c005a05f
Loading
Loading
Loading
Loading
+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);
    }

}
+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
+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));
    }

+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;
    }
+8 −0
Original line number Diff line number Diff line
{
  "presubmit": [
      // Unit tests
      {
          "name": "PlatformCompatFrameworkTests"
      }
  ]
}
 No newline at end of file
Loading