From b26af054612b2071ae742d82850a87747c835efb Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 27 Oct 2023 16:50:33 +0100 Subject: [PATCH] [snapshot] Avoid deriving the ID string each time Store the (now immutable) ID string in a snapshot field and provide an accessor to it, rather than deriving it each time it is needed. Adapt client code accordingly. --- src/platform/backends/qemu/qemu_snapshot.cpp | 6 +++--- src/platform/backends/shared/base_snapshot.cpp | 6 +----- src/platform/backends/shared/base_snapshot.h | 10 ++++++++-- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/platform/backends/qemu/qemu_snapshot.cpp b/src/platform/backends/qemu/qemu_snapshot.cpp index 85f86a0ea08..cd02cc8decb 100644 --- a/src/platform/backends/qemu/qemu_snapshot.cpp +++ b/src/platform/backends/qemu/qemu_snapshot.cpp @@ -71,7 +71,7 @@ mp::QemuSnapshot::QemuSnapshot(const QString& filename, QemuVirtualMachine& vm, void mp::QemuSnapshot::capture_impl() { - const auto tag = derive_id(); + const auto& tag = get_id(); // Avoid creating more than one snapshot with the same tag (creation would succeed, but we'd then be unable to // identify the snapshot by tag) @@ -86,7 +86,7 @@ void mp::QemuSnapshot::capture_impl() void mp::QemuSnapshot::erase_impl() { - const auto tag = derive_id(); + const auto& tag = get_id(); if (backend::instance_image_has_snapshot(image_path, tag)) mp::backend::checked_exec_qemu_img(make_delete_spec(tag, image_path)); else @@ -107,6 +107,6 @@ void mp::QemuSnapshot::apply_impl() desc.mem_size = get_mem_size(); desc.disk_space = get_disk_space(); - mp::backend::checked_exec_qemu_img(make_restore_spec(derive_id(), image_path)); + mp::backend::checked_exec_qemu_img(make_restore_spec(get_id(), image_path)); rollback.dismiss(); } diff --git a/src/platform/backends/shared/base_snapshot.cpp b/src/platform/backends/shared/base_snapshot.cpp index 907e36a9a95..d613993b95a 100644 --- a/src/platform/backends/shared/base_snapshot.cpp +++ b/src/platform/backends/shared/base_snapshot.cpp @@ -113,6 +113,7 @@ mp::BaseSnapshot::BaseSnapshot(const std::string& name, // NOLINT(modernize-p comment{comment}, parent{std::move(parent)}, index{index}, + id{snapshot_template.arg(index)}, creation_timestamp{std::move(creation_timestamp)}, num_cores{num_cores}, mem_size{mem_size}, @@ -225,11 +226,6 @@ void mp::BaseSnapshot::persist() const MP_JSONUTILS.write_json(serialize(), snapshot_filepath); } -QString mp::BaseSnapshot::derive_id() const -{ - return snapshot_template.arg(index); -} - auto mp::BaseSnapshot::erase_helper() { // Remove snapshot file diff --git a/src/platform/backends/shared/base_snapshot.h b/src/platform/backends/shared/base_snapshot.h index a07afcb0c9c..eda4fd6fdbb 100644 --- a/src/platform/backends/shared/base_snapshot.h +++ b/src/platform/backends/shared/base_snapshot.h @@ -69,12 +69,12 @@ class BaseSnapshot : public Snapshot void apply() final; protected: + const QString& get_id() const noexcept; + virtual void capture_impl() = 0; virtual void erase_impl() = 0; virtual void apply_impl() = 0; - QString derive_id() const; - private: BaseSnapshot(const QJsonObject& json, VirtualMachine& vm); BaseSnapshot(const std::string& name, @@ -103,6 +103,7 @@ class BaseSnapshot : public Snapshot // This class is non-copyable and having these const simplifies thread safety const int index; // NOLINT(cppcoreguidelines-avoid-const-or-ref-data-members) + const QString id; // NOLINT(cppcoreguidelines-avoid-const-or-ref-data-members) const QDateTime creation_timestamp; // NOLINT(cppcoreguidelines-avoid-const-or-ref-data-members) const int num_cores; // NOLINT(cppcoreguidelines-avoid-const-or-ref-data-members) const MemorySize mem_size; // NOLINT(cppcoreguidelines-avoid-const-or-ref-data-members) @@ -244,4 +245,9 @@ inline void multipass::BaseSnapshot::apply() // those cannot be affected by apply_impl (except by setters, which already persist) } +inline const QString& multipass::BaseSnapshot::get_id() const noexcept +{ + return id; +} + #endif // MULTIPASS_BASE_SNAPSHOT_H