Revert "Resolve compat perf regression via @NoLogging annotation" (c005a05f) · Commits · e / os / android_frameworks_base

core/java/android/app/AppCompatCallbacks.java

+5 −10

Original line number	Diff line number	Diff line
		@@ -86,29 +86,24 @@ 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, doStatsLog);
		reportChange(changeId, ChangeReporter.STATE_ENABLED, isLoggable, doStatsLog);
		return true;
		}
		reportChange(changeId, ChangeReporter.STATE_DISABLED, doStatsLog);
		reportChange(changeId, ChangeReporter.STATE_DISABLED, isLoggable, doStatsLog);
		return false;
		}

		private void reportChange(long changeId, int state, boolean doStatsLog) {
		private void reportChange(long changeId, int state, boolean isLoggable, boolean doStatsLog) {
		int uid = Process.myUid();
		mChangeReporter.reportChange(uid, changeId, state, false, doStatsLog);
		mChangeReporter.reportChange(uid, changeId, state, false, isLoggable, doStatsLog);
		}

		}

core/java/android/widget/TextView.java

+0 −4

Original line number	Diff line number	Diff line
		@@ -60,7 +60,6 @@ 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;
		@@ -538,7 +537,6 @@ 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

		@@ -547,7 +545,6 @@ 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

		@@ -557,7 +554,6 @@ 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

+34 −10

Original line number	Diff line number	Diff line
		@@ -94,17 +94,20 @@ 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 doStatsLog) {
		boolean isLoggableBySdk, 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)) {
		if (shouldWriteToDebug(isAlreadyReported, state, isLoggableBySdk)) {
		debugLog(uid, changeId, state);
		}
		}
		@@ -119,7 +122,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, doStatsLog);
		reportChange(uid, changeId, state, false, true, doStatsLog);
		}

		/**
		@@ -145,7 +148,7 @@ public class ChangeReporter {
		* @return true if the report should be logged
		*/
		@VisibleForTesting
		public boolean shouldWriteToStatsLog(boolean isKnownSystemApp, boolean isAlreadyReported) {
		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;
		}
		@@ -155,9 +158,13 @@ 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) {
		private boolean shouldWriteToDebug(
		boolean isAlreadyReported, int state, boolean isLoggableBySdk) {
		// If log all bit is on, always return true.
		if (mDebugLogAll) return true;
		// If the change has already been reported, do not write.
		@@ -169,8 +176,8 @@ public class ChangeReporter {
		boolean skipLoggingFlag = Flags.skipOldAndDisabledCompatLogging();
		if (!skipLoggingFlag \|\| Log.isLoggable(TAG, Log.DEBUG)) return true;

		// Log if the change is enabled
		return state != STATE_DISABLED;
		// Log if the change is enabled and targets the latest sdk version.
		return isLoggableBySdk && state != STATE_DISABLED;
		}

		/**
		@@ -179,12 +186,29 @@ 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
		public boolean shouldWriteToDebug(int uid, long changeId, int state) {
		boolean shouldWriteToDebug(int uid, long changeId, int state, boolean isLoggableBySdk) {
		return shouldWriteToDebug(
		isAlreadyReported(uid, new ChangeReport(changeId, state)), state);
		isAlreadyReported(uid, new ChangeReport(changeId, state)), state, isLoggableBySdk);
		}

		/**
		@@ -216,7 +240,7 @@ public class ChangeReporter {
		* @return true if the report should be logged
		*/
		@VisibleForTesting
		public boolean isAlreadyReported(int uid, long changeId, int state) {
		boolean isAlreadyReported(int uid, long changeId, int state) {
		return isAlreadyReported(uid, new ChangeReport(changeId, state));
		}

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

+1 −15

Original line number	Diff line number	Diff line
		@@ -32,7 +32,6 @@ 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;

		@@ -57,10 +56,6 @@ public class CompatibilityChangeInfo implements Parcelable {
		return mLoggingOnly;
		}

		public boolean getNoLogging() {
		return mNoLogging;
		}

		public String getDescription() {
		return mDescription;
		}
		@@ -71,8 +66,7 @@ public class CompatibilityChangeInfo implements Parcelable {

		public CompatibilityChangeInfo(
		Long changeId, String name, int enableAfterTargetSdk, int enableSinceTargetSdk,
		boolean disabled, boolean loggingOnly, boolean noLogging, String description,
		boolean overridable) {
		boolean disabled, boolean loggingOnly, String description, boolean overridable) {
		this.mChangeId = changeId;
		this.mName = name;
		if (enableAfterTargetSdk > 0) {
		@@ -86,7 +80,6 @@ public class CompatibilityChangeInfo implements Parcelable {
		}
		this.mDisabled = disabled;
		this.mLoggingOnly = loggingOnly;
		this.mNoLogging = noLogging;
		this.mDescription = description;
		this.mOverridable = overridable;
		}
		@@ -97,7 +90,6 @@ 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;
		}
		@@ -108,7 +100,6 @@ public class CompatibilityChangeInfo implements Parcelable {
		mEnableSinceTargetSdk = in.readInt();
		mDisabled = in.readBoolean();
		mLoggingOnly = in.readBoolean();
		mNoLogging = in.readBoolean();
		mDescription = in.readString();
		mOverridable = in.readBoolean();
		}
		@@ -125,7 +116,6 @@ public class CompatibilityChangeInfo implements Parcelable {
		dest.writeInt(mEnableSinceTargetSdk);
		dest.writeBoolean(mDisabled);
		dest.writeBoolean(mLoggingOnly);
		dest.writeBoolean(mNoLogging);
		dest.writeString(mDescription);
		dest.writeBoolean(mOverridable);
		}
		@@ -146,9 +136,6 @@ public class CompatibilityChangeInfo implements Parcelable {
		if (getLoggingOnly()) {
		sb.append("; loggingOnly");
		}
		if (getNoLogging()) {
		sb.append("; noLogging");
		}
		if (getOverridable()) {
		sb.append("; overridable");
		}
		@@ -169,7 +156,6 @@ 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

deleted100644 → 0

+0 −8

Original line number	Diff line number	Diff line
		{
		"presubmit": [
		// Unit tests
		{
		"name": "PlatformCompatFrameworkTests"
		}
		]
		}
		No newline at end of file