From 65adfeecd2acc4e63c00fc1f2073cc0b229f3467 Mon Sep 17 00:00:00 2001 From: Joe Onorato Date: Thu, 7 Apr 2016 16:59:59 -0700 Subject: [PATCH] Add some tests for procstats, and fix some bugs that the tests uncovered. Bug: 27045736 Change-Id: Ia910730c5a3a899c89aa63dd7ab48be62cf9cfef --- .../internal/app/procstats/ProcessStats.java | 2 +- .../app/procstats/SparseMappingTable.java | 66 +++--- .../app/procstats/SparseMappingTableTest.java | 209 ++++++++++++++++++ 3 files changed, 251 insertions(+), 26 deletions(-) create mode 100644 core/tests/coretests/src/com/android/internal/app/procstats/SparseMappingTableTest.java diff --git a/core/java/com/android/internal/app/procstats/ProcessStats.java b/core/java/com/android/internal/app/procstats/ProcessStats.java index 06542f7af7e7f..2f80b862c72ba 100644 --- a/core/java/com/android/internal/app/procstats/ProcessStats.java +++ b/core/java/com/android/internal/app/procstats/ProcessStats.java @@ -516,7 +516,7 @@ public final class ProcessStats implements Parcelable { out.writeInt((int)val); } else { int top = ~((int)((val>>32)&0x7fffffff)); - int bottom = (int)(val&0xfffffff); + int bottom = (int)(val&0x0ffffffffL); out.writeInt(top); out.writeInt(bottom); } diff --git a/core/java/com/android/internal/app/procstats/SparseMappingTable.java b/core/java/com/android/internal/app/procstats/SparseMappingTable.java index 76102af9bcb0a..f941836d2c099 100644 --- a/core/java/com/android/internal/app/procstats/SparseMappingTable.java +++ b/core/java/com/android/internal/app/procstats/SparseMappingTable.java @@ -68,11 +68,8 @@ public class SparseMappingTable { * A table of data as stored in a SparseMappingTable. */ public static class Table { - // When mSequence is this this our data better be empty - private static final int UNINITIALIZED_SEQUENCE = -1; - private SparseMappingTable mParent; - private int mSequence = UNINITIALIZED_SEQUENCE; + private int mSequence = 1; private int[] mTable; private int mSize; @@ -119,12 +116,6 @@ public class SparseMappingTable { * but should be considered opaque to the caller. */ public int getOrAddKey(byte id, int count) { - // This is the only place we add data to mParent.mLongs, so this is the time - // to update our sequence to match there. - if (mSequence == UNINITIALIZED_SEQUENCE) { - mSequence = mParent.mSequence; - } - assertConsistency(); final int idx = binarySearch(id); @@ -311,7 +302,7 @@ public class SparseMappingTable { // Reset our sequence number. This will make all read/write calls // start to fail, and then when we re-allocate it will be re-synced // to that of mParent. - mSequence = UNINITIALIZED_SEQUENCE; + mSequence = mParent.mSequence; } /** @@ -377,27 +368,19 @@ public class SparseMappingTable { // Original bug: b/27045736 // New bug: b/27960286 if (false) { - // Assert that our sequence number has been initialized. If it hasn't - // that means someone tried to read or write data without allocating it - // since we were created or reset. - if (mSequence == UNINITIALIZED_SEQUENCE) { - logOrThrow("mSequence == UNINITIALIZED_SEQUENCE in" - + " SparseMappingTable.Table. -- " - + dumpInternalState()); - return; - } - // Assert that our sequence number matches mParent's. If it isn't that means - // we have been reset and our + // we have been reset and our. If our sequence is UNITIALIZED_SEQUENCE, then + // it's possible that everything is working fine and we just haven't been + // written to since the last resetTable(). if (mSequence != mParent.mSequence) { if (mSequence < mParent.mSequence) { - logOrThrow("Sequence mismatch. SparseMappingTable.resetTable()" + logOrThrow("Sequence mismatch. SparseMappingTable.reset()" + " called but not Table.resetTable() -- " + dumpInternalState()); return; } else if (mSequence > mParent.mSequence) { logOrThrow("Sequence mismatch. Table.resetTable()" - + " called but not SparseMappingTable.resetTable() -- " + + " called but not SparseMappingTable.reset() -- " + dumpInternalState()); return; } @@ -494,6 +477,10 @@ public class SparseMappingTable { } } + public SparseMappingTable() { + mLongs.add(new long[ARRAY_SIZE]); + } + /** * Wipe out all the data. */ @@ -544,6 +531,35 @@ public class SparseMappingTable { } } + /** + * Return a string for debugging. + */ + public String dumpInternalState(boolean includeData) { + final StringBuilder sb = new StringBuilder(); + sb.append("SparseMappingTable{"); + sb.append("mSequence="); + sb.append(mSequence); + sb.append(" mNextIndex="); + sb.append(mNextIndex); + sb.append(" mLongs.size="); + final int N = mLongs.size(); + sb.append(N); + sb.append("\n"); + if (includeData) { + for (int i=0; i>32)&0x7fffffff)); - int bottom = (int)(val&0xfffffff); + int bottom = (int)(val&0x0ffffffffL); out.writeInt(top); out.writeInt(bottom); } diff --git a/core/tests/coretests/src/com/android/internal/app/procstats/SparseMappingTableTest.java b/core/tests/coretests/src/com/android/internal/app/procstats/SparseMappingTableTest.java new file mode 100644 index 0000000000000..fd57baa217b0f --- /dev/null +++ b/core/tests/coretests/src/com/android/internal/app/procstats/SparseMappingTableTest.java @@ -0,0 +1,209 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.internal.app.procstats; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.ArrayList; + +import android.os.BatteryStats; +import android.os.Parcel; +import android.test.suitebuilder.annotation.SmallTest; +import android.util.Log; + +import junit.framework.Assert; +import junit.framework.TestCase; + +import org.mockito.Mockito; + +/** + * Provides test cases for SparseMappingTable. + */ +public class SparseMappingTableTest extends TestCase { + private static final String TAG = "SparseMappingTableTest"; + + final byte ID1 = 1; + final byte ID2 = 2; + + final long VALUE1 = 100; + final long VALUE2 = 10000000000L; + + /** + * Test the parceling and unparceling logic when there is no data. + */ + @SmallTest + public void testParcelingEmpty() throws Exception { + final SparseMappingTable data = new SparseMappingTable(); + final SparseMappingTable.Table table = new SparseMappingTable.Table(data); + + final Parcel dataParcel = Parcel.obtain(); + data.writeToParcel(dataParcel); + + final Parcel tableParcel = Parcel.obtain(); + table.writeToParcel(tableParcel); + + dataParcel.setDataPosition(0); + final SparseMappingTable data1 = new SparseMappingTable(); + data1.readFromParcel(dataParcel); + Assert.assertEquals(0, dataParcel.dataAvail()); + dataParcel.recycle(); + + tableParcel.setDataPosition(0); + final SparseMappingTable.Table table1 = new SparseMappingTable.Table(data1); + table1.readFromParcel(tableParcel); + Assert.assertEquals(0, tableParcel.dataAvail()); + tableParcel.recycle(); + } + + /** + * Test the parceling and unparceling logic. + */ + @SmallTest + public void testParceling() throws Exception { + int key; + final SparseMappingTable data = new SparseMappingTable(); + final SparseMappingTable.Table table = new SparseMappingTable.Table(data); + + key = table.getOrAddKey(ID1, 1); + table.setValue(key, VALUE1); + + key = table.getOrAddKey(ID2, 1); + table.setValue(key, VALUE2); + + final Parcel dataParcel = Parcel.obtain(); + data.writeToParcel(dataParcel); + + final Parcel tableParcel = Parcel.obtain(); + table.writeToParcel(tableParcel); + + dataParcel.setDataPosition(0); + final SparseMappingTable data1 = new SparseMappingTable(); + data1.readFromParcel(dataParcel); + Assert.assertEquals(0, dataParcel.dataAvail()); + dataParcel.recycle(); + + tableParcel.setDataPosition(0); + final SparseMappingTable.Table table1 = new SparseMappingTable.Table(data1); + table1.readFromParcel(tableParcel); + Assert.assertEquals(0, tableParcel.dataAvail()); + tableParcel.recycle(); + + key = table1.getKey(ID1); + Assert.assertEquals(VALUE1, table1.getValue(key)); + + key = table1.getKey(ID2); + Assert.assertEquals(VALUE2, table1.getValue(key)); + } + + + /** + * Test that after resetting you can still read data, you just get no values. + */ + @SmallTest + public void testParcelingWithReset() throws Exception { + int key; + final SparseMappingTable data = new SparseMappingTable(); + final SparseMappingTable.Table table = new SparseMappingTable.Table(data); + + key = table.getOrAddKey(ID1, 1); + table.setValue(key, VALUE1); + + data.reset(); + table.resetTable(); + + key = table.getOrAddKey(ID2, 1); + table.setValue(key, VALUE2); + + Log.d(TAG, "before: " + data.dumpInternalState(true)); + Log.d(TAG, "before: " + table.dumpInternalState()); + + final Parcel dataParcel = Parcel.obtain(); + data.writeToParcel(dataParcel); + + final Parcel tableParcel = Parcel.obtain(); + table.writeToParcel(tableParcel); + + dataParcel.setDataPosition(0); + final SparseMappingTable data1 = new SparseMappingTable(); + data1.readFromParcel(dataParcel); + Assert.assertEquals(0, dataParcel.dataAvail()); + dataParcel.recycle(); + + tableParcel.setDataPosition(0); + final SparseMappingTable.Table table1 = new SparseMappingTable.Table(data1); + table1.readFromParcel(tableParcel); + Assert.assertEquals(0, tableParcel.dataAvail()); + tableParcel.recycle(); + + key = table1.getKey(ID1); + Assert.assertEquals(SparseMappingTable.INVALID_KEY, key); + + key = table1.getKey(ID2); + Assert.assertEquals(VALUE2, table1.getValue(key)); + + Log.d(TAG, " after: " + data1.dumpInternalState(true)); + Log.d(TAG, " after: " + table1.dumpInternalState()); + } + + /** + * Test that it fails if you reset the data and not the table. + * + * Resetting the table and not the data is basically okay. The data in the + * SparseMappingTable will be leaked. + */ + @SmallTest + public void testResetDataOnlyFails() throws Exception { + int key; + final SparseMappingTable data = new SparseMappingTable(); + final SparseMappingTable.Table table = new SparseMappingTable.Table(data); + + key = table.getOrAddKey(ID1, 1); + table.setValue(key, VALUE1); + + Assert.assertEquals(VALUE1, table.getValue(key)); + + data.reset(); + + try { + table.getValue(key); + throw new Exception("Exception not thrown after mismatched reset calls."); + } catch (RuntimeException ex) { + // Good + } + } + + /** + * Test that trying to get data that you didn't add fails correctly. + */ + @SmallTest + public void testInvalidKey() throws Exception { + int key; + final SparseMappingTable data = new SparseMappingTable(); + final SparseMappingTable.Table table = new SparseMappingTable.Table(data); + + key = table.getKey(ID1); + + // The key should be INVALID_KEY + Assert.assertEquals(SparseMappingTable.INVALID_KEY, key); + + // If you get the value with getValueForId you get 0. + Assert.assertEquals(0, table.getValueForId(ID1)); + } +} + + +