Skip to content

Commit

Permalink
[snapshot] Avoid deriving the ID string each time
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ricab committed Oct 27, 2023
1 parent c9c1415 commit b26af05
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 10 deletions.
6 changes: 3 additions & 3 deletions src/platform/backends/qemu/qemu_snapshot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ mp::QemuSnapshot::QemuSnapshot(const QString& filename, QemuVirtualMachine& vm,

void mp::QemuSnapshot::capture_impl()

Check warning on line 72 in src/platform/backends/qemu/qemu_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/qemu/qemu_snapshot.cpp#L72

Added line #L72 was not covered by tests
{
const auto tag = derive_id();
const auto& tag = get_id();

Check warning on line 74 in src/platform/backends/qemu/qemu_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/qemu/qemu_snapshot.cpp#L74

Added line #L74 was not covered by tests

// 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)
Expand All @@ -86,7 +86,7 @@ void mp::QemuSnapshot::capture_impl()

void mp::QemuSnapshot::erase_impl()

Check warning on line 87 in src/platform/backends/qemu/qemu_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/qemu/qemu_snapshot.cpp#L87

Added line #L87 was not covered by tests
{
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));

Check warning on line 91 in src/platform/backends/qemu/qemu_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/qemu/qemu_snapshot.cpp#L89-L91

Added lines #L89 - L91 were not covered by tests
else
Expand All @@ -107,6 +107,6 @@ void mp::QemuSnapshot::apply_impl()
desc.mem_size = get_mem_size();
desc.disk_space = get_disk_space();

Check warning on line 108 in src/platform/backends/qemu/qemu_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/qemu/qemu_snapshot.cpp#L106-L108

Added lines #L106 - L108 were not covered by tests

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();

Check warning on line 111 in src/platform/backends/qemu/qemu_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/qemu/qemu_snapshot.cpp#L110-L111

Added lines #L110 - L111 were not covered by tests
}
6 changes: 1 addition & 5 deletions src/platform/backends/shared/base_snapshot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -225,11 +226,6 @@ void mp::BaseSnapshot::persist() const
MP_JSONUTILS.write_json(serialize(), snapshot_filepath);

Check warning on line 226 in src/platform/backends/shared/base_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/shared/base_snapshot.cpp#L225-L226

Added lines #L225 - L226 were not covered by tests
}

QString mp::BaseSnapshot::derive_id() const
{
return snapshot_template.arg(index);
}

auto mp::BaseSnapshot::erase_helper()

Check warning on line 229 in src/platform/backends/shared/base_snapshot.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/shared/base_snapshot.cpp#L229

Added line #L229 was not covered by tests
{
// Remove snapshot file
Expand Down
10 changes: 8 additions & 2 deletions src/platform/backends/shared/base_snapshot.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Check warning on line 248 in src/platform/backends/shared/base_snapshot.h

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/shared/base_snapshot.h#L248

Added line #L248 was not covered by tests
{
return id;

Check warning on line 250 in src/platform/backends/shared/base_snapshot.h

View check run for this annotation

Codecov / codecov/patch

src/platform/backends/shared/base_snapshot.h#L250

Added line #L250 was not covered by tests
}

#endif // MULTIPASS_BASE_SNAPSHOT_H

0 comments on commit b26af05

Please sign in to comment.