Redesign the undo stack

Store the data in vectors rather than AssFiles since even an intrusive
linked list has comically high memory overhead. Cuts memory usage of a
full undo stack with 15k lines by 65 MB for 32-bit and 130 MB for
64-bit. Also roughly halves how long it takes to copy the file for the
undo stack, and makes undo/redo a bit faster.

aegisub/src/ass_dialogue.cpp

@@ -53,34 +53,24 @@ using namespace boost::adaptors;
 
 static int next_id = 0;
 
-AssDialogue::AssDialogue()
+AssDialogue::AssDialogue() {
-: Id(++next_id)
+	Id = ++next_id;
-{
-	memset(Margin, 0, sizeof Margin);
 }
 
-AssDialogue::AssDialogue(AssDialogue const& that)
+AssDialogue::AssDialogue(AssDialogue const& that) : AssDialogueBase(that) {
-: Id(++next_id)
+	Id = ++next_id;
-, Comment(that.Comment)
-, Layer(that.Layer)
-, Start(that.Start)
-, End(that.End)
-, Style(that.Style)
-, Actor(that.Actor)
-, Effect(that.Effect)
-, Text(that.Text)
-{
-	memmove(Margin, that.Margin, sizeof Margin);
 }
 
-AssDialogue::AssDialogue(std::string const& data)
+AssDialogue::AssDialogue(AssDialogueBase const& that) : AssDialogueBase(that) {
-: Id(++next_id)
+	Id = ++next_id;
-{
+}
+
+AssDialogue::AssDialogue(std::string const& data) {
+	Id = ++next_id;
 	Parse(data);
 }
 
-AssDialogue::~AssDialogue () {
+AssDialogue::~AssDialogue () { }
-}
 
 class tokenizer {
 	agi::StringRange str;
@@ -177,14 +167,6 @@ std::string AssDialogue::GetData(bool ssa) const {
 	return str;
 }
 
-const std::string AssDialogue::GetEntryData() const {
-	return GetData(false);
-}
-
-std::string AssDialogue::GetSSAText() const {
-	return GetData(true);
-}
-
 std::auto_ptr<boost::ptr_vector<AssDialogueBlock>> AssDialogue::ParseTags() const {
 	boost::ptr_vector<AssDialogueBlock> Blocks;
 
@@ -278,6 +260,6 @@ std::string AssDialogue::GetStrippedText() const {
 
 AssEntry *AssDialogue::Clone() const {
 	auto clone = new AssDialogue(*this);
-	*const_cast<int *>(&clone->Id) = Id;
+	clone->Id = Id;
 	return clone;
 }

aegisub/src/ass_dialogue.h

@@ -38,6 +38,7 @@
 
 #include <libaegisub/exception.h>
 
+#include <array>
 #include <boost/flyweight.hpp>
 #include <boost/ptr_container/ptr_vector.hpp>
 #include <vector>
@@ -123,24 +124,18 @@ public:
 	void ProcessParameters(ProcessParametersCallback callback, void *userData);
 };
 
-class AssDialogue : public AssEntry {
+struct AssDialogueBase {
-	std::string GetData(bool ssa) const;
-
-	/// @brief Parse raw ASS data into everything else
-	/// @param data ASS line
-	void Parse(std::string const& data);
-public:
 	/// Unique ID of this line. Copies of the line for Undo/Redo purposes
 	/// preserve the unique ID, so that the equivalent lines can be found in
 	/// the different versions of the file.
-	const int Id;
+	int Id;
 
 	/// Is this a comment line?
 	bool Comment = false;
 	/// Layer number
 	int Layer = 0;
 	/// Margins: 0 = Left, 1 = Right, 2 = Top (Vertical)
-	int Margin[3];
+	std::array<int, 3> Margin = {{0, 0, 0}};
 	/// Starting time
 	AssTime Start = 0;
 	/// Ending time
@@ -153,7 +148,15 @@ public:
 	boost::flyweight<std::string> Effect;
 	/// Raw text data
 	boost::flyweight<std::string> Text;
+};
 
+class AssDialogue : public AssEntry, public AssDialogueBase {
+	std::string GetData(bool ssa) const;
+
+	/// @brief Parse raw ASS data into everything else
+	/// @param data ASS line
+	void Parse(std::string const& data);
+public:
 	AssEntryGroup Group() const override { return AssEntryGroup::DIALOGUE; }
 
 	/// Parse text as ASS and return block information
@@ -167,10 +170,10 @@ public:
 
 	/// Update the text of the line from parsed blocks
 	void UpdateText(boost::ptr_vector<AssDialogueBlock>& blocks);
-	const std::string GetEntryData() const override;
+	const std::string GetEntryData() const override { return GetData(false); }
 
 	/// Get the line as SSA rather than ASS
-	std::string GetSSAText() const override;
+	std::string GetSSAText() const override { return GetData(true); }
 	/// Does this line collide with the passed line?
 	bool CollidesWith(const AssDialogue *target) const;
 
@@ -178,6 +181,7 @@ public:
 
 	AssDialogue();
 	AssDialogue(AssDialogue const&);
+	AssDialogue(AssDialogueBase const&);
 	AssDialogue(std::string const& data);
 	~AssDialogue();
 };

aegisub/src/ass_file.cpp

@@ -206,8 +206,7 @@ AssStyle *AssFile::GetStyle(std::string const& name) {
 }
 
 int AssFile::Commit(wxString const& desc, int type, int amend_id, AssEntry *single_line) {
-	AssFileCommit c = { desc, &amend_id, single_line };
+	PushState({desc, &amend_id, single_line});
-	PushState(c);
 
 	std::set<const AssEntry*> changed_lines;
 	if (single_line)

aegisub/src/ass_info.h

@@ -23,7 +23,7 @@ class AssInfo : public AssEntry {
 	std::string value;
 
 public:
-	AssInfo(AssInfo const& o) : key(o.key), value(o.value) { }
+	AssInfo(AssInfo const& o) = default;
 	AssInfo(std::string key, std::string value) : key(std::move(key)), value(std::move(value)) { }
 
 	AssEntry *Clone() const override { return new AssInfo(*this); }

aegisub/src/search_replace_engine.cpp

@@ -34,17 +34,17 @@
 static const size_t bad_pos = -1;
 
 namespace {
-auto get_dialogue_field(SearchReplaceSettings::Field field) -> decltype(&AssDialogue::Text) {
+auto get_dialogue_field(SearchReplaceSettings::Field field) -> decltype(&AssDialogueBase::Text) {
 	switch (field) {
-		case SearchReplaceSettings::Field::TEXT: return &AssDialogue::Text;
+		case SearchReplaceSettings::Field::TEXT: return &AssDialogueBase::Text;
-		case SearchReplaceSettings::Field::STYLE: return &AssDialogue::Style;
+		case SearchReplaceSettings::Field::STYLE: return &AssDialogueBase::Style;
-		case SearchReplaceSettings::Field::ACTOR: return &AssDialogue::Actor;
+		case SearchReplaceSettings::Field::ACTOR: return &AssDialogueBase::Actor;
-		case SearchReplaceSettings::Field::EFFECT: return &AssDialogue::Effect;
+		case SearchReplaceSettings::Field::EFFECT: return &AssDialogueBase::Effect;
 	}
 	throw agi::InternalError("Bad field for search", nullptr);
 }
 
-std::string const& get_normalized(const AssDialogue *diag, decltype(&AssDialogue::Text) field) {
+std::string const& get_normalized(const AssDialogue *diag, decltype(&AssDialogueBase::Text) field) {
 	auto& value = const_cast<AssDialogue*>(diag)->*field;
 	auto normalized = boost::locale::normalize(value.get());
 	if (normalized != value)
@@ -55,7 +55,7 @@ std::string const& get_normalized(const AssDialogue *diag, decltype(&AssDialogue
 typedef std::function<MatchState (const AssDialogue*, size_t)> matcher;
 
 class noop_accessor {
-	boost::flyweight<std::string> AssDialogue::*field;
+	boost::flyweight<std::string> AssDialogueBase::*field;
 	size_t start;
 
 public:
@@ -72,7 +72,7 @@ public:
 };
 
 class skip_tags_accessor {
-	boost::flyweight<std::string> AssDialogue::*field;
+	boost::flyweight<std::string> AssDialogueBase::*field;
 	std::vector<std::pair<size_t, size_t>> blocks;
 	size_t start;
 

aegisub/src/subs_controller.cpp

@@ -18,8 +18,10 @@
 
 #include "subs_controller.h"
 
+#include "ass_attachment.h"
 #include "ass_dialogue.h"
 #include "ass_file.h"
+#include "ass_info.h"
 #include "ass_style.h"
 #include "charset_detect.h"
 #include "compat.h"
@@ -50,10 +52,73 @@ namespace {
 }
 
 struct SubsController::UndoInfo {
-	AssFile file;
+	std::vector<std::pair<std::string, std::string>> script_info;
+	std::vector<AssStyle> styles;
+	std::vector<AssDialogueBase> events;
+	std::vector<AssAttachment> graphics;
+	std::vector<AssAttachment> fonts;
+
 	wxString undo_description;
 	int commit_id;
-	UndoInfo(AssFile const& f, wxString const& d, int c) : file(f), undo_description(d), commit_id(c) { }
+	UndoInfo(AssFile const& f, wxString const& d, int c)
+	: undo_description(d), commit_id(c)
+	{
+		size_t info_count = 0, style_count = 0, event_count = 0, font_count = 0, graphics_count = 0;
+		for (auto const& line : f.Line) {
+			switch (line.Group()) {
+				case AssEntryGroup::DIALOGUE: ++event_count; break;
+				case AssEntryGroup::INFO: ++info_count; break;
+				case AssEntryGroup::STYLE: ++style_count; break;
+				case AssEntryGroup::FONT: ++font_count; break;
+				case AssEntryGroup::GRAPHIC: ++graphics_count; break;
+				default: assert(false); break;
+			}
+		}
+
+		script_info.reserve(info_count);
+		styles.reserve(style_count);
+		events.reserve(event_count);
+
+		for (auto const& line : f.Line) {
+			switch (line.Group()) {
+			case AssEntryGroup::DIALOGUE:
+				events.push_back(static_cast<AssDialogue const&>(line));
+				break;
+			case AssEntryGroup::INFO: {
+				auto info = static_cast<const AssInfo *>(&line);
+				script_info.emplace_back(info->Key(), info->Value());
+				break;
+			}
+			case AssEntryGroup::STYLE:
+				styles.push_back(static_cast<AssStyle const&>(line));
+				break;
+			case AssEntryGroup::FONT:
+				fonts.push_back(static_cast<AssAttachment const&>(line));
+				break;
+			case AssEntryGroup::GRAPHIC:
+				graphics.push_back(static_cast<AssAttachment const&>(line));
+				break;
+			default:
+				assert(false);
+				break;
+			}
+		}
+	}
+
+	operator AssFile() const {
+		AssFile ret;
+		for (auto const& info : script_info)
+			ret.Line.push_back(*new AssInfo(info.first, info.second));
+		for (auto const& style : styles)
+			ret.Line.push_back(*new AssStyle(style));
+		for (auto const& event : events)
+			ret.Line.push_back(*new AssDialogue(event));
+		for (auto const& attachment : graphics)
+			ret.Line.push_back(*new AssAttachment(attachment));
+		for (auto const& attachment : fonts)
+			ret.Line.push_back(*new AssAttachment(attachment));
+		return ret;
+	}
 };
 
 SubsController::SubsController(agi::Context *context)
@@ -275,20 +340,19 @@ void SubsController::OnCommit(AssFileCommit c) {
 	// saved since the last change
 	if (commit_id == *c.commit_id+1 && redo_stack.empty() && saved_commit_id+1 != commit_id && autosaved_commit_id+1 != commit_id) {
 		// If only one line changed just modify it instead of copying the file
-		if (c.single_line) {
+		if (c.single_line && c.single_line->Group() == AssEntryGroup::DIALOGUE) {
-			entryIter this_it = context->ass->Line.begin(), undo_it = undo_stack.back().file.Line.begin();
+			auto src_diag = static_cast<const AssDialogue *>(c.single_line);
-			while (&*this_it != c.single_line) {
+			for (auto& diag : undo_stack.back().events) {
-				++this_it;
+				if (diag.Id == src_diag->Id) {
-				++undo_it;
+					diag = *src_diag;
+					break;
+				}
 			}
-			undo_stack.back().file.Line.insert(undo_it, *c.single_line->Clone());
+			*c.commit_id = commit_id;
-			delete &*undo_it;
+			return;
 		}
-		else
-			undo_stack.back().file = *context->ass;
 
-		*c.commit_id = commit_id;
+		undo_stack.pop_back();
-		return;
 	}
 
 	redo_stack.clear();
@@ -305,28 +369,27 @@ void SubsController::OnCommit(AssFileCommit c) {
 	*c.commit_id = commit_id;
 }
 
-void SubsController::Undo() {
+void SubsController::ApplyUndo() {
-	if (undo_stack.size() <= 1) return;
+	// Keep old lines alive until after the commit is complete
+	AssFile old;
+	old.swap(*context->ass);
 
-	redo_stack.splice(redo_stack.end(), undo_stack, std::prev(undo_stack.end()));
+	*context->ass = undo_stack.back();
-	*context->ass = undo_stack.back().file;
 	commit_id = undo_stack.back().commit_id;
 
 	context->ass->Commit("", AssFile::COMMIT_NEW);
 }
 
+void SubsController::Undo() {
+	if (undo_stack.size() <= 1) return;
+	redo_stack.splice(redo_stack.end(), undo_stack, std::prev(undo_stack.end()));
+	ApplyUndo();
+}
+
 void SubsController::Redo() {
 	if (redo_stack.empty()) return;
-
+	undo_stack.splice(undo_stack.end(), redo_stack, std::prev(redo_stack.end()));
-	context->ass->swap(redo_stack.back().file);
+	ApplyUndo();
-	commit_id = redo_stack.back().commit_id;
-	undo_stack.emplace_back(*context->ass, redo_stack.back().undo_description, commit_id);
-
-	context->ass->Commit("", AssFile::COMMIT_NEW);
-
-    // Done after commit so that the old active line and selection stay alive
-    // while the commit is being processed
-	redo_stack.pop_back();
 }
 
 wxString SubsController::GetUndoDescription() const {

aegisub/src/subs_controller.h

@@ -62,6 +62,9 @@ class SubsController {
 	/// Set the filename, updating things like the MRU and last used path
 	void SetFileName(agi::fs::path const& file);
 
+	/// Set the current file to the file on top of the undo stack
+	void ApplyUndo();
+
 public:
 	SubsController(agi::Context *context);
 

aegisub/src/visual_tool.cpp

@@ -361,8 +361,7 @@ Vector2D VisualToolBase::GetLinePosition(AssDialogue *diag) {
 	if (Vector2D ret = vec_or_bad(find_tag(blocks, "\\move"), 0, 1)) return ret;
 
 	// Get default position
-	int margin[3];
+	auto margin = diag->Margin;
-	memcpy(margin, diag->Margin, sizeof margin);
 	int align = 2;
 
 	if (AssStyle *style = c->ass->GetStyle(diag->Style)) {