diff --git a/NEWS b/NEWS index 376674c24..5f68398f1 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,7 @@ ver 0.23 (not yet released) ver 0.22.7 (not yet released) * protocol - don't use glibc extension to parse time stamps + - optimize the "albumart" command * input - curl: send user/password in the first request, save one roundtrip * decoder diff --git a/src/client/Client.hxx b/src/client/Client.hxx index 5ddc87a32..516830669 100644 --- a/src/client/Client.hxx +++ b/src/client/Client.hxx @@ -23,6 +23,7 @@ #include "Message.hxx" #include "command/CommandResult.hxx" #include "command/CommandListBuilder.hxx" +#include "input/LastInputStream.hxx" #include "tag/Mask.hxx" #include "event/FullyBufferedSocket.hxx" #include "event/CoarseTimerEvent.hxx" @@ -90,6 +91,13 @@ public: */ size_t binary_limit = 8192; + /** + * This caches the last "albumart" InputStream instance, to + * avoid repeating the search for each chunk requested by this + * client. + */ + LastInputStream last_album_art; + private: static constexpr size_t MAX_SUBSCRIPTIONS = 16; diff --git a/src/client/New.cxx b/src/client/New.cxx index 75019290f..f9633816c 100644 --- a/src/client/New.cxx +++ b/src/client/New.cxx @@ -44,7 +44,8 @@ Client::Client(EventLoop &_loop, Partition &_partition, partition(&_partition), permission(_permission), uid(_uid), - num(_num) + num(_num), + last_album_art(_loop) { timeout_event.Schedule(client_timeout); } diff --git a/src/command/FileCommands.cxx b/src/command/FileCommands.cxx index ce750414c..a09d2bfad 100644 --- a/src/command/FileCommands.cxx +++ b/src/command/FileCommands.cxx @@ -188,9 +188,16 @@ read_stream_art(Response &r, const char *uri, size_t offset) { const auto art_directory = PathTraitsUTF8::GetParent(uri); - Mutex mutex; + // TODO: eliminate this const_cast + auto &client = const_cast(r.GetClient()); - InputStreamPtr is = find_stream_art(art_directory, mutex); + /* to avoid repeating the search for each chunk request by the + same client, use the #LastInputStream class to cache the + #InputStream instance */ + auto *is = client.last_album_art.Open(art_directory, [](std::string_view directory, + Mutex &mutex){ + return find_stream_art(directory, mutex); + }); if (is == nullptr) { r.Error(ACK_ERROR_NO_EXIST, "No file exists"); @@ -216,7 +223,7 @@ read_stream_art(Response &r, const char *uri, size_t offset) std::size_t read_size = 0; if (buffer_size > 0) { - std::unique_lock lock(mutex); + std::unique_lock lock(is->mutex); is->Seek(lock, offset); read_size = is->Read(lock, buffer.get(), buffer_size); } diff --git a/src/command/PlaylistCommands.cxx b/src/command/PlaylistCommands.cxx index 5f424b556..5670f401e 100644 --- a/src/command/PlaylistCommands.cxx +++ b/src/command/PlaylistCommands.cxx @@ -92,7 +92,7 @@ handle_load(Client &client, Request args, [[maybe_unused]] Response &r) auto &instance = client.GetInstance(); const unsigned new_size = playlist.GetLength(); for (unsigned i = old_size; i < new_size; ++i) - instance.LookupRemoteTag(playlist.queue.Get(i).GetURI()); + instance.LookupRemoteTag(playlist.queue.Get(i).GetRealURI()); return CommandResult::OK; } diff --git a/src/input/LastInputStream.cxx b/src/input/LastInputStream.cxx new file mode 100644 index 000000000..40bd5ccc2 --- /dev/null +++ b/src/input/LastInputStream.cxx @@ -0,0 +1,45 @@ +/* + * Copyright 2003-2021 The Music Player Daemon Project + * http://www.musicpd.org + * + * This program 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 2 of the License, or + * (at your option) any later version. + * + * This program 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, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "LastInputStream.hxx" +#include "InputStream.hxx" + +#include + +LastInputStream::LastInputStream(EventLoop &event_loop) noexcept + :close_timer(event_loop, BIND_THIS_METHOD(OnCloseTimer)) +{ +} + +LastInputStream::~LastInputStream() noexcept = default; + +void +LastInputStream::Close() noexcept +{ + is.reset(); + close_timer.Cancel(); +} + +void +LastInputStream::OnCloseTimer() noexcept +{ + assert(is); + + is.reset(); +} diff --git a/src/input/LastInputStream.hxx b/src/input/LastInputStream.hxx new file mode 100644 index 000000000..2121772a0 --- /dev/null +++ b/src/input/LastInputStream.hxx @@ -0,0 +1,86 @@ +/* + * Copyright 2003-2021 The Music Player Daemon Project + * http://www.musicpd.org + * + * This program 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 2 of the License, or + * (at your option) any later version. + * + * This program 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, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifndef MPD_LAST_INPUT_STREAM_HXX +#define MPD_LAST_INPUT_STREAM_HXX + +#include "Ptr.hxx" +#include "thread/Mutex.hxx" +#include "event/TimerEvent.hxx" + +#include + +/** + * A helper class which maintains an #InputStream that is opened once + * and may be reused later for some time. It will be closed + * automatically after some time. + * + * This class is not thread-safe. All methods must be called on the + * thread which runs the #EventLoop. + */ +class LastInputStream { + std::string uri; + + Mutex mutex; + + InputStreamPtr is; + + TimerEvent close_timer; + +public: + explicit LastInputStream(EventLoop &event_loop) noexcept; + ~LastInputStream() noexcept; + + /** + * Open an #InputStream instance with the given opener + * function, but returns the cached instance if it matches. + * + * This object keeps owning the #InputStream; the caller shall + * not close it. + */ + template + InputStream *Open(U &&new_uri, O &&open) { + if (new_uri == uri) { + if (is) + /* refresh the timeout */ + ScheduleClose(); + + return is.get(); + } + + is.reset(); + close_timer.Cancel(); + + is = open(new_uri, mutex); + uri = std::forward(new_uri); + ScheduleClose(); + return is.get(); + } + + void Close() noexcept; + +private: + void ScheduleClose() noexcept { + close_timer.Schedule(std::chrono::seconds(20)); + } + + void OnCloseTimer() noexcept; +}; + +#endif diff --git a/src/input/meson.build b/src/input/meson.build index 374b5704e..03795b21b 100644 --- a/src/input/meson.build +++ b/src/input/meson.build @@ -8,6 +8,7 @@ input_api = static_library( 'ThreadInputStream.cxx', 'AsyncInputStream.cxx', 'ProxyInputStream.cxx', + 'LastInputStream.cxx', include_directories: inc, dependencies: [ boost_dep, diff --git a/src/queue/Playlist.cxx b/src/queue/Playlist.cxx index c5d15c780..f4a10b503 100644 --- a/src/queue/Playlist.cxx +++ b/src/queue/Playlist.cxx @@ -44,13 +44,13 @@ playlist::TagModified(DetachedSong &&song) noexcept } void -playlist::TagModified(const char *uri, const Tag &tag) noexcept +playlist::TagModified(const char *real_uri, const Tag &tag) noexcept { bool modified = false; for (unsigned i = 0; i < queue.length; ++i) { auto &song = *queue.items[i].song; - if (song.IsURI(uri)) { + if (song.IsRealURI(real_uri)) { song.SetTag(tag); queue.ModifyAtPosition(i); modified = true; diff --git a/src/queue/Playlist.hxx b/src/queue/Playlist.hxx index a04685a2d..69cd7d8d2 100644 --- a/src/queue/Playlist.hxx +++ b/src/queue/Playlist.hxx @@ -197,7 +197,12 @@ public: * the song matches. */ void TagModified(DetachedSong &&song) noexcept; - void TagModified(const char *uri, const Tag &tag) noexcept; + + /** + * @param real_uri the song's "real uri" (see + * DetachedSong::GetRealURI(), DetachedSong::IsRealURI()) + */ + void TagModified(const char *real_uri, const Tag &tag) noexcept; #ifdef ENABLE_DATABASE /** diff --git a/src/song/DetachedSong.hxx b/src/song/DetachedSong.hxx index 63203526b..255f0c7f7 100644 --- a/src/song/DetachedSong.hxx +++ b/src/song/DetachedSong.hxx @@ -169,6 +169,11 @@ public: return uri == other_uri; } + gcc_pure gcc_nonnull_all + bool IsRealURI(const char *other_uri) const noexcept { + return (HasRealURI() ? real_uri : uri) == other_uri; + } + gcc_pure bool IsRemote() const noexcept;