From b7edebc8adc1aeb1063afc4b6726a46f87aff6e4 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Mon, 11 Apr 2016 01:38:23 -0700 Subject: [PATCH] Fix inefficient CursorAnchorInfo#hashCode(). It turns out that the current CursorAnchorInfo#equals() is quite inefficient because our CursorAnchorInfo#hashCode() tries to use almost all the fields. Even worse, as Matrix#hashCode() is hard-coded to 44, we get the same hashCode() when comparing two CursorAnchorInfo objects that are different only in transformation Matrix after such a complex hash calculation. In the real world scenarios, most likely calculation hash code only from Matrix and composing text would be good enough for our use case, because the former can cover UI scrolling scenario and the latter can cover the text typing scenario. More complex hash calculation is probably inefficient. With this CL, CursorAnchorInfo#hashCode() is pre-calculated only from those two fields, and carefully reorder comparisons in CursorAnchorInfo#equals() to improve the likelihood of returning false with fewer comparisons. Bug: 28105733 Change-Id: Id896adeab5ffe87ceddb2c2762d6d91475e28ec4 --- .../view/inputmethod/CursorAnchorInfo.java | 90 ++++++++++++------- 1 file changed, 58 insertions(+), 32 deletions(-) diff --git a/core/java/android/view/inputmethod/CursorAnchorInfo.java b/core/java/android/view/inputmethod/CursorAnchorInfo.java index fd73432b42d30..24739bf8f3b3e 100644 --- a/core/java/android/view/inputmethod/CursorAnchorInfo.java +++ b/core/java/android/view/inputmethod/CursorAnchorInfo.java @@ -16,6 +16,7 @@ package android.view.inputmethod; +import android.annotation.NonNull; import android.graphics.Matrix; import android.graphics.RectF; import android.os.Parcel; @@ -25,6 +26,7 @@ import android.text.SpannedString; import android.text.TextUtils; import android.view.inputmethod.SparseRectFArray.SparseRectFArrayBuilder; +import java.util.Arrays; import java.util.Objects; /** @@ -35,6 +37,11 @@ import java.util.Objects; * actually inserted.

*/ public final class CursorAnchorInfo implements Parcelable { + /** + * The pre-computed hash code. + */ + private final int mHashCode; + /** * The index of the first character of the selected text (inclusive). {@code -1} when there is * no text selection. @@ -100,7 +107,8 @@ public final class CursorAnchorInfo implements Parcelable { * Transformation matrix that is applied to any positional information of this class to * transform local coordinates into screen coordinates. */ - private final Matrix mMatrix; + @NonNull + private final float[] mMatrixValues; /** * Flag for {@link #getInsertionMarkerFlags()} and {@link #getCharacterBoundsFlags(int)}: the @@ -121,6 +129,7 @@ public final class CursorAnchorInfo implements Parcelable { public static final int FLAG_IS_RTL = 0x04; public CursorAnchorInfo(final Parcel source) { + mHashCode = source.readInt(); mSelectionStart = source.readInt(); mSelectionEnd = source.readInt(); mComposingTextStart = source.readInt(); @@ -131,8 +140,7 @@ public final class CursorAnchorInfo implements Parcelable { mInsertionMarkerBaseline = source.readFloat(); mInsertionMarkerBottom = source.readFloat(); mCharacterBoundsArray = source.readParcelable(SparseRectFArray.class.getClassLoader()); - mMatrix = new Matrix(); - mMatrix.setValues(source.createFloatArray()); + mMatrixValues = source.createFloatArray(); } /** @@ -143,6 +151,7 @@ public final class CursorAnchorInfo implements Parcelable { */ @Override public void writeToParcel(Parcel dest, int flags) { + dest.writeInt(mHashCode); dest.writeInt(mSelectionStart); dest.writeInt(mSelectionEnd); dest.writeInt(mComposingTextStart); @@ -153,27 +162,12 @@ public final class CursorAnchorInfo implements Parcelable { dest.writeFloat(mInsertionMarkerBaseline); dest.writeFloat(mInsertionMarkerBottom); dest.writeParcelable(mCharacterBoundsArray, flags); - final float[] matrixArray = new float[9]; - mMatrix.getValues(matrixArray); - dest.writeFloatArray(matrixArray); + dest.writeFloatArray(mMatrixValues); } @Override public int hashCode(){ - final float floatHash = mInsertionMarkerHorizontal + mInsertionMarkerTop - + mInsertionMarkerBaseline + mInsertionMarkerBottom; - int hash = floatHash > 0 ? (int) floatHash : (int)(-floatHash); - hash *= 31; - hash += mInsertionMarkerFlags; - hash *= 31; - hash += mSelectionStart + mSelectionEnd + mComposingTextStart; - hash *= 31; - hash += Objects.hashCode(mComposingText); - hash *= 31; - hash += Objects.hashCode(mCharacterBoundsArray); - hash *= 31; - hash += Objects.hashCode(mMatrix); - return hash; + return mHashCode; } /** @@ -202,13 +196,13 @@ public final class CursorAnchorInfo implements Parcelable { if (hashCode() != that.hashCode()) { return false; } + + // Check fields that are not covered by hashCode() first. + if (mSelectionStart != that.mSelectionStart || mSelectionEnd != that.mSelectionEnd) { return false; } - if (mComposingTextStart != that.mComposingTextStart - || !Objects.equals(mComposingText, that.mComposingText)) { - return false; - } + if (mInsertionMarkerFlags != that.mInsertionMarkerFlags || !areSameFloatImpl(mInsertionMarkerHorizontal, that.mInsertionMarkerHorizontal) || !areSameFloatImpl(mInsertionMarkerTop, that.mInsertionMarkerTop) @@ -216,18 +210,35 @@ public final class CursorAnchorInfo implements Parcelable { || !areSameFloatImpl(mInsertionMarkerBottom, that.mInsertionMarkerBottom)) { return false; } + if (!Objects.equals(mCharacterBoundsArray, that.mCharacterBoundsArray)) { return false; } - if (!Objects.equals(mMatrix, that.mMatrix)) { + + // Following fields are (partially) covered by hashCode(). + + if (mComposingTextStart != that.mComposingTextStart + || !Objects.equals(mComposingText, that.mComposingText)) { return false; } + + // We do not use Arrays.equals(float[], float[]) to keep the previous behavior regarding + // NaN, 0.0f, and -0.0f. + if (mMatrixValues.length != that.mMatrixValues.length) { + return false; + } + for (int i = 0; i < mMatrixValues.length; ++i) { + if (mMatrixValues[i] != that.mMatrixValues[i]) { + return false; + } + } return true; } @Override public String toString() { - return "SelectionInfo{mSelection=" + mSelectionStart + "," + mSelectionEnd + return "CursorAnchorInfo{mHashCode=" + mHashCode + + " mSelection=" + mSelectionStart + "," + mSelectionEnd + " mComposingTextStart=" + mComposingTextStart + " mComposingText=" + Objects.toString(mComposingText) + " mInsertionMarkerFlags=" + mInsertionMarkerFlags @@ -236,7 +247,7 @@ public final class CursorAnchorInfo implements Parcelable { + " mInsertionMarkerBaseline=" + mInsertionMarkerBaseline + " mInsertionMarkerBottom=" + mInsertionMarkerBottom + " mCharacterBoundsArray=" + Objects.toString(mCharacterBoundsArray) - + " mMatrix=" + Objects.toString(mMatrix) + + " mMatrix=" + Arrays.toString(mMatrixValues) + "}"; } @@ -254,7 +265,7 @@ public final class CursorAnchorInfo implements Parcelable { private float mInsertionMarkerBottom = Float.NaN; private int mInsertionMarkerFlags = 0; private SparseRectFArrayBuilder mCharacterBoundsArrayBuilder = null; - private final Matrix mMatrix = new Matrix(Matrix.IDENTITY_MATRIX); + private float[] mMatrixValues = null; private boolean mMatrixInitialized = false; /** @@ -349,7 +360,10 @@ public final class CursorAnchorInfo implements Parcelable { * is interpreted as an identity matrix. */ public Builder setMatrix(final Matrix matrix) { - mMatrix.set(matrix != null ? matrix : Matrix.IDENTITY_MATRIX); + if (mMatrixValues == null) { + mMatrixValues = new float[9]; + } + (matrix != null ? matrix : Matrix.IDENTITY_MATRIX).getValues(mMatrixValues); mMatrixInitialized = true; return this; } @@ -391,7 +405,6 @@ public final class CursorAnchorInfo implements Parcelable { mInsertionMarkerTop = Float.NaN; mInsertionMarkerBaseline = Float.NaN; mInsertionMarkerBottom = Float.NaN; - mMatrix.set(Matrix.IDENTITY_MATRIX); mMatrixInitialized = false; if (mCharacterBoundsArrayBuilder != null) { mCharacterBoundsArrayBuilder.reset(); @@ -411,7 +424,18 @@ public final class CursorAnchorInfo implements Parcelable { mInsertionMarkerBottom = builder.mInsertionMarkerBottom; mCharacterBoundsArray = builder.mCharacterBoundsArrayBuilder != null ? builder.mCharacterBoundsArrayBuilder.build() : null; - mMatrix = new Matrix(builder.mMatrix); + mMatrixValues = new float[9]; + if (builder.mMatrixInitialized) { + System.arraycopy(builder.mMatrixValues, 0, mMatrixValues, 0, 9); + } else { + Matrix.IDENTITY_MATRIX.getValues(mMatrixValues); + } + + // To keep hash function simple, we only use some complex objects for hash. + int hash = Objects.hashCode(mComposingText); + hash *= 31; + hash += Arrays.hashCode(mMatrixValues); + mHashCode = hash; } /** @@ -527,7 +551,9 @@ public final class CursorAnchorInfo implements Parcelable { * @return a new instance (copy) of the transformation matrix. */ public Matrix getMatrix() { - return new Matrix(mMatrix); + final Matrix matrix = new Matrix(); + matrix.setValues(mMatrixValues); + return matrix; } /**