From 2d760a7bff71c46c5aeba37c40d236ea16eefea2 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sun, 3 Apr 2016 17:31:59 -0500 Subject: 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 Signed-off-by: Dirk Hohndel --- 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 3c9518247..eddf90b98 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 6d46a49c7..6c9ec3f96 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(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 e4b03ebcb..35a9594a3 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(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 048e787cf..c6e6c5025 100644 --- a/qt-mobile/qmlmanager.cpp +++ b/qt-mobile/qmlmanager.cpp @@ -447,6 +447,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 12154704f..ce730da28 100644 --- a/subsurface-core/dive.c +++ b/subsurface-core/dive.c @@ -156,6 +156,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) @@ -464,6 +465,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); @@ -474,11 +476,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; @@ -3247,6 +3248,7 @@ void shift_times(const timestamp_t amount) if (!dive->selected) continue; dive->when += amount; + invalidate_dive_cache(dive); } } @@ -3410,6 +3412,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) @@ -3441,6 +3444,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); } } } @@ -3475,6 +3479,7 @@ void dive_remove_picture(char *filename) struct picture *temp = (*picture)->next; picture_free(*picture); *picture = temp; + invalidate_dive_cache(current_dive); } } @@ -3498,6 +3503,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 */ @@ -3537,6 +3543,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 204d34819..ae24b7409 100644 --- a/subsurface-core/dive.h +++ b/subsurface-core/dive.h @@ -339,8 +339,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 a78082c07..a1f2d2031 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 693abb623..e22019ab0 100644 --- a/subsurface-core/save-git.c +++ b/subsurface-core/save-git.c @@ -622,7 +622,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 }; @@ -631,6 +631,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); @@ -747,7 +763,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; @@ -781,7 +797,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; @@ -897,7 +913,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; @@ -946,11 +962,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; } @@ -1176,6 +1192,7 @@ 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"); @@ -1183,6 +1200,12 @@ int do_git_save(git_repository *repo, const char *branch, const char *remote, bo if (!create_empty) // so we are actually saving the dives git_storage_update_progress(19, "start git save"); + /* + * 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; @@ -1191,7 +1214,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 (verbose) -- cgit v1.2.3-70-g09d2