[PATCH 1/1] Don't write back dive data that hasn't changed in git
Linus Torvalds
torvalds at linux-foundation.org
Mon Apr 4 16:35:20 PDT 2016
From: Linus Torvalds <torvalds at linux-foundation.org>
Date: Sun, 3 Apr 2016 17:31:59 -0500
Subject: [PATCH 1/1] Don't write back dive data that hasn't changed in git
This caches the git ID for the dive on load, and avoids building the
dive directory and hashing it on save as long as nothing has invalidated
the git ID cache.
That should make it much faster to write back data to the git
repository, since the dive tree structure and the divecomputer blobs in
particular are the bulk of it (due to all the sample data). It's not
actually the git operations that are all that expensive, it's literally
generating the big blob with all the snprintf() calls for the data.
The git save used to be a fairly expensive with large data sets,
especially noticeable on mobile with much weaker CPU's. This should
speed things up by at least a factor of two.
Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
---
NOTE! as mentioned, this has room for future expansion, to do things a
trip at a time etc. And right now when we invalidate a dive cache, it
*stays* invalidated - we could re-validate it again when we save it etc
etc.
But even in this simplistic form, it does cut down on the work we do at
saving a lot, and the common case is that you don't change very many
dives, so this is pretty efficient at cutting down the extra work.
desktop-widgets/maintab.cpp | 4 +++-
desktop-widgets/simplewidgets.cpp | 8 ++++++--
profile-widget/profilewidget2.cpp | 4 ++++
qt-mobile/qmlmanager.cpp | 1 +
subsurface-core/dive.c | 15 +++++++++++----
subsurface-core/dive.h | 13 +++++++++++++
subsurface-core/load-git.c | 11 +++++++++--
subsurface-core/save-git.c | 37 ++++++++++++++++++++++++++++++-------
8 files changed, 77 insertions(+), 16 deletions(-)
diff --git a/desktop-widgets/maintab.cpp b/desktop-widgets/maintab.cpp
index f1d14e8b7544..26c5eb36a640 100644
--- a/desktop-widgets/maintab.cpp
+++ b/desktop-widgets/maintab.cpp
@@ -1049,8 +1049,10 @@ void MainTab::acceptChanges()
// each dive that was selected might have had the temperatures in its active divecomputer changed
// so re-populate the temperatures - easiest way to do this is by calling fixup_dive
for_each_dive (i, d) {
- if (d->selected)
+ if (d->selected) {
fixup_dive(d);
+ invalidate_dive_cache(d);
+ }
}
}
if (editMode != TRIP && current_dive->divetrip) {
diff --git a/desktop-widgets/simplewidgets.cpp b/desktop-widgets/simplewidgets.cpp
index 6d46a49c7252..6c9ec3f96aa8 100644
--- a/desktop-widgets/simplewidgets.cpp
+++ b/desktop-widgets/simplewidgets.cpp
@@ -153,8 +153,10 @@ void RenumberDialog::buttonClicked(QAbstractButton *button)
int newNr = ui.spinBox->value();
struct dive *dive = NULL;
for_each_dive (i, dive) {
- if (!selectedOnly || dive->selected)
+ if (!selectedOnly || dive->selected) {
+ invalidate_dive_cache(dive);
renumberedDives.insert(dive->id, QPair<int,int>(dive->number, newNr++));
+ }
}
UndoRenumberDives *undoCommand = new UndoRenumberDives(renumberedDives);
MainWindow::instance()->undoStack->push(undoCommand);
@@ -189,8 +191,10 @@ void SetpointDialog::setpointData(struct divecomputer *divecomputer, int second)
void SetpointDialog::buttonClicked(QAbstractButton *button)
{
- if (ui.buttonBox->buttonRole(button) == QDialogButtonBox::AcceptRole && dc)
+ if (ui.buttonBox->buttonRole(button) == QDialogButtonBox::AcceptRole && dc) {
add_event(dc, time, SAMPLE_EVENT_PO2, 0, (int)(1000.0 * ui.spinbox->value()), "SP change");
+ invalidate_dive_cache(current_dive);
+ }
mark_divelist_changed(true);
MainWindow::instance()->graphics()->replot();
}
diff --git a/profile-widget/profilewidget2.cpp b/profile-widget/profilewidget2.cpp
index e4b03ebcbccb..35a9594a38a5 100644
--- a/profile-widget/profilewidget2.cpp
+++ b/profile-widget/profilewidget2.cpp
@@ -1515,6 +1515,7 @@ void ProfileWidget2::removeEvent()
tr("%1 @ %2:%3").arg(event->name).arg(event->time.seconds / 60).arg(event->time.seconds % 60, 2, 10, QChar('0'))),
QMessageBox::Ok | QMessageBox::Cancel) == QMessageBox::Ok) {
remove_event(event);
+ invalidate_dive_cache(current_dive);
mark_divelist_changed(true);
replot();
}
@@ -1525,6 +1526,7 @@ void ProfileWidget2::addBookmark()
QAction *action = qobject_cast<QAction *>(sender());
QPointF scenePos = mapToScene(mapFromGlobal(action->data().toPoint()));
add_event(current_dc, timeAxis->valueAt(scenePos), SAMPLE_EVENT_BOOKMARK, 0, 0, "bookmark");
+ invalidate_dive_cache(current_dive);
mark_divelist_changed(true);
replot();
}
@@ -1575,6 +1577,7 @@ void ProfileWidget2::changeGas()
add_gas_switch_event(&displayed_dive, get_dive_dc(&displayed_dive, dc_number), seconds, tank);
// this means we potentially have a new tank that is being used and needs to be shown
fixup_dive(&displayed_dive);
+ invalidate_dive_cache(current_dive);
// FIXME - this no longer gets written to the dive list - so we need to enableEdition() here
@@ -1647,6 +1650,7 @@ void ProfileWidget2::editName()
// and will be freed as part of changing the name!
update_event_name(current_dive, event, newName.toUtf8().data());
update_event_name(&displayed_dive, event, newName.toUtf8().data());
+ invalidate_dive_cache(current_dive);
mark_divelist_changed(true);
replot();
}
diff --git a/qt-mobile/qmlmanager.cpp b/qt-mobile/qmlmanager.cpp
index 96ab5421c43c..8dc7b75482ff 100644
--- a/qt-mobile/qmlmanager.cpp
+++ b/qt-mobile/qmlmanager.cpp
@@ -397,6 +397,7 @@ void QMLManager::commitChanges(QString diveId, QString date, QString location, Q
bool diveChanged = false;
bool needResort = false;
+ invalidate_dive_cache(d);
if (date != get_dive_date_string(d->when)) {
diveChanged = needResort = true;
QDateTime newDate;
diff --git a/subsurface-core/dive.c b/subsurface-core/dive.c
index 6d899e2a1819..ecddda6b9c81 100644
--- a/subsurface-core/dive.c
+++ b/subsurface-core/dive.c
@@ -138,6 +138,7 @@ void update_event_name(struct dive *d, struct event *event, char *name)
*removep = (*removep)->next;
add_event(dc, event->time.seconds, event->type, event->flags, event->value, name);
free(remove);
+ invalidate_dive_cache(d);
}
void add_extra_data(struct divecomputer *dc, const char *key, const char *value)
@@ -450,6 +451,7 @@ void copy_dive(struct dive *s, struct dive *d)
* relevant components that are referenced through pointers,
* so all the strings and the structured lists */
*d = *s;
+ invalidate_dive_cache(d);
d->buddy = copy_string(s->buddy);
d->divemaster = copy_string(s->divemaster);
d->notes = copy_string(s->notes);
@@ -460,11 +462,10 @@ void copy_dive(struct dive *s, struct dive *d)
d->weightsystem[i].description = copy_string(s->weightsystem[i].description);
STRUCTURED_LIST_COPY(struct picture, s->picture_list, d->picture_list, copy_pl);
STRUCTURED_LIST_COPY(struct tag_entry, s->tag_list, d->tag_list, copy_tl);
+
+ // Copy the first dc explicitly, then the list of subsequent dc's
+ copy_dc(&s->dc, &d->dc);
STRUCTURED_LIST_COPY(struct divecomputer, s->dc.next, d->dc.next, copy_dc);
- /* this only copied dive computers 2 and up. The first dive computer is part
- * of the struct dive, so let's make copies of its samples and events */
- copy_samples(&s->dc, &d->dc);
- copy_events(&s->dc, &d->dc);
}
/* make a clone of the source dive and clean out the source dive;
@@ -3258,6 +3259,7 @@ void shift_times(const timestamp_t amount)
if (!dive->selected)
continue;
dive->when += amount;
+ invalidate_dive_cache(dive);
}
}
@@ -3421,6 +3423,7 @@ void dive_create_picture(struct dive *dive, char *filename, int shift_time, bool
dive_add_picture(dive, picture);
dive_set_geodata_from_picture(dive, picture);
+ invalidate_dive_cache(dive);
}
void dive_add_picture(struct dive *dive, struct picture *newpic)
@@ -3452,6 +3455,7 @@ void dive_set_geodata_from_picture(struct dive *dive, struct picture *picture)
ds->longitude = picture->longitude;
} else {
dive->dive_site_uuid = create_dive_site_with_gps("", picture->latitude, picture->longitude, dive->when);
+ invalidate_dive_cache(dive);
}
}
}
@@ -3486,6 +3490,7 @@ void dive_remove_picture(char *filename)
struct picture *temp = (*picture)->next;
picture_free(*picture);
*picture = temp;
+ invalidate_dive_cache(current_dive);
}
}
@@ -3509,6 +3514,7 @@ void make_first_dc()
current_dive->dc = *cur_dc;
current_dive->dc.next = newdc;
free(cur_dc);
+ invalidate_dive_cache(current_dive);
}
/* always acts on the current dive */
@@ -3548,6 +3554,7 @@ void delete_current_divecomputer(void)
}
if (dc_number == count_divecomputers())
dc_number--;
+ invalidate_dive_cache(current_dive);
}
/* helper function to make it easier to work with our structures
diff --git a/subsurface-core/dive.h b/subsurface-core/dive.h
index 9390ffa6c32e..be721348c93e 100644
--- a/subsurface-core/dive.h
+++ b/subsurface-core/dive.h
@@ -338,8 +338,20 @@ struct dive {
int id; // unique ID for this dive
struct picture *picture_list;
int oxygen_cylinder_index, diluent_cylinder_index; // CCR dive cylinder indices
+ unsigned char git_id[20];
};
+static inline void invalidate_dive_cache(struct dive *dive)
+{
+ memset(dive->git_id, 0, 20);
+}
+
+static inline bool dive_cache_is_valid(const struct dive *dive)
+{
+ static const unsigned char null_id[20] = { 0, };
+ return !!memcmp(dive->git_id, null_id, 20);
+}
+
extern int get_cylinder_idx_by_use(struct dive *dive, enum cylinderuse cylinder_use_type);
extern void dc_cylinder_renumber(struct dive *dive, struct divecomputer *dc, int mapping[]);
@@ -755,6 +767,7 @@ extern int nr_weightsystems(struct dive *dive);
extern void add_cylinder_description(cylinder_type_t *);
extern void add_weightsystem_description(weightsystem_t *);
extern void remember_event(const char *eventname);
+extern void invalidate_dive_cache(struct dive *dc);
#if WE_DONT_USE_THIS /* this is a missing feature in Qt - selecting which events to display */
extern int evn_foreach(void (*callback)(const char *, bool *, void *), void *data);
diff --git a/subsurface-core/load-git.c b/subsurface-core/load-git.c
index a78082c0744f..a1f2d203112f 100644
--- a/subsurface-core/load-git.c
+++ b/subsurface-core/load-git.c
@@ -1236,7 +1236,7 @@ static int dive_trip_directory(const char *root, const char *name)
*
* The root path will be of the form yyyy/mm[/tripdir],
*/
-static int dive_directory(const char *root, const char *name, int timeoff)
+static int dive_directory(const char *root, const git_tree_entry *entry, const char *name, int timeoff)
{
int yyyy = -1, mm = -1, dd = -1;
int h, m, s;
@@ -1314,6 +1314,7 @@ static int dive_directory(const char *root, const char *name, int timeoff)
finish_active_dive();
active_dive = create_new_dive(utc_mktime(&tm));
+ memcpy(active_dive->git_id, git_tree_entry_id(entry)->id, 20);
return GIT_WALK_OK;
}
@@ -1412,7 +1413,7 @@ static int walk_tree_directory(const char *root, const git_tree_entry *entry)
* two digits and a dash
*/
if (name[len-3] == ':' || name[len-3] == '=')
- return dive_directory(root, name, len-8);
+ return dive_directory(root, entry, name, len-8);
if (digits != 2)
return GIT_WALK_SKIP;
@@ -1470,6 +1471,12 @@ static int parse_divecomputer_entry(git_repository *repo, const git_tree_entry *
return 0;
}
+/*
+ * NOTE! The "git_id" for the dive is the hash for the whole dive directory.
+ * As such, it covers not just the dive, but the divecomputers and the
+ * pictures too. So if any of the dive computers change, the dive cache
+ * has to be invalidated too.
+ */
static int parse_dive_entry(git_repository *repo, const git_tree_entry *entry, const char *suffix)
{
struct dive *dive = active_dive;
diff --git a/subsurface-core/save-git.c b/subsurface-core/save-git.c
index 4d9cd38e1818..9dc68996f29d 100644
--- a/subsurface-core/save-git.c
+++ b/subsurface-core/save-git.c
@@ -626,7 +626,7 @@ static int save_pictures(git_repository *repo, struct dir *dir, struct dive *div
return 0;
}
-static int save_one_dive(git_repository *repo, struct dir *tree, struct dive *dive, struct tm *tm)
+static int save_one_dive(git_repository *repo, struct dir *tree, struct dive *dive, struct tm *tm, bool cached_ok)
{
struct divecomputer *dc;
struct membuffer buf = { 0 }, name = { 0 };
@@ -635,6 +635,22 @@ static int save_one_dive(git_repository *repo, struct dir *tree, struct dive *di
/* Create dive directory */
create_dive_name(dive, &name, tm);
+
+ /*
+ * If the dive git ID is valid, we just create the whole directory
+ * with that ID
+ */
+ if (cached_ok && dive_cache_is_valid(dive)) {
+ git_oid oid;
+ git_oid_fromraw(&oid, dive->git_id);
+ ret = tree_insert(tree->files, mb_cstring(&name), 1,
+ &oid, GIT_FILEMODE_TREE);
+ free_buffer(&name);
+ if (ret)
+ return report_error("cached dive tree insert failed");
+ return 0;
+ }
+
subdir = new_directory(repo, tree, &name);
subdir->unique = 1;
free_buffer(&name);
@@ -751,7 +767,7 @@ static void verify_shared_date(timestamp_t when, struct tm *tm)
#define MIN_TIMESTAMP (0)
#define MAX_TIMESTAMP (0x7fffffffffffffff)
-static int save_one_trip(git_repository *repo, struct dir *tree, dive_trip_t *trip, struct tm *tm)
+static int save_one_trip(git_repository *repo, struct dir *tree, dive_trip_t *trip, struct tm *tm, bool cached_ok)
{
int i;
struct dive *dive;
@@ -785,7 +801,7 @@ static int save_one_trip(git_repository *repo, struct dir *tree, dive_trip_t *tr
/* Save each dive in the directory */
for_each_dive(i, dive) {
if (dive->divetrip == trip)
- save_one_dive(repo, subdir, dive, tm);
+ save_one_dive(repo, subdir, dive, tm, cached_ok);
}
return 0;
@@ -901,7 +917,7 @@ static void save_divesites(git_repository *repo, struct dir *tree)
}
}
-static int create_git_tree(git_repository *repo, struct dir *root, bool select_only)
+static int create_git_tree(git_repository *repo, struct dir *root, bool select_only, bool cached_ok)
{
int i;
struct dive *dive;
@@ -940,11 +956,11 @@ static int create_git_tree(git_repository *repo, struct dir *root, bool select_o
trip->index = 1;
/* Pass that new subdirectory in for save-trip */
- save_one_trip(repo, tree, trip, &tm);
+ save_one_trip(repo, tree, trip, &tm, cached_ok);
continue;
}
- save_one_dive(repo, tree, dive, &tm);
+ save_one_dive(repo, tree, dive, &tm, cached_ok);
}
return 0;
}
@@ -1170,10 +1186,17 @@ int do_git_save(git_repository *repo, const char *branch, const char *remote, bo
{
struct dir tree;
git_oid id;
+ bool cached_ok;
if (verbose)
fprintf(stderr, "git storage: do git save\n");
+ /*
+ * Check if we can do the cached writes - we need to
+ * have the original git commit we loaded in the repo
+ */
+ cached_ok = try_to_find_parent(saved_git_id, repo);
+
/* Start with an empty tree: no subdirectories, no files */
tree.name[0] = 0;
tree.subdirs = NULL;
@@ -1182,7 +1205,7 @@ int do_git_save(git_repository *repo, const char *branch, const char *remote, bo
if (!create_empty)
/* Populate our tree data structure */
- if (create_git_tree(repo, &tree, select_only))
+ if (create_git_tree(repo, &tree, select_only, cached_ok))
return -1;
if (write_git_tree(repo, &tree, &id))
--
2.8.0.rc4.303.g3931579
More information about the subsurface
mailing list