From 7f9a8b87488d45138c230f5980a55d21c9075999 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 3 Sep 2019 20:23:46 +0200 Subject: [PATCH] db/simple/Song: convert "uri" to a std::string No longer allocate it as a "VarSize". This used to be a clever trick to save memory 10 years ago, but these days, keeping the code maintainable seems more important than saving a few kilobytes of memory. --- src/SongSave.cxx | 2 +- src/db/plugins/simple/Directory.cxx | 5 ++-- src/db/plugins/simple/Disposer.hxx | 29 ---------------------- src/db/plugins/simple/Ptr.hxx | 6 ++--- src/db/plugins/simple/Song.cxx | 38 ++++------------------------- src/db/plugins/simple/Song.hxx | 5 +--- src/db/plugins/simple/SongSort.cxx | 2 +- src/db/update/Container.cxx | 2 +- src/db/update/Editor.cxx | 2 +- src/db/update/Walk.cxx | 4 +-- 10 files changed, 17 insertions(+), 78 deletions(-) delete mode 100644 src/db/plugins/simple/Disposer.hxx diff --git a/src/SongSave.cxx b/src/SongSave.cxx index 836f5d5a0..afd5bc55a 100644 --- a/src/SongSave.cxx +++ b/src/SongSave.cxx @@ -52,7 +52,7 @@ range_save(BufferedOutputStream &os, unsigned start_ms, unsigned end_ms) void song_save(BufferedOutputStream &os, const Song &song) { - os.Format(SONG_BEGIN "%s\n", song.uri); + os.Format(SONG_BEGIN "%s\n", song.uri.c_str()); range_save(os, song.start_time.ToMS(), song.end_time.ToMS()); diff --git a/src/db/plugins/simple/Directory.cxx b/src/db/plugins/simple/Directory.cxx index e4a279dc1..2c1a5eced 100644 --- a/src/db/plugins/simple/Directory.cxx +++ b/src/db/plugins/simple/Directory.cxx @@ -20,7 +20,6 @@ #include "Directory.hxx" #include "SongSort.hxx" #include "Song.hxx" -#include "Disposer.hxx" #include "Mount.hxx" #include "db/LightDirectory.hxx" #include "song/LightSong.hxx" @@ -51,7 +50,7 @@ Directory::~Directory() noexcept mounted_database.reset(); } - songs.clear_and_dispose(SongDisposer()); + songs.clear_and_dispose(DeleteDisposer()); children.clear_and_dispose(DeleteDisposer()); } @@ -192,7 +191,7 @@ Directory::FindSong(const char *name_utf8) const noexcept for (auto &song : songs) { assert(song.parent == this); - if (strcmp(song.uri, name_utf8) == 0) + if (song.uri == name_utf8) return &song; } diff --git a/src/db/plugins/simple/Disposer.hxx b/src/db/plugins/simple/Disposer.hxx deleted file mode 100644 index e72cf9040..000000000 --- a/src/db/plugins/simple/Disposer.hxx +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright 2003-2019 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_SONG_DISPOSER_HXX -#define MPD_SONG_DISPOSER_HXX - -struct Song; - -struct SongDisposer { - void operator()(Song *song) const noexcept; -}; - -#endif diff --git a/src/db/plugins/simple/Ptr.hxx b/src/db/plugins/simple/Ptr.hxx index b649b6395..3882713e2 100644 --- a/src/db/plugins/simple/Ptr.hxx +++ b/src/db/plugins/simple/Ptr.hxx @@ -20,10 +20,10 @@ #ifndef MPD_SONG_PTR_HXX #define MPD_SONG_PTR_HXX -#include "Disposer.hxx" - #include -using SongPtr = std::unique_ptr; +struct Song; + +using SongPtr = std::unique_ptr; #endif diff --git a/src/db/plugins/simple/Song.cxx b/src/db/plugins/simple/Song.cxx index 0ff9cda07..c6c4cc8f5 100644 --- a/src/db/plugins/simple/Song.cxx +++ b/src/db/plugins/simple/Song.cxx @@ -18,36 +18,22 @@ */ #include "Song.hxx" -#include "Disposer.hxx" #include "Directory.hxx" #include "tag/Tag.hxx" -#include "util/VarSize.hxx" #include "song/DetachedSong.hxx" #include "song/LightSong.hxx" #include "util/StringView.hxx" -#include -#include - inline Song::Song(StringView _uri, Directory &_parent) noexcept - :parent(&_parent) -{ - memcpy(uri, _uri.data, _uri.size + 1); -} - -inline -Song::~Song() noexcept + :parent(&_parent), uri(_uri.data, _uri.size) { } static SongPtr song_alloc(StringView uri, Directory &parent) noexcept { - auto *song = NewVarSize(sizeof(Song::uri), - uri.size + 1, - uri, parent); - return SongPtr(song); + return std::make_unique(uri, parent); } SongPtr @@ -67,30 +53,16 @@ Song::NewFile(const char *path, Directory &parent) noexcept return SongPtr(song_alloc(path, parent)); } -void -Song::Free() noexcept -{ - DeleteVarSize(this); -} - -void -SongDisposer::operator()(Song *song) const noexcept -{ - song->Free(); -} - std::string Song::GetURI() const noexcept { - assert(*uri); - if (parent->IsRoot()) - return std::string(uri); + return uri; else { const char *path = parent->GetPath(); std::string result; - result.reserve(strlen(path) + 1 + strlen(uri)); + result.reserve(strlen(path) + 1 + uri.length()); result.assign(path); result.push_back('/'); result.append(uri); @@ -101,7 +73,7 @@ Song::GetURI() const noexcept LightSong Song::Export() const noexcept { - LightSong dest(uri, tag); + LightSong dest(uri.c_str(), tag); dest.directory = parent->IsRoot() ? nullptr : parent->GetPath(); dest.real_uri = nullptr; diff --git a/src/db/plugins/simple/Song.hxx b/src/db/plugins/simple/Song.hxx index aa034e02d..7637b764e 100644 --- a/src/db/plugins/simple/Song.hxx +++ b/src/db/plugins/simple/Song.hxx @@ -92,10 +92,9 @@ struct Song { /** * The file name. */ - char uri[sizeof(int)]; + std::string uri; Song(StringView _uri, Directory &parent) noexcept; - ~Song() noexcept; static SongPtr NewFrom(DetachedSong &&other, Directory &parent) noexcept; @@ -115,8 +114,6 @@ struct Song { static SongPtr LoadFile(Storage &storage, const char *name_utf8, Directory &parent); - void Free() noexcept; - /** * Throws on error. * diff --git a/src/db/plugins/simple/SongSort.cxx b/src/db/plugins/simple/SongSort.cxx index a23aae157..982c07862 100644 --- a/src/db/plugins/simple/SongSort.cxx +++ b/src/db/plugins/simple/SongSort.cxx @@ -96,7 +96,7 @@ song_cmp(const Song &a, const Song &b) noexcept return ret < 0; /* still no difference? compare file name */ - return IcuCollate(a.uri, b.uri) < 0; + return IcuCollate(a.uri.c_str(), b.uri.c_str()) < 0; } void diff --git a/src/db/update/Container.cxx b/src/db/update/Container.cxx index b1474df90..651a10bee 100644 --- a/src/db/update/Container.cxx +++ b/src/db/update/Container.cxx @@ -75,7 +75,7 @@ UpdateWalk::UpdateContainerFile(Directory &directory, song->mtime = info.mtime; FormatDefault(update_domain, "added %s/%s", - contdir->GetPath(), song->uri); + contdir->GetPath(), song->uri.c_str()); { const ScopeDatabaseLock protect; diff --git a/src/db/update/Editor.cxx b/src/db/update/Editor.cxx index db9f2b3c3..c2ee27847 100644 --- a/src/db/update/Editor.cxx +++ b/src/db/update/Editor.cxx @@ -41,7 +41,7 @@ DatabaseEditor::DeleteSong(Directory &dir, Song *del) remove.Remove(del->GetURI()); /* finally, all possible references gone, free it */ - del->Free(); + delete del; } void diff --git a/src/db/update/Walk.cxx b/src/db/update/Walk.cxx index f27b757e1..245c045d8 100644 --- a/src/db/update/Walk.cxx +++ b/src/db/update/Walk.cxx @@ -81,7 +81,7 @@ UpdateWalk::RemoveExcludedFromDirectory(Directory &directory, directory.ForEachSongSafe([&](Song &song){ assert(song.parent == &directory); - const auto name_fs = AllocatedPath::FromUTF8(song.uri); + const auto name_fs = AllocatedPath::FromUTF8(song.uri.c_str()); if (name_fs.IsNull() || exclude_list.Check(name_fs)) { editor.DeleteSong(directory, &song); modified = true; @@ -103,7 +103,7 @@ UpdateWalk::PurgeDeletedFromDirectory(Directory &directory) noexcept directory.ForEachSongSafe([&](Song &song){ if (!directory_child_is_regular(storage, directory, - song.uri)) { + song.uri.c_str())) { editor.LockDeleteSong(directory, &song); modified = true;