From 32b8b2093dbada86436506d0b548dd4e5d28b66a Mon Sep 17 00:00:00 2001 From: Rodrigo Damazio Date: Fri, 13 Aug 2010 03:50:55 -0300 Subject: [PATCH] REAL performance improvement in importer - using a bulk insertion --- .../mytracks/content/MyTracksProvider.java | 83 ++++++++---- .../content/MyTracksProviderUtils.java | 15 ++- .../content/MyTracksProviderUtilsImpl.java | 16 ++- .../android/apps/mytracks/io/GpxImporter.java | 64 +++++++-- .../apps/mytracks/io/GpxImporterTest.java | 127 +++++++++++------- 5 files changed, 216 insertions(+), 89 deletions(-) diff --git a/MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProvider.java b/MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProvider.java index a4b542e25..010e553f8 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProvider.java +++ b/MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProvider.java @@ -167,24 +167,25 @@ public class MyTracksProvider extends ContentProvider { @Override public int delete(Uri url, String where, String[] selectionArgs) { - if (urlMatcher.match(url) == TRACKPOINTS) { - Log.w(MyTracksProvider.TAG, "provider trackpoints delete!"); - int count = db.delete(TRACKPOINTS_TABLE, where, selectionArgs); - getContext().getContentResolver().notifyChange(url, null, true); - return count; - } else if (urlMatcher.match(url) == TRACKS) { - Log.w(MyTracksProvider.TAG, "provider track delete!"); - int count = db.delete(TRACKS_TABLE, where, selectionArgs); - getContext().getContentResolver().notifyChange(url, null, true); - return count; - } else if (urlMatcher.match(url) == WAYPOINTS) { - Log.w(MyTracksProvider.TAG, "provider waypoint delete!"); - int count = db.delete(WAYPOINTS_TABLE, where, selectionArgs); - getContext().getContentResolver().notifyChange(url, null, true); - return count; - } else { - throw new IllegalArgumentException("Unknown URL " + url); + String table; + switch (urlMatcher.match(url)) { + case TRACKPOINTS: + table = TRACKPOINTS_TABLE; + break; + case TRACKS: + table = TRACKS_TABLE; + break; + case WAYPOINTS: + table = WAYPOINTS_TABLE; + break; + default: + throw new IllegalArgumentException("Unknown URL " + url); } + + Log.w(MyTracksProvider.TAG, "provider delete in " + table + "!"); + int count = db.delete(table, where, selectionArgs); + getContext().getContentResolver().notifyChange(url, null, true); + return count; } @Override @@ -216,19 +217,49 @@ public class MyTracksProvider extends ContentProvider { } else { values = new ContentValues(); } - if (urlMatcher.match(url) == TRACKPOINTS) { - return insertTrackPoint(url, values); - } else if (urlMatcher.match(url) == TRACKS) { - return insertTrack(url, values); - } else if (urlMatcher.match(url) == WAYPOINTS) { - return insertWaypoint(url, values); - } else { - throw new IllegalArgumentException("Unknown URL " + url); + + int urlMatchType = urlMatcher.match(url); + return insertType(url, urlMatchType, values); + } + + private Uri insertType(Uri url, int urlMatchType, ContentValues values) { + switch (urlMatchType) { + case TRACKPOINTS: + return insertTrackPoint(url, values); + case TRACKS: + return insertTrack(url, values); + case WAYPOINTS: + return insertWaypoint(url, values); + default: + throw new IllegalArgumentException("Unknown URL " + url); } } + @Override + public int bulkInsert(Uri url, ContentValues[] valuesBulk) { + Log.d(MyTracksProvider.TAG, "MyTracksProvider.bulkInsert"); + int numInserted = 0; + try { + // Use a transaction in order to make the insertions run as a single batch + db.beginTransaction(); + + int urlMatch = urlMatcher.match(url); + for (numInserted = 0; numInserted < valuesBulk.length; numInserted++) { + ContentValues values = valuesBulk[numInserted]; + if (values == null) { values = new ContentValues(); } + + insertType(url, urlMatch, values); + } + + db.setTransactionSuccessful(); + } finally { + db.endTransaction(); + } + + return numInserted; + } + private Uri insertTrackPoint(Uri url, ContentValues values) { - Log.d(MyTracksProvider.TAG, "MyTracksProvider.insertTrackPoint"); boolean hasLat = values.containsKey(TrackPointsColumns.LATITUDE); boolean hasLong = values.containsKey(TrackPointsColumns.LONGITUDE); boolean hasTime = values.containsKey(TrackPointsColumns.TIME); diff --git a/MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProviderUtils.java b/MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProviderUtils.java index 5764b489b..2c5c9e60a 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProviderUtils.java +++ b/MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProviderUtils.java @@ -232,6 +232,17 @@ public interface MyTracksProviderUtils { */ Uri insertTrackPoint(Location location, long trackId); + /** + * Inserts multiple track points in a single operation. + * + * @param locations an array of locations to insert + * @param length the number of locations (from the beginning of the array) + * to actually insert, or -1 for all of them + * @param trackId the ID of the track to insert the points into + * @return the number of points inserted + */ + int bulkInsertTrackPoints(Location[] locations, int length, long trackId); + /** * Inserts a waypoint in the provider. * @@ -262,7 +273,7 @@ public interface MyTracksProviderUtils { * @param cursor a cursor pointing at a db or provider with locations * @return a new location object */ - public Location createLocation(Cursor cursor); + Location createLocation(Cursor cursor); /** * Creates a waypoint object from a given cursor. @@ -270,7 +281,7 @@ public interface MyTracksProviderUtils { * @param cursor a cursor pointing at a db or provider with waypoints. * @return a new waypoint object */ - public Waypoint createWaypoint(Cursor cursor); + Waypoint createWaypoint(Cursor cursor); /** * A factory which can produce instances of {@link MyTracksProviderUtils}, diff --git a/MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProviderUtilsImpl.java b/MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProviderUtilsImpl.java index 9244dccba..8b13603a6 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProviderUtilsImpl.java +++ b/MyTracks/src/com/google/android/apps/mytracks/content/MyTracksProviderUtilsImpl.java @@ -840,8 +840,6 @@ public class MyTracksProviderUtilsImpl implements MyTracksProviderUtils { Cursor cursor = getLocationsCursor(track.getId(), startingPoint, buffer.getSize(), false); - final int idColumnIdx = - cursor.getColumnIndexOrThrow(TrackPointsColumns._ID); if (cursor == null) { Log.w(MyTracksProvider.TAG, "Cannot get a locations cursor!"); buffer.setInvalid(); @@ -860,6 +858,8 @@ public class MyTracksProviderUtilsImpl implements MyTracksProviderUtils { return; } + final int idColumnIdx = + cursor.getColumnIndexOrThrow(TrackPointsColumns._ID); do { Location location = createLocation(cursor); if (location == null) { @@ -900,6 +900,18 @@ public class MyTracksProviderUtilsImpl implements MyTracksProviderUtils { createContentValues(location, trackId)); } + @Override + public int bulkInsertTrackPoints(Location[] locations, int length, long trackId) { + if (length == -1) { length = locations.length; } + + ContentValues[] values = new ContentValues[length]; + for (int i = 0; i < length; i++) { + values[i] = createContentValues(locations[i], trackId); + } + + return context.getContentResolver().bulkInsert(TrackPointsColumns.CONTENT_URI, values); + } + @Override public Uri insertWaypoint(Waypoint waypoint) { Log.d(MyTracksProvider.TAG, "MyTracksProviderUtilsImpl.insertWaypoint"); diff --git a/MyTracks/src/com/google/android/apps/mytracks/io/GpxImporter.java b/MyTracks/src/com/google/android/apps/mytracks/io/GpxImporter.java index 390156b85..c65e735b3 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/io/GpxImporter.java +++ b/MyTracks/src/com/google/android/apps/mytracks/io/GpxImporter.java @@ -15,6 +15,7 @@ */ package com.google.android.apps.mytracks.io; +import com.google.android.apps.mytracks.MyTracksConstants; import com.google.android.apps.mytracks.content.MyTracksProviderUtils; import com.google.android.apps.mytracks.content.Track; import com.google.android.apps.mytracks.stats.TripStatisticsBuilder; @@ -23,6 +24,7 @@ import com.google.android.apps.mytracks.util.MyTracksUtils; import android.location.Location; import android.location.LocationManager; import android.net.Uri; +import android.util.Log; import java.io.IOException; import java.io.InputStream; @@ -52,7 +54,7 @@ import org.xml.sax.helpers.DefaultHandler; */ public class GpxImporter extends DefaultHandler { - /** + /* * Different date formats used in GPX files */ static final SimpleDateFormat DATE_FORMAT1 = new SimpleDateFormat( @@ -63,7 +65,7 @@ public class GpxImporter extends DefaultHandler { "yyyy-MM-dd'T'HH:mm:ss.SSSZ"); static final SimpleTimeZone UTC_TIMEZONE = new SimpleTimeZone(0, "UTC"); - /** + /* * GPX-XML tag names and attributes. */ private static final String TAG_TRACK = "trk"; @@ -76,6 +78,14 @@ public class GpxImporter extends DefaultHandler { private static final String ATT_LAT = "lat"; private static final String ATT_LON = "lon"; + /** + * The maximum number of locations to buffer for bulk-insertion into the database. + */ + private static final int MAX_BUFFERED_LOCATIONS = 512; + + /** + * Utilities for accessing the contnet provider. + */ private final MyTracksProviderUtils providerUtils; /** @@ -98,12 +108,6 @@ public class GpxImporter extends DefaultHandler { * Previous location, required for calculations. */ private Location lastLocation; - - /** - * URI of the last point inserted into the database. - * We parse point IDs out of this when necessary. - */ - private Uri lastPointIdUri; /** * Currently reading track. @@ -115,6 +119,16 @@ public class GpxImporter extends DefaultHandler { */ private TripStatisticsBuilder statsBuilder; + /** + * Buffer of locations to be bulk-inserted into the database. + */ + private Location[] bufferedPointInserts = new Location[MAX_BUFFERED_LOCATIONS]; + + /** + * Number of locations buffered to be inserted into the database. + */ + private int numBufferedPointInserts = 0; + /** * Number of locations already processed. */ @@ -163,7 +177,13 @@ public class GpxImporter extends DefaultHandler { long[] trackIds = null; try { + long start = System.currentTimeMillis(); + parser.parse(is, handler); + + long end = System.currentTimeMillis(); + Log.d(MyTracksConstants.TAG, "Total import time: " + (end - start) + "ms"); + trackIds = handler.getImportedTrackIds(); } catch (SAXException e) { throw e; @@ -182,7 +202,7 @@ public class GpxImporter extends DefaultHandler { this.providerUtils = providerUtils; tracksWritten = new ArrayList(); } - + @Override public void characters(char[] ch, int start, int length) throws SAXException { String newContent = new String(ch, start, length); @@ -333,7 +353,7 @@ public class GpxImporter extends DefaultHandler { statsBuilder.addLocation(location, location.getTime()); // insert in db - lastPointIdUri = providerUtils.insertTrackPoint(location, track.getId()); + insertTrackPoint(location); // first track point? if (lastLocation == null) { @@ -349,7 +369,23 @@ public class GpxImporter extends DefaultHandler { throw new SAXException(msg); } } - + + protected void insertTrackPoint(Location loc) { + bufferedPointInserts[numBufferedPointInserts] = loc; + numBufferedPointInserts++; + + if (numBufferedPointInserts >= MAX_BUFFERED_LOCATIONS) { + flushPointInserts(); + } + } + + private void flushPointInserts() { + if (numBufferedPointInserts <= 0) { return; } + + providerUtils.bulkInsertTrackPoints(bufferedPointInserts, numBufferedPointInserts, track.getId()); + numBufferedPointInserts = 0; + } + /** * Track segment finished. */ @@ -363,6 +399,8 @@ public class GpxImporter extends DefaultHandler { */ private void onTrackElementEnd() { if (lastLocation != null) { + flushPointInserts(); + // Calculate statistics for the imported track and update statsBuilder.pauseAt(lastLocation.getTime()); track.setStopId(getLastPointId()); @@ -493,7 +531,9 @@ public class GpxImporter extends DefaultHandler { * Returns the ID of the last point inserted into the database. */ private long getLastPointId() { - return Long.parseLong(lastPointIdUri.getLastPathSegment()); + flushPointInserts(); + + return providerUtils.getLastLocationId(track.getId()); } /** diff --git a/MyTracksTest/src/com/google/android/apps/mytracks/io/GpxImporterTest.java b/MyTracksTest/src/com/google/android/apps/mytracks/io/GpxImporterTest.java index b05f001a3..091d2b0f0 100644 --- a/MyTracksTest/src/com/google/android/apps/mytracks/io/GpxImporterTest.java +++ b/MyTracksTest/src/com/google/android/apps/mytracks/io/GpxImporterTest.java @@ -15,17 +15,20 @@ */ package com.google.android.apps.mytracks.io; +import static com.google.android.testing.mocking.AndroidMock.eq; +import static com.google.android.testing.mocking.AndroidMock.expect; + import com.google.android.apps.mytracks.content.MyTracksProviderUtils; -import com.google.android.apps.mytracks.content.Track; -import com.google.android.apps.mytracks.content.TrackPointsColumns; -import com.google.android.apps.mytracks.content.TracksColumns; import com.google.android.apps.mytracks.content.MyTracksProviderUtils.Factory; +import com.google.android.apps.mytracks.content.Track; +import com.google.android.apps.mytracks.content.TracksColumns; import com.google.android.apps.mytracks.testing.TestingProviderUtilsFactory; import com.google.android.testing.mocking.AndroidMock; import com.google.android.testing.mocking.UsesMocks; import android.content.ContentUris; import android.location.Location; +import android.location.LocationManager; import android.net.Uri; import android.test.AndroidTestCase; @@ -33,10 +36,12 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; import java.text.SimpleDateFormat; +import java.util.Arrays; import javax.xml.parsers.ParserConfigurationException; import org.easymock.Capture; +import org.easymock.IArgumentMatcher; import org.xml.sax.SAXException; /** @@ -78,14 +83,10 @@ public class GpxImporterTest extends AndroidTestCase { private static final long TRACK_ID = 1; private static final long TRACK_POINT_ID_1 = 1; - private static final long TRACK_POINT_ID_2 = 1; + private static final long TRACK_POINT_ID_2 = 2; private static final Uri TRACK_ID_URI = ContentUris.appendId( TracksColumns.CONTENT_URI.buildUpon(), TRACK_ID).build(); - private static final Uri TRACK_POINT_ID_URI_1 = ContentUris.appendId( - TrackPointsColumns.CONTENT_URI.buildUpon(), TRACK_POINT_ID_1).build(); - private static final Uri TRACK_POINT_ID_URI_2 = ContentUris.appendId( - TrackPointsColumns.CONTENT_URI.buildUpon(), TRACK_POINT_ID_2).build(); private MyTracksProviderUtils providerUtils; @@ -95,6 +96,7 @@ public class GpxImporterTest extends AndroidTestCase { @Override protected void setUp() throws Exception { super.setUp(); + providerUtils = AndroidMock.createMock(MyTracksProviderUtils.class); oldProviderUtilsFactory = TestingProviderUtilsFactory.installWithInstance(providerUtils); @@ -111,20 +113,31 @@ public class GpxImporterTest extends AndroidTestCase { */ public void testImportSuccess() throws Exception { Capture trackParam = new Capture(); - Capture locParam1 = new MyLocationCapture(); - Capture locParam2 = new MyLocationCapture(); - AndroidMock.expect( - providerUtils.insertTrack(AndroidMock.capture(trackParam))).andReturn( - TRACK_ID_URI); + SimpleDateFormat format = GpxImporter.DATE_FORMAT2; + Location loc1 = new Location(LocationManager.GPS_PROVIDER); + loc1.setTime(format.parse(TRACK_TIME_1).getTime()); + loc1.setLatitude(Double.parseDouble(TRACK_LAT_1)); + loc1.setLongitude(Double.parseDouble(TRACK_LON_1)); + loc1.setAltitude(Double.parseDouble(TRACK_ELE_1)); - AndroidMock.expect( - providerUtils.insertTrackPoint(AndroidMock.capture(locParam1), - AndroidMock.anyLong())).andReturn(TRACK_POINT_ID_URI_1); + Location loc2 = new Location(LocationManager.GPS_PROVIDER); + loc2.setTime(format.parse(TRACK_TIME_2).getTime()); + loc2.setLatitude(Double.parseDouble(TRACK_LAT_2)); + loc2.setLongitude(Double.parseDouble(TRACK_LON_2)); + loc2.setAltitude(Double.parseDouble(TRACK_ELE_2)); - AndroidMock.expect( - providerUtils.insertTrackPoint(AndroidMock.capture(locParam2), - AndroidMock.anyLong())).andReturn(TRACK_POINT_ID_URI_2); + expect(providerUtils.insertTrack(AndroidMock.capture(trackParam))) + .andReturn(TRACK_ID_URI); + + expect(providerUtils.getLastLocationId(TRACK_ID)).andReturn(TRACK_POINT_ID_1).andReturn(TRACK_POINT_ID_2); + + // A flush happens after the first insertion to get the starting point ID, + // which is why we get two calls + expect(providerUtils.bulkInsertTrackPoints(LocationsMatcher.eqLoc(loc1), + eq(1), eq(TRACK_ID))).andReturn(1); + expect(providerUtils.bulkInsertTrackPoints(LocationsMatcher.eqLoc(loc2), + eq(1), eq(TRACK_ID))).andReturn(1); providerUtils.updateTrack(AndroidMock.capture(trackParam)); @@ -133,9 +146,7 @@ public class GpxImporterTest extends AndroidTestCase { InputStream is = new ByteArrayInputStream(VALID_TEST_GPX.getBytes()); GpxImporter.importGPXFile(is, providerUtils); - AndroidMock.verify(); - - SimpleDateFormat format = GpxImporter.DATE_FORMAT2; + AndroidMock.verify(providerUtils); // verify track parameter Track track = trackParam.getValue(); @@ -145,19 +156,6 @@ public class GpxImporterTest extends AndroidTestCase { .getStartTime()); assertNotSame(-1, track.getStartId()); assertNotSame(-1, track.getStopId()); - - // verify last location parameter - Location loc1 = locParam1.getValue(); - assertEquals(Double.parseDouble(TRACK_LAT_1), loc1.getLatitude()); - assertEquals(Double.parseDouble(TRACK_LON_1), loc1.getLongitude()); - assertEquals(Double.parseDouble(TRACK_ELE_1), loc1.getAltitude()); - assertEquals(format.parse(TRACK_TIME_1).getTime(), loc1.getTime()); - - Location loc2 = locParam2.getValue(); - assertEquals(Double.parseDouble(TRACK_LAT_2), loc2.getLatitude()); - assertEquals(Double.parseDouble(TRACK_LON_2), loc2.getLongitude()); - assertEquals(Double.parseDouble(TRACK_ELE_2), loc2.getAltitude()); - assertEquals(format.parse(TRACK_TIME_2).getTime(), loc2.getTime()); } /** @@ -186,13 +184,12 @@ public class GpxImporterTest extends AndroidTestCase { private void testInvalidXML(String xml) throws ParserConfigurationException, IOException { - AndroidMock.expect( - providerUtils.insertTrack((Track) AndroidMock.anyObject())).andReturn( - TRACK_ID_URI); + expect(providerUtils.insertTrack((Track) AndroidMock.anyObject())) + .andReturn(TRACK_ID_URI); - AndroidMock.expect( - providerUtils.insertTrackPoint((Location) AndroidMock.anyObject(), - AndroidMock.anyLong())).andStubReturn(TRACK_POINT_ID_URI_1); + expect(providerUtils.bulkInsertTrackPoints((Location[]) AndroidMock.anyObject(), + AndroidMock.anyInt(), AndroidMock.anyLong())).andStubReturn(1); + expect(providerUtils.getLastLocationId(TRACK_ID)).andStubReturn(TRACK_POINT_ID_1); providerUtils.deleteTrack(TRACK_ID); @@ -205,7 +202,7 @@ public class GpxImporterTest extends AndroidTestCase { // expected exception } - AndroidMock.verify(); + AndroidMock.verify(providerUtils); } /** @@ -213,13 +210,49 @@ public class GpxImporterTest extends AndroidTestCase { * http://sourceforge.net * /tracker/?func=detail&aid=2617107&group_id=82958&atid=567837 */ - @SuppressWarnings("serial") - class MyLocationCapture extends Capture { + private static class LocationsMatcher implements IArgumentMatcher { + private final Location[] matchLocs; + + private LocationsMatcher(Location[] expected) { + this.matchLocs = expected; + } + + public static Location[] eqLoc(Location[] expected) { + IArgumentMatcher matcher = new LocationsMatcher(expected); + AndroidMock.reportMatcher(matcher); + return null; + } + + public static Location[] eqLoc(Location expected) { + return eqLoc(new Location[] { expected}); + } + @Override - public void setValue(Location value) { - if (!hasCaptured()) { - super.setValue(value); + public void appendTo(StringBuffer buf) { + buf.append("eqLoc(").append(Arrays.toString(matchLocs)).append(")"); + } + + @Override + public boolean matches(Object obj) { + if (! (obj instanceof Location[])) { return false; } + Location[] locs = (Location[]) obj; + if (locs.length < matchLocs.length) { return false; } + + // Only check the first elements (those that will be taken into account) + for (int i = 0; i < matchLocs.length; i++) { + if (!locationsMatch(locs[i], matchLocs[i])) { + return false; + } } + + return true; + } + + private boolean locationsMatch(Location loc1, Location loc2) { + return (loc1.getTime() == loc2.getTime()) && + (loc1.getLatitude() == loc2.getLatitude()) && + (loc1.getLongitude() == loc2.getLongitude()) && + (loc1.getAltitude() == loc2.getAltitude()); } } }