From 7bb251dad81734323fae24d4f030c9c1cca9d8eb Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 22 May 2023 19:45:02 +0200 Subject: [PATCH] db/update/Walk: use marker to remove deleted items This eliminates all `Storage::GetInfo()` calls from `UpdateWalk::PurgeDeletedFromDirectory()` and instead uses a "marker" field to mark items that have been visited; later, all unmarked items can be deleted. This eliminates a lot of redundant I/O which is noticable with the `curl` storage plugin (i.e. WebDAV). --- NEWS | 2 ++ src/db/PlaylistInfo.hxx | 6 ++++++ src/db/PlaylistVector.cxx | 5 ++++- src/db/plugins/simple/Directory.hxx | 6 ++++++ src/db/plugins/simple/Song.hxx | 6 ++++++ src/db/update/Archive.cxx | 3 ++- src/db/update/UpdateSong.cxx | 17 ++++++++--------- src/db/update/VirtualDirectory.cxx | 2 ++ src/db/update/Walk.cxx | 27 ++++++++++++++++++++------- 9 files changed, 56 insertions(+), 18 deletions(-) diff --git a/NEWS b/NEWS index 1c898d2c2..19aa3b898 100644 --- a/NEWS +++ b/NEWS @@ -14,6 +14,8 @@ ver 0.24 (not yet released) - proxy: require libmpdclient 2.15 or later * archive - add option to disable archive plugins in mpd.conf +* storage + - curl: optimize database update * input - curl: add "connect_timeout" configuration * decoder diff --git a/src/db/PlaylistInfo.hxx b/src/db/PlaylistInfo.hxx index a52b0cedc..6fa36b3c8 100644 --- a/src/db/PlaylistInfo.hxx +++ b/src/db/PlaylistInfo.hxx @@ -24,6 +24,12 @@ struct PlaylistInfo { std::chrono::system_clock::time_point mtime = std::chrono::system_clock::time_point::min(); + /** + * This field is used by the database update to check whether + * an item has disappeared. + */ + bool mark; + class CompareName { const std::string_view name; diff --git a/src/db/PlaylistVector.cxx b/src/db/PlaylistVector.cxx index 08137e1d9..2a96b7014 100644 --- a/src/db/PlaylistVector.cxx +++ b/src/db/PlaylistVector.cxx @@ -27,8 +27,11 @@ PlaylistVector::UpdateOrInsert(PlaylistInfo &&pi) noexcept return false; i->mtime = pi.mtime; - } else + i->mark = true; + } else { + pi.mark = true; push_back(std::move(pi)); + } return true; } diff --git a/src/db/plugins/simple/Directory.hxx b/src/db/plugins/simple/Directory.hxx index 852c792c6..7500d194b 100644 --- a/src/db/plugins/simple/Directory.hxx +++ b/src/db/plugins/simple/Directory.hxx @@ -75,6 +75,12 @@ struct Directory : IntrusiveListHook<> { */ DatabasePtr mounted_database; + /** + * This field is used by the database update to check whether + * an item has disappeared. + */ + bool mark; + public: Directory(std::string &&_path_utf8, Directory *_parent) noexcept; ~Directory() noexcept; diff --git a/src/db/plugins/simple/Song.hxx b/src/db/plugins/simple/Song.hxx index deb6a1ffd..0342f28c2 100644 --- a/src/db/plugins/simple/Song.hxx +++ b/src/db/plugins/simple/Song.hxx @@ -79,6 +79,12 @@ struct Song : IntrusiveListHook<> { */ bool in_playlist = false; + /** + * This field is used by the database update to check whether + * an item has disappeared. + */ + bool mark; + template Song(F &&_filename, Directory &_parent) noexcept :parent(_parent), filename(std::forward(_filename)) {} diff --git a/src/db/update/Archive.cxx b/src/db/update/Archive.cxx index a78e946c3..9a7168e5c 100644 --- a/src/db/update/Archive.cxx +++ b/src/db/update/Archive.cxx @@ -137,7 +137,6 @@ UpdateWalk::UpdateArchiveFile(Directory &parent, std::string_view name, file = archive_file_open(&plugin, path_fs); } catch (...) { LogError(std::current_exception()); - editor.LockDeleteDirectory(directory); return; } @@ -145,6 +144,8 @@ UpdateWalk::UpdateArchiveFile(Directory &parent, std::string_view name, UpdateArchiveVisitor visitor(*this, *file, *directory); file->Visit(visitor); + + directory->mark = true; } bool diff --git a/src/db/update/UpdateSong.cxx b/src/db/update/UpdateSong.cxx index fdf8de56c..1d695822e 100644 --- a/src/db/update/UpdateSong.cxx +++ b/src/db/update/UpdateSong.cxx @@ -29,17 +29,11 @@ try { FmtError(update_domain, "no read permissions on {}/{}", directory.GetPath(), name); - if (song != nullptr) - editor.LockDeleteSong(directory, song); - return; } if (!(song != nullptr && info.mtime == song->mtime && !walk_discard) && UpdateContainerFile(directory, name, suffix, info)) { - if (song != nullptr) - editor.LockDeleteSong(directory, song); - return; } @@ -55,6 +49,8 @@ try { return; } + new_song->mark = true; + { const ScopeDatabaseLock protect; directory.AddSong(std::move(new_song)); @@ -66,14 +62,17 @@ try { } else if (info.mtime != song->mtime || walk_discard) { FmtNotice(update_domain, "updating {}/{}", directory.GetPath(), name); - if (!song->UpdateFile(storage)) { + if (song->UpdateFile(storage)) + song->mark = true; + else FmtDebug(update_domain, "deleting unrecognized file {}/{}", directory.GetPath(), name); - editor.LockDeleteSong(directory, song); - } modified = true; + } else { + /* not modified */ + song->mark = true; } } catch (...) { FmtError(update_domain, diff --git a/src/db/update/VirtualDirectory.cxx b/src/db/update/VirtualDirectory.cxx index 55a0f71bf..6357a9d8c 100644 --- a/src/db/update/VirtualDirectory.cxx +++ b/src/db/update/VirtualDirectory.cxx @@ -22,6 +22,7 @@ UpdateWalk::MakeVirtualDirectoryIfModified(Directory &parent, std::string_view n directory->device == virtual_device && !walk_discard) { /* not modified */ + directory->mark = true; return nullptr; } @@ -32,6 +33,7 @@ UpdateWalk::MakeVirtualDirectoryIfModified(Directory &parent, std::string_view n directory = parent.MakeChild(name); directory->mtime = info.mtime; directory->device = virtual_device; + directory->mark = true; return directory; } diff --git a/src/db/update/Walk.cxx b/src/db/update/Walk.cxx index dfa8fddd5..9653fffeb 100644 --- a/src/db/update/Walk.cxx +++ b/src/db/update/Walk.cxx @@ -73,6 +73,19 @@ UpdateWalk::RemoveExcludedFromDirectory(Directory &directory, }); } +static void +UnmarkAllIn(Directory &directory) noexcept +{ + for (auto &i : directory.children) + i.mark = false; + + for (auto &i : directory.songs) + i.mark = false; + + for (auto &i : directory.playlists) + i.mark = false; +} + inline void UpdateWalk::PurgeDeletedFromDirectory(Directory &directory) noexcept { @@ -81,8 +94,7 @@ UpdateWalk::PurgeDeletedFromDirectory(Directory &directory) noexcept /* mount points are always preserved */ return; - if (DirectoryExists(storage, child) && - child.IsPluginAvailable()) + if (child.mark) return; /* the directory was deleted (or the plugin which @@ -94,9 +106,7 @@ UpdateWalk::PurgeDeletedFromDirectory(Directory &directory) noexcept }); directory.ForEachSongSafe([&](Song &song){ - if (!directory_child_is_regular(storage, directory, - song.filename) || - !song.IsPluginAvailable()) { + if (!song.mark) { /* the song file was deleted (or the decoder plugin is unavailable) */ @@ -109,7 +119,7 @@ UpdateWalk::PurgeDeletedFromDirectory(Directory &directory) noexcept for (auto i = directory.playlists.begin(), end = directory.playlists.end(); i != end;) { - if (!directory_child_is_regular(storage, directory, i->name)) { + if (!i->mark) { const ScopeDatabaseLock protect; i = directory.playlists.erase(i); } else @@ -343,7 +353,7 @@ UpdateWalk::UpdateDirectory(Directory &directory, if (!child_exclude_list.IsEmpty()) RemoveExcludedFromDirectory(directory, child_exclude_list); - PurgeDeletedFromDirectory(directory); + UnmarkAllIn(directory); const char *name_utf8; while (!cancel && (name_utf8 = reader->Read()) != nullptr) { @@ -370,7 +380,10 @@ UpdateWalk::UpdateDirectory(Directory &directory, UpdateDirectoryChild(directory, child_exclude_list, name_utf8, info2); } + PurgeDeletedFromDirectory(directory); + directory.mtime = info.mtime; + directory.mark = true; return true; }