From e844390614d20bf06297eda1eea959d1e7fdc611 Mon Sep 17 00:00:00 2001 From: Alinson Xavier Date: Mon, 29 May 2017 15:32:45 -0400 Subject: [PATCH 1/4] Improve performance of repeated getTodayValue calls --- .gitignore | 1 + .../org/isoron/uhabits/BaseAndroidTest.java | 16 ++++++ .../uhabits/performance/PerformanceTest.java | 50 +++++++++++++++++ .../isoron/uhabits/models/CheckmarkList.java | 2 +- .../models/sqlite/SQLiteCheckmarkList.java | 56 +++++++++++++------ .../models/sqlite/SQLiteRepetitionList.java | 20 ++++--- .../models/sqlite/SQLiteScoreList.java | 55 ++++++++++++------ .../models/sqlite/SQLiteStreakList.java | 19 ++++--- 8 files changed, 167 insertions(+), 52 deletions(-) create mode 100644 app/src/androidTest/java/org/isoron/uhabits/performance/PerformanceTest.java diff --git a/.gitignore b/.gitignore index 1c7687f38..8b44c8900 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,4 @@ docs/ gen/ local.properties crowdin.yaml +local diff --git a/app/src/androidTest/java/org/isoron/uhabits/BaseAndroidTest.java b/app/src/androidTest/java/org/isoron/uhabits/BaseAndroidTest.java index 912009ece..36f4df5d3 100644 --- a/app/src/androidTest/java/org/isoron/uhabits/BaseAndroidTest.java +++ b/app/src/androidTest/java/org/isoron/uhabits/BaseAndroidTest.java @@ -24,6 +24,7 @@ import android.content.*; import android.os.*; import android.support.annotation.*; import android.support.test.*; +import android.util.*; import org.isoron.uhabits.models.*; import org.isoron.uhabits.preferences.*; @@ -31,6 +32,7 @@ import org.isoron.uhabits.tasks.*; import org.isoron.uhabits.utils.*; import org.junit.*; +import java.io.*; import java.util.*; import java.util.concurrent.*; @@ -132,4 +134,18 @@ public class BaseAndroidTest fail(); } } + + protected void startTracing() + { + File dir = FileUtils.getFilesDir(targetContext, "Profile"); + assertNotNull(dir); + String tracePath = dir.getAbsolutePath() + "/performance.trace"; + Log.d("PerformanceTest", String.format("Saving trace file to %s", tracePath)); + Debug.startMethodTracingSampling(tracePath, 0, 1000); + } + + protected void stopTracing() + { + Debug.stopMethodTracing(); + } } diff --git a/app/src/androidTest/java/org/isoron/uhabits/performance/PerformanceTest.java b/app/src/androidTest/java/org/isoron/uhabits/performance/PerformanceTest.java new file mode 100644 index 000000000..765c4cd3a --- /dev/null +++ b/app/src/androidTest/java/org/isoron/uhabits/performance/PerformanceTest.java @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2017 Álinson Santos Xavier + * + * This file is part of Loop Habit Tracker. + * + * Loop Habit Tracker is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by the + * Free Software Foundation, either version 3 of the License, or (at your + * option) any later version. + * + * Loop Habit Tracker is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + */ + +package org.isoron.uhabits.performance; + +import android.support.test.filters.*; + +import org.isoron.uhabits.*; +import org.isoron.uhabits.models.*; +import org.junit.*; + +@MediumTest +public class PerformanceTest extends BaseAndroidTest +{ + private Habit habit; + + @Override + public void setUp() + { + super.setUp(); + habit = fixtures.createLongHabit(); + } + + @Test(timeout = 1000) + public void testRepeatedGetTodayValue() + { + for (int i = 0; i < 100000; i++) + { + habit.getScores().getTodayValue(); + habit.getCheckmarks().getTodayValue(); + } + } + +} diff --git a/app/src/main/java/org/isoron/uhabits/models/CheckmarkList.java b/app/src/main/java/org/isoron/uhabits/models/CheckmarkList.java index 86488a8d1..cf617628f 100644 --- a/app/src/main/java/org/isoron/uhabits/models/CheckmarkList.java +++ b/app/src/main/java/org/isoron/uhabits/models/CheckmarkList.java @@ -108,7 +108,7 @@ public abstract class CheckmarkList * * @return value of today's checkmark */ - public final int getTodayValue() + public int getTodayValue() { Checkmark today = getToday(); if (today != null) return today.getValue(); diff --git a/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteCheckmarkList.java b/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteCheckmarkList.java index 7c9b552e0..42de31712 100644 --- a/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteCheckmarkList.java +++ b/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteCheckmarkList.java @@ -24,7 +24,6 @@ import android.support.annotation.*; import android.support.annotation.Nullable; import com.activeandroid.*; -import com.activeandroid.query.*; import org.isoron.uhabits.models.*; import org.isoron.uhabits.models.sqlite.records.*; @@ -38,38 +37,54 @@ import java.util.*; */ public class SQLiteCheckmarkList extends CheckmarkList { + @Nullable private HabitRecord habitRecord; @NonNull private final SQLiteUtils sqlite; + @Nullable + private Integer todayValue; + + @NonNull + private final SQLiteStatement invalidateStatement; + + @NonNull + private final SQLiteStatement addStatement; + + @NonNull + private final SQLiteDatabase db; + + private static final String ADD_QUERY = + "insert into Checkmarks(habit, timestamp, value) values (?,?,?)"; + + private static final String INVALIDATE_QUERY = + "delete from Checkmarks where habit = ? and timestamp >= ?"; + public SQLiteCheckmarkList(Habit habit) { super(habit); sqlite = new SQLiteUtils<>(CheckmarkRecord.class); + + db = Cache.openDatabase(); + addStatement = db.compileStatement(ADD_QUERY); + invalidateStatement = db.compileStatement(INVALIDATE_QUERY); } @Override public void add(List checkmarks) { check(habit.getId()); - - String query = - "insert into Checkmarks(habit, timestamp, value) values (?,?,?)"; - - SQLiteDatabase db = Cache.openDatabase(); db.beginTransaction(); try { - SQLiteStatement statement = db.compileStatement(query); - for (Checkmark c : checkmarks) { - statement.bindLong(1, habit.getId()); - statement.bindLong(2, c.getTimestamp()); - statement.bindLong(3, c.getValue()); - statement.execute(); + addStatement.bindLong(1, habit.getId()); + addStatement.bindLong(2, c.getTimestamp()); + addStatement.bindLong(3, c.getValue()); + addStatement.execute(); } db.setTransactionSuccessful(); @@ -115,12 +130,10 @@ public class SQLiteCheckmarkList extends CheckmarkList @Override public void invalidateNewerThan(long timestamp) { - new Delete() - .from(CheckmarkRecord.class) - .where("habit = ?", habit.getId()) - .and("timestamp >= ?", timestamp) - .execute(); - + todayValue = null; + invalidateStatement.bindLong(1, habit.getId()); + invalidateStatement.bindLong(2, timestamp); + invalidateStatement.execute(); observable.notifyListeners(); } @@ -179,4 +192,11 @@ public class SQLiteCheckmarkList extends CheckmarkList for (CheckmarkRecord r : records) checkmarks.add(r.toCheckmark()); return checkmarks; } + + @Override + public int getTodayValue() + { + if(todayValue == null) todayValue = super.getTodayValue(); + return todayValue; + } } diff --git a/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteRepetitionList.java b/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteRepetitionList.java index 6278863e9..758ac993a 100644 --- a/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteRepetitionList.java +++ b/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteRepetitionList.java @@ -19,12 +19,12 @@ package org.isoron.uhabits.models.sqlite; -import android.database.DatabaseUtils; -import android.database.sqlite.SQLiteDatabase; +import android.database.*; +import android.database.sqlite.*; import android.support.annotation.*; import android.support.annotation.Nullable; -import com.activeandroid.Cache; +import com.activeandroid.*; import com.activeandroid.query.*; import org.isoron.uhabits.models.*; @@ -43,10 +43,16 @@ public class SQLiteRepetitionList extends RepetitionList @Nullable private HabitRecord habitRecord; + private SQLiteStatement addStatement; + public SQLiteRepetitionList(@NonNull Habit habit) { super(habit); sqlite = new SQLiteUtils<>(RepetitionRecord.class); + + SQLiteDatabase db = Cache.openDatabase(); + String addQuery = "insert into repetitions(habit, timestamp) values (?,?)"; + addStatement = db.compileStatement(addQuery); } /** @@ -61,11 +67,9 @@ public class SQLiteRepetitionList extends RepetitionList public void add(Repetition rep) { check(habit.getId()); - - RepetitionRecord record = new RepetitionRecord(); - record.copyFrom(rep); - record.habit = habitRecord; - record.save(); + addStatement.bindLong(1, habit.getId()); + addStatement.bindLong(2, rep.getTimestamp()); + addStatement.execute(); observable.notifyListeners(); } diff --git a/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteScoreList.java b/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteScoreList.java index e44e7e6ed..f5c291900 100644 --- a/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteScoreList.java +++ b/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteScoreList.java @@ -24,7 +24,6 @@ import android.support.annotation.*; import android.support.annotation.Nullable; import com.activeandroid.*; -import com.activeandroid.query.*; import org.isoron.uhabits.models.*; import org.isoron.uhabits.models.sqlite.records.*; @@ -37,12 +36,30 @@ import java.util.*; */ public class SQLiteScoreList extends ScoreList { + @Nullable private HabitRecord habitRecord; @NonNull private final SQLiteUtils sqlite; + @Nullable + private Integer todayValue; + + @NonNull + private final SQLiteStatement invalidateStatement; + + @NonNull + private final SQLiteStatement addStatement; + + public static final String ADD_QUERY = + "insert into Score(habit, timestamp, score) values (?,?,?)"; + + public static final String INVALIDATE_QUERY = + "delete from Score where habit = ? " + "and timestamp >= ?"; + + private final SQLiteDatabase db; + /** * Constructs a new ScoreList associated with the given habit. * @@ -52,28 +69,25 @@ public class SQLiteScoreList extends ScoreList { super(habit); sqlite = new SQLiteUtils<>(ScoreRecord.class); + + db = Cache.openDatabase(); + addStatement = db.compileStatement(ADD_QUERY); + invalidateStatement = db.compileStatement(INVALIDATE_QUERY); } @Override public void add(List scores) { check(habit.getId()); - String query = - "insert into Score(habit, timestamp, score) values (?,?,?)"; - - SQLiteDatabase db = Cache.openDatabase(); db.beginTransaction(); - try { - SQLiteStatement statement = db.compileStatement(query); - for (Score s : scores) { - statement.bindLong(1, habit.getId()); - statement.bindLong(2, s.getTimestamp()); - statement.bindLong(3, s.getValue()); - statement.execute(); + addStatement.bindLong(1, habit.getId()); + addStatement.bindLong(2, s.getTimestamp()); + addStatement.bindLong(3, s.getValue()); + addStatement.execute(); } db.setTransactionSuccessful(); @@ -126,12 +140,10 @@ public class SQLiteScoreList extends ScoreList @Override public void invalidateNewerThan(long timestamp) { - new Delete() - .from(ScoreRecord.class) - .where("habit = ?", habit.getId()) - .and("timestamp >= ?", timestamp) - .execute(); - + todayValue = null; + invalidateStatement.bindLong(1, habit.getId()); + invalidateStatement.bindLong(2, timestamp); + invalidateStatement.execute(); getObservable().notifyListeners(); } @@ -204,4 +216,11 @@ public class SQLiteScoreList extends ScoreList for (ScoreRecord r : records) scores.add(r.toScore()); return scores; } + + @Override + public int getTodayValue() + { + if (todayValue == null) todayValue = super.getTodayValue(); + return todayValue; + } } diff --git a/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteStreakList.java b/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteStreakList.java index 8cf10b49f..8d6068579 100644 --- a/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteStreakList.java +++ b/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteStreakList.java @@ -19,10 +19,11 @@ package org.isoron.uhabits.models.sqlite; +import android.database.sqlite.*; import android.support.annotation.*; import android.support.annotation.Nullable; -import com.activeandroid.query.*; +import com.activeandroid.*; import org.isoron.uhabits.models.*; import org.isoron.uhabits.models.sqlite.records.*; @@ -41,10 +42,17 @@ public class SQLiteStreakList extends StreakList @NonNull private final SQLiteUtils sqlite; + private final SQLiteStatement invalidateStatement; + public SQLiteStreakList(Habit habit) { super(habit); sqlite = new SQLiteUtils<>(StreakRecord.class); + + SQLiteDatabase db = Cache.openDatabase(); + String invalidateQuery = "delete from Streak where habit = ? " + + "and end >= ?"; + invalidateStatement = db.compileStatement(invalidateQuery); } @Override @@ -73,12 +81,9 @@ public class SQLiteStreakList extends StreakList @Override public void invalidateNewerThan(long timestamp) { - new Delete() - .from(StreakRecord.class) - .where("habit = ?", habit.getId()) - .and("end >= ?", timestamp - DateUtils.millisecondsInOneDay) - .execute(); - + invalidateStatement.bindLong(1, habit.getId()); + invalidateStatement.bindLong(2, timestamp - DateUtils.millisecondsInOneDay); + invalidateStatement.execute(); observable.notifyListeners(); } From 34ca9d17a23d2ba21614293f2c0100d25839d071 Mon Sep 17 00:00:00 2001 From: Alinson Xavier Date: Tue, 30 May 2017 09:05:05 -0400 Subject: [PATCH 2/4] Make usage of BundleSavedState more robust --- .../common/views/BundleSavedState.java | 29 ++++++++++--------- .../common/views/ScrollableChart.java | 6 ++++ .../habits/list/views/HabitCardListView.java | 6 ++++ 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/org/isoron/uhabits/activities/common/views/BundleSavedState.java b/app/src/main/java/org/isoron/uhabits/activities/common/views/BundleSavedState.java index b5011b03c..6f9e1163d 100644 --- a/app/src/main/java/org/isoron/uhabits/activities/common/views/BundleSavedState.java +++ b/app/src/main/java/org/isoron/uhabits/activities/common/views/BundleSavedState.java @@ -20,24 +20,27 @@ package org.isoron.uhabits.activities.common.views; import android.os.*; +import android.support.v4.os.*; public class BundleSavedState extends android.support.v4.view.AbsSavedState { public static final Parcelable.Creator CREATOR = - new Parcelable.Creator() - { - @Override - public BundleSavedState createFromParcel(Parcel source) + ParcelableCompat.newCreator( + new ParcelableCompatCreatorCallbacks() { - return new BundleSavedState(source, getClass().getClassLoader()); - } + @Override + public BundleSavedState createFromParcel(Parcel source, + ClassLoader loader) + { + return new BundleSavedState(source, loader); + } - @Override - public BundleSavedState[] newArray(int size) - { - return new BundleSavedState[size]; - } - }; + @Override + public BundleSavedState[] newArray(int size) + { + return new BundleSavedState[size]; + } + }); public final Bundle bundle; @@ -50,7 +53,7 @@ public class BundleSavedState extends android.support.v4.view.AbsSavedState public BundleSavedState(Parcel source, ClassLoader loader) { super(source, loader); - this.bundle = source.readBundle(getClass().getClassLoader()); + this.bundle = source.readBundle(loader); } @Override diff --git a/app/src/main/java/org/isoron/uhabits/activities/common/views/ScrollableChart.java b/app/src/main/java/org/isoron/uhabits/activities/common/views/ScrollableChart.java index 8b54b324f..c488cb17a 100644 --- a/app/src/main/java/org/isoron/uhabits/activities/common/views/ScrollableChart.java +++ b/app/src/main/java/org/isoron/uhabits/activities/common/views/ScrollableChart.java @@ -107,6 +107,12 @@ public abstract class ScrollableChart extends View @Override public void onRestoreInstanceState(Parcelable state) { + if(!(state instanceof BundleSavedState)) + { + super.onRestoreInstanceState(state); + return; + } + BundleSavedState bss = (BundleSavedState) state; int x = bss.bundle.getInt("x"); int y = bss.bundle.getInt("y"); diff --git a/app/src/main/java/org/isoron/uhabits/activities/habits/list/views/HabitCardListView.java b/app/src/main/java/org/isoron/uhabits/activities/habits/list/views/HabitCardListView.java index bc784465e..cd6f24dbe 100644 --- a/app/src/main/java/org/isoron/uhabits/activities/habits/list/views/HabitCardListView.java +++ b/app/src/main/java/org/isoron/uhabits/activities/habits/list/views/HabitCardListView.java @@ -153,6 +153,12 @@ public class HabitCardListView extends RecyclerView @Override protected void onRestoreInstanceState(Parcelable state) { + if(!(state instanceof BundleSavedState)) + { + super.onRestoreInstanceState(state); + return; + } + BundleSavedState bss = (BundleSavedState) state; dataOffset = bss.bundle.getInt("dataOffset"); super.onRestoreInstanceState(bss.getSuperState()); From 238a1c724d3207f078d022fdbafb24852a6aaa1b Mon Sep 17 00:00:00 2001 From: Alinson Xavier Date: Tue, 30 May 2017 09:06:50 -0400 Subject: [PATCH 3/4] Bump version and update changelog --- CHANGELOG.md | 5 +++++ app/src/main/AndroidManifest.xml | 4 ++-- gradle/wrapper/gradle-wrapper.properties | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc0407636..307b541c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +### 1.7.3 (May 30, 2017) + +* Improve performance of 'sort by score' +* Other minor bug fixes + ### 1.7.2 (May 27, 2017) * Fix crash at startup diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 7d2b19b95..575e9bd53 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -21,8 +21,8 @@ + android:versionCode="30" + android:versionName="1.7.3"> diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 7dafcc01a..ba0e730da 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -3,4 +3,4 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-3.3-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-3.5-all.zip From b6501c9a29c42d98f3603f0217b6781a9d56f4ff Mon Sep 17 00:00:00 2001 From: Alinson Xavier Date: Tue, 30 May 2017 09:22:15 -0400 Subject: [PATCH 4/4] Fix NullPointerException --- .../main/java/org/isoron/uhabits/models/CheckmarkList.java | 4 +++- .../org/isoron/uhabits/models/memory/MemoryCheckmarkList.java | 2 ++ .../org/isoron/uhabits/models/sqlite/SQLiteCheckmarkList.java | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/isoron/uhabits/models/CheckmarkList.java b/app/src/main/java/org/isoron/uhabits/models/CheckmarkList.java index cf617628f..0556038fd 100644 --- a/app/src/main/java/org/isoron/uhabits/models/CheckmarkList.java +++ b/app/src/main/java/org/isoron/uhabits/models/CheckmarkList.java @@ -192,7 +192,7 @@ public abstract class CheckmarkList Checkmark newest = getNewestComputed(); Checkmark oldest = getOldestComputed(); - if (newest == null) + if (newest == null || oldest == null) { forceRecompute(from, to); } @@ -208,6 +208,7 @@ public abstract class CheckmarkList * * @return oldest checkmark already computed */ + @Nullable protected abstract Checkmark getOldestComputed(); /** @@ -285,5 +286,6 @@ public abstract class CheckmarkList * * @return newest checkmark already computed */ + @Nullable protected abstract Checkmark getNewestComputed(); } diff --git a/app/src/main/java/org/isoron/uhabits/models/memory/MemoryCheckmarkList.java b/app/src/main/java/org/isoron/uhabits/models/memory/MemoryCheckmarkList.java index 5b02be26a..9a566e51f 100644 --- a/app/src/main/java/org/isoron/uhabits/models/memory/MemoryCheckmarkList.java +++ b/app/src/main/java/org/isoron/uhabits/models/memory/MemoryCheckmarkList.java @@ -72,6 +72,7 @@ public class MemoryCheckmarkList extends CheckmarkList } @Override + @Nullable protected Checkmark getOldestComputed() { if(list.isEmpty()) return null; @@ -79,6 +80,7 @@ public class MemoryCheckmarkList extends CheckmarkList } @Override + @Nullable protected Checkmark getNewestComputed() { if(list.isEmpty()) return null; diff --git a/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteCheckmarkList.java b/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteCheckmarkList.java index 42de31712..c63e1e1fd 100644 --- a/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteCheckmarkList.java +++ b/app/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteCheckmarkList.java @@ -153,6 +153,7 @@ public class SQLiteCheckmarkList extends CheckmarkList } @Override + @Nullable protected Checkmark getOldestComputed() { check(habit.getId());