REAL performance improvement in importer - using a bulk insertion

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

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},

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");

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<Long>();
   }
-
+  
   @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());
   }
 
   /**

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<Track> trackParam = new Capture<Track>();
-    Capture<Location> locParam1 = new MyLocationCapture();
-    Capture<Location> 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<Location> {
+  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());
     }
   }
 }