Bug 1369604: Use HighlightCandidateCursorIndices to cache. r=liuche
☠☠ backed out by f91ecef40cab ☠ ☠
authorMichael Comella <michael.l.comella@gmail.com>
Mon, 24 Jul 2017 18:12:05 -0700
changeset 420490 9e2a5ef546dd9da5325f62de1ffb6336faffde9c
parent 420489 114785959b0c805b921d3cbe1384caa850914339
child 420491 e226cd5d64b254722e0fbfc21fc343b59542d5b8
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersliuche
bugs1369604
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1369604: Use HighlightCandidateCursorIndices to cache. r=liuche This reduces the calls to `getColumnIndexOrThrow` to 9 (from 1.6k) and HighlightsRanking.extractFeatures goes from 77.1% inclusive CPU time -> 40.8%, 14.6k ms -> 7.1k ms. MozReview-Commit-ID: L6HqvBK5I4i
mobile/android/base/java/org/mozilla/gecko/activitystream/Utils.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Highlight.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Metadata.java
mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightCandidate.java
mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightCandidateCursorIndices.java
mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightsRanking.java
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/Utils.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/Utils.java
@@ -1,32 +1,31 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 4; indent-tabs-mode: nil; -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.activitystream;
 
 import android.database.Cursor;
-
-import org.mozilla.gecko.db.BrowserContract;
+import org.mozilla.gecko.activitystream.ranking.HighlightCandidateCursorIndices;
 
 /**
  * Various util methods and constants that are shared by different parts of Activity Stream.
  */
 public class Utils {
     public enum HighlightSource {
         VISITED,
         BOOKMARKED
     }
 
-    public static HighlightSource highlightSource(final Cursor cursor) {
-        if (-1 != cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.BOOKMARK_ID))) {
+    public static HighlightSource highlightSource(final Cursor cursor, final HighlightCandidateCursorIndices cursorIndices) {
+        if (-1 != cursor.getLong(cursorIndices.bookmarkIDColumnIndex)) {
             return HighlightSource.BOOKMARKED;
         }
 
-        if (-1 != cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.HISTORY_ID))) {
+        if (-1 != cursor.getLong(cursorIndices.historyIDColumnIndex)) {
             return HighlightSource.VISITED;
         }
 
         throw new IllegalArgumentException("Unknown highlight source.");
     }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Highlight.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Highlight.java
@@ -3,46 +3,45 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.activitystream.homepanel.model;
 
 import android.database.Cursor;
 import android.support.annotation.Nullable;
 import android.text.format.DateUtils;
-
 import org.mozilla.gecko.activitystream.Utils;
-import org.mozilla.gecko.db.BrowserContract;
+import org.mozilla.gecko.activitystream.ranking.HighlightCandidateCursorIndices;
 
 public class Highlight implements Item {
     private final String title;
     private final String url;
     private final Utils.HighlightSource source;
     private final long time;
 
     private long historyId;
 
     private Metadata metadata;
 
     private @Nullable Boolean isPinned;
     private @Nullable Boolean isBookmarked;
 
-    public static Highlight fromCursor(Cursor cursor) {
-        return new Highlight(cursor);
+    public static Highlight fromCursor(final Cursor cursor, final HighlightCandidateCursorIndices cursorIndices) {
+        return new Highlight(cursor, cursorIndices);
     }
 
-    private Highlight(Cursor cursor) {
-        title = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.History.TITLE));
-        url = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Combined.URL));
-        source = Utils.highlightSource(cursor);
-        time = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.DATE));
+    private Highlight(final Cursor cursor, final HighlightCandidateCursorIndices cursorIndices) {
+        title = cursor.getString(cursorIndices.titleColumnIndex);
+        url = cursor.getString(cursorIndices.urlColumnIndex);
+        source = Utils.highlightSource(cursor, cursorIndices);
+        time = cursor.getLong(cursorIndices.highlightsDateColumnIndex);
 
-        historyId = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.HISTORY_ID));
+        historyId = cursor.getLong(cursorIndices.historyIDColumnIndex);
 
-        metadata = Metadata.fromCursor(cursor);
+        metadata = Metadata.fromCursor(cursor, cursorIndices);
 
         updateState();
     }
 
     private void updateState() {
         // We can only be certain of bookmark state if an item is a bookmark item.
         // Otherwise, due to the underlying highlights query, we have to look up states when
         // menus are displayed.
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Metadata.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Metadata.java
@@ -3,27 +3,25 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.activitystream.homepanel.model;
 
 import android.database.Cursor;
 import android.text.TextUtils;
 import android.util.Log;
-
 import org.json.JSONException;
 import org.json.JSONObject;
-import org.mozilla.gecko.db.BrowserContract;
+import org.mozilla.gecko.activitystream.ranking.HighlightCandidateCursorIndices;
 
 public class Metadata {
     private static final String LOGTAG = "GeckoMetadata";
 
-    public static Metadata fromCursor(Cursor cursor) {
-        return new Metadata(
-                cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.METADATA)));
+    public static Metadata fromCursor(final Cursor cursor, final HighlightCandidateCursorIndices cursorIndices) {
+        return new Metadata(cursor.getString(cursorIndices.metadataColumnIndex));
     }
 
     private String provider;
     private String imageUrl;
     private int descriptionLength;
 
     private Metadata(String json) {
         if (TextUtils.isEmpty(json)) {
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightCandidate.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightCandidate.java
@@ -7,17 +7,16 @@ package org.mozilla.gecko.activitystream
 
 import android.database.Cursor;
 import android.net.Uri;
 import android.support.annotation.Nullable;
 import android.support.annotation.StringDef;
 import android.support.annotation.VisibleForTesting;
 
 import org.mozilla.gecko.activitystream.ranking.RankingUtils.Func1;
-import org.mozilla.gecko.db.BrowserContract;
 import org.mozilla.gecko.activitystream.homepanel.model.Highlight;
 
 import java.util.HashMap;
 import java.util.Map;
 
 /**
  * A highlight candidate (Highlight object + features). Ranking will determine whether this is an
  * actual highlight.
@@ -39,44 +38,47 @@ import java.util.Map;
     public @interface Feature {}
 
     @VisibleForTesting final Map<String, Double> features;
     private Highlight highlight;
     private @Nullable String imageUrl;
     private String host;
     private double score;
 
-    public static HighlightCandidate fromCursor(Cursor cursor) throws InvalidHighlightCandidateException {
+    public static HighlightCandidate fromCursor(final Cursor cursor, final HighlightCandidateCursorIndices cursorIndices)
+            throws InvalidHighlightCandidateException {
         final HighlightCandidate candidate = new HighlightCandidate();
 
-        extractHighlight(candidate, cursor);
-        extractFeatures(candidate, cursor);
+        extractHighlight(candidate, cursor, cursorIndices);
+        extractFeatures(candidate, cursor, cursorIndices);
 
         return candidate;
     }
 
     /**
      * Extract highlight object from cursor.
      */
-    private static void extractHighlight(HighlightCandidate candidate, Cursor cursor) {
-        candidate.highlight = Highlight.fromCursor(cursor);
+    private static void extractHighlight(final HighlightCandidate candidate, final Cursor cursor,
+            final HighlightCandidateCursorIndices cursorIndices) {
+        candidate.highlight = Highlight.fromCursor(cursor, cursorIndices);
     }
 
     /**
      * Extract and assign features that will be used for ranking.
      */
-    private static void extractFeatures(HighlightCandidate candidate, Cursor cursor) throws InvalidHighlightCandidateException {
+    private static void extractFeatures(final HighlightCandidate candidate, final Cursor cursor,
+            final HighlightCandidateCursorIndices cursorIndices) throws InvalidHighlightCandidateException {
         candidate.features.put(
                 FEATURE_AGE_IN_DAYS,
-                (System.currentTimeMillis() - cursor.getDouble(cursor.getColumnIndexOrThrow(BrowserContract.History.DATE_LAST_VISITED)))
+                (System.currentTimeMillis() - cursor.getDouble(cursorIndices.historyDateLastVisitedColumnIndex))
                         / (1000 * 3600 * 24));
 
         candidate.features.put(
                 FEATURE_VISITS_COUNT,
-                cursor.getDouble(cursor.getColumnIndexOrThrow(BrowserContract.History.VISITS)));
+                cursor.getDouble(cursorIndices.visitsColumnIndex));
 
         // Until we can determine those numbers we assume this domain has only been visited once
         // and the cursor returned all database entries.
         // TODO: Calculate values based using domain hash field (bug 1335817)
         final int occurrences = 1; // Number of times host shows up in history (Bug 1319485)
         final int domainCountSize = cursor.getCount(); // Number of domains visited (Bug 1319485)
 
         candidate.features.put(
@@ -103,25 +105,24 @@ import java.util.Map;
 
         // Historical note: before Bug 1335198, this value was not really the time at which the
         // bookmark was created by the user. Especially synchronized bookmarks could have a recent
         // value but have been bookmarked a long time ago.
         // Current behaviour: synchronized clients will, over time, converge DATE_CREATED field
         // to the real creation date, or the earliest one mentioned in the clients constellation.
         // We are sourcing highlights from the recent visited history - so in order to
         // show up this bookmark need to have been visited recently too.
-        final int bookmarkDateColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.DATE_CREATED);
-        if (cursor.isNull(bookmarkDateColumnIndex)) {
+        if (cursor.isNull(cursorIndices.bookmarkDateCreatedColumnIndex)) {
             candidate.features.put(
                     FEATURE_BOOKMARK_AGE_IN_MILLISECONDS,
                     0d);
         } else {
             candidate.features.put(
                     FEATURE_BOOKMARK_AGE_IN_MILLISECONDS,
-                    Math.max(1, System.currentTimeMillis() - cursor.getDouble(bookmarkDateColumnIndex)));
+                    Math.max(1, System.currentTimeMillis() - cursor.getDouble(cursorIndices.bookmarkDateCreatedColumnIndex)));
         }
 
         candidate.features.put(
                 FEATURE_DESCRIPTION_LENGTH,
                 (double) candidate.highlight.getMetadata().getDescriptionLength());
 
         final Uri uri = Uri.parse(candidate.highlight.getUrl());
 
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightCandidateCursorIndices.java
@@ -0,0 +1,45 @@
+/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.gecko.activitystream.ranking;
+
+import android.database.Cursor;
+import org.mozilla.gecko.db.BrowserContract;
+
+/**
+ * A cache of the column indices of the given Cursor.
+ *
+ * We use this for performance reasons: {@link Cursor#getColumnIndexOrThrow(String)} will do a HashMap look-up and
+ * String comparison each time it's called, which gets expensive while iterating through HighlightCandidate results
+ * (currently a maximum of 500 items), so we cache the values.
+ */
+public class HighlightCandidateCursorIndices {
+
+    public final int titleColumnIndex;
+    public final int urlColumnIndex;
+    public final int visitsColumnIndex;
+    public final int metadataColumnIndex;
+
+    public final int highlightsDateColumnIndex;
+    public final int bookmarkDateCreatedColumnIndex;
+    public final int historyDateLastVisitedColumnIndex;
+
+    public final int historyIDColumnIndex;
+    public final int bookmarkIDColumnIndex;
+
+    HighlightCandidateCursorIndices(final Cursor cursor) {
+        titleColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.History.TITLE);
+        urlColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Combined.URL);
+        visitsColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.History.VISITS);
+        metadataColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Highlights.METADATA);
+
+        highlightsDateColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Highlights.DATE);
+        bookmarkDateCreatedColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.DATE_CREATED);
+        historyDateLastVisitedColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.History.DATE_LAST_VISITED);
+
+        historyIDColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Highlights.HISTORY_ID);
+        bookmarkIDColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Highlights.BOOKMARK_ID);
+    }
+}
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightsRanking.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightsRanking.java
@@ -2,18 +2,18 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.activitystream.ranking;
 
 import android.database.Cursor;
 import android.support.annotation.VisibleForTesting;
+
 import android.util.Log;
-
 import org.mozilla.gecko.activitystream.homepanel.model.Highlight;
 
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -91,22 +91,24 @@ public class HighlightsRanking {
 
         return createHighlightsList(highlights, limit);
     }
 
     /**
      * Extract features for every candidate. The heavy lifting is done in
      * HighlightCandidate.fromCursor().
      */
-    @VisibleForTesting static List<HighlightCandidate> extractFeatures(Cursor cursor) {
+    @VisibleForTesting static List<HighlightCandidate> extractFeatures(final Cursor cursor) {
+        // Cache column indices for performance: see class Javadoc for more info.
+        final HighlightCandidateCursorIndices cursorIndices = new HighlightCandidateCursorIndices(cursor);
         return looselyMapCursor(cursor, new Func1<Cursor, HighlightCandidate>() {
             @Override
             public HighlightCandidate call(Cursor cursor) {
                 try {
-                    return HighlightCandidate.fromCursor(cursor);
+                    return HighlightCandidate.fromCursor(cursor, cursorIndices);
                 } catch (HighlightCandidate.InvalidHighlightCandidateException e) {
                     Log.w(LOG_TAG, "Skipping invalid highlight item", e);
                     return null;
                 }
             }
         });
     }