aboutsummaryrefslogtreecommitdiffhomepage
path: root/src
diff options
context:
space:
mode:
authorRobin Haberkorn <robin.haberkorn@googlemail.com>2016-02-10 15:08:46 +0100
committerRobin Haberkorn <robin.haberkorn@googlemail.com>2016-02-10 15:46:17 +0100
commit1da5bdeb986657c5cfd83d495d15b7f2308d3b5b (patch)
tree258c5297dea9b2296a7ff231c73dd101ff1ab18d /src
parentb21d294b8168e20282f31026cbc2e50a3bcd0222 (diff)
downloadsciteco-1da5bdeb986657c5cfd83d495d15b7f2308d3b5b.tar.gz
avoid unnecessary undo token allocations in batch mode: greatly speeds up batch mode
* by using variadic templates, UndoStack::push() is now responsible for allocating undo tokens. This is avoided in batch mode. * The old UndoStack::push(UndoToken *) method has been made private to avoid confusion around UndoStack's API. The old UndoStack::push() no longer needs to handle !undo.enabled, but at least asserts on it. * C++11 support is now required, so variadic templates can be used. This could have also been done using manual undo.enabled checks; or using multiple versions of the template with different numbers of template arguments. The latter could be done if we one day have to support a non-C++11 compiler. However since we're depending on GCC 4.4, variadic template use should be OK. Clang supports it since v2.9. * Sometimes, undo token pushing passed ownership of some memory to the undo token. The old behaviour was relied on to reclaim the memory even in batch mode -- the undo token was always deleted. To avoid leaks or repeated manual undo.enabled checking, another method UndoStack::push_own() had to be introduced that makes sure that an undo token is always created. In batch mode (!undo.enabled), this will however create the object on the stack which is much cheaper than using `new`. * Having to know which kind of undo token is to be pushed (taking ownership or not) is inconvenient. It may be better to add static methods to the UndoToken classes that can take care of reclaiming memory. * Benchmarking certain SciTECO scripts have shown 50% (!!!) speed increases at the highest possible optimization level (-O3 -mtune=native -march=native).
Diffstat (limited to 'src')
-rw-r--r--src/Makefile.am2
-rw-r--r--src/expressions.h4
-rw-r--r--src/goto.h2
-rw-r--r--src/interface-curses/Makefile.am2
-rw-r--r--src/interface-gtk/Makefile.am4
-rw-r--r--src/interface.h10
-rw-r--r--src/ioview.cpp8
-rw-r--r--src/parser.cpp2
-rw-r--r--src/qregisters.cpp10
-rw-r--r--src/qregisters.h2
-rw-r--r--src/ring.cpp4
-rw-r--r--src/ring.h2
-rw-r--r--src/undo.cpp10
-rw-r--r--src/undo.h48
14 files changed, 77 insertions, 33 deletions
diff --git a/src/Makefile.am b/src/Makefile.am
index 306bfb2..6da8d84 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -11,7 +11,7 @@ endif
include $(top_srcdir)/bootstrap.am
include $(top_srcdir)/scintilla.am
-AM_CXXFLAGS = -Wall -Wno-char-subscripts
+AM_CXXFLAGS = -std=c++11 -Wall -Wno-char-subscripts
if CLANG
AM_CXXFLAGS += -Wno-mismatched-tags
endif
diff --git a/src/expressions.h b/src/expressions.h
index f969b67..216ec7a 100644
--- a/src/expressions.h
+++ b/src/expressions.h
@@ -111,7 +111,7 @@ public:
inline void
undo_push(Type value, guint index = 0)
{
- undo.push(new UndoTokenPush(this, value, index));
+ undo.push<UndoTokenPush>(this, value, index);
}
inline Type
@@ -132,7 +132,7 @@ public:
inline void
undo_pop(guint index = 0)
{
- undo.push(new UndoTokenPop(this, index));
+ undo.push<UndoTokenPop>(this, index);
}
inline Type &
diff --git a/src/goto.h b/src/goto.h
index 6c5587d..1621acd 100644
--- a/src/goto.h
+++ b/src/goto.h
@@ -96,7 +96,7 @@ public:
undo_set(gchar *name, gint pc = -1)
{
if (must_undo)
- undo.push(new UndoTokenSet(this, name, pc));
+ undo.push<UndoTokenSet>(this, name, pc);
}
#ifdef DEBUG
diff --git a/src/interface-curses/Makefile.am b/src/interface-curses/Makefile.am
index 44e94da..e9a2d14 100644
--- a/src/interface-curses/Makefile.am
+++ b/src/interface-curses/Makefile.am
@@ -1,6 +1,6 @@
AM_CPPFLAGS += -I$(top_srcdir)/src
-AM_CXXFLAGS = -Wall -Wno-char-subscripts
+AM_CXXFLAGS = -std=c++11 -Wall -Wno-char-subscripts
if CLANG
AM_CXXFLAGS += -Wno-mismatched-tags
endif
diff --git a/src/interface-gtk/Makefile.am b/src/interface-gtk/Makefile.am
index 6c81879..dc4da3a 100644
--- a/src/interface-gtk/Makefile.am
+++ b/src/interface-gtk/Makefile.am
@@ -1,7 +1,7 @@
AM_CPPFLAGS += -I$(top_srcdir)/src
-AM_CFLAGS = -Wall -std=c99
-AM_CXXFLAGS = -Wall -Wno-char-subscripts
+AM_CFLAGS = -std=c99 -Wall
+AM_CXXFLAGS = -std=c++11 -Wall -Wno-char-subscripts
if CLANG
AM_CXXFLAGS += -Wno-mismatched-tags
endif
diff --git a/src/interface.h b/src/interface.h
index 9552984..84182ea 100644
--- a/src/interface.h
+++ b/src/interface.h
@@ -122,14 +122,14 @@ public:
undo_ssm(unsigned int iMessage,
uptr_t wParam = 0, sptr_t lParam = 0)
{
- undo.push(new UndoTokenMessage(impl(), iMessage, wParam, lParam));
+ undo.push<UndoTokenMessage>(impl(), iMessage, wParam, lParam);
}
void set_representations(void);
inline void
undo_set_representations(void)
{
- undo.push(new UndoTokenSetRepresentations(impl()));
+ undo.push<UndoTokenSetRepresentations>(impl());
}
inline void
@@ -233,7 +233,7 @@ public:
inline void
undo_show_view(ViewImpl *view)
{
- undo.push(new UndoTokenShowView(view));
+ undo.push<UndoTokenShowView>(view);
}
inline ViewImpl *
@@ -273,12 +273,12 @@ public:
inline void
undo_info_update(const QRegister *reg)
{
- undo.push(new UndoTokenInfoUpdate<QRegister>(reg));
+ undo.push<UndoTokenInfoUpdate<QRegister>>(reg);
}
inline void
undo_info_update(const Buffer *buffer)
{
- undo.push(new UndoTokenInfoUpdate<Buffer>(buffer));
+ undo.push<UndoTokenInfoUpdate<Buffer>>(buffer);
}
inline void
diff --git a/src/ioview.cpp b/src/ioview.cpp
index 1d32b11..cc0e8fc 100644
--- a/src/ioview.cpp
+++ b/src/ioview.cpp
@@ -463,8 +463,10 @@ make_savepoint(const gchar *filename)
}
savepoint_id++;
- /* NOTE: passes ownership of savepoint string to undo token */
- undo.push(new UndoTokenRestoreSavePoint(savepoint, filename));
+ /*
+ * NOTE: passes ownership of savepoint string to undo token.
+ */
+ undo.push_own<UndoTokenRestoreSavePoint>(savepoint, filename);
}
#endif
@@ -618,7 +620,7 @@ IOView::save(const gchar *filename)
attributes = get_file_attributes(filename);
make_savepoint(filename);
} else {
- undo.push(new UndoTokenRemoveFile(filename));
+ undo.push<UndoTokenRemoveFile>(filename);
}
}
diff --git a/src/parser.cpp b/src/parser.cpp
index a9797b6..2ae6f73 100644
--- a/src/parser.cpp
+++ b/src/parser.cpp
@@ -1640,7 +1640,7 @@ StateChangeDir::got_file(const gchar *filename)
BEGIN_EXEC(&States::start);
/* passes ownership of string to undo token object */
- undo.push(new UndoTokenChangeDir(g_get_current_dir()));
+ undo.push_own<UndoTokenChangeDir>(g_get_current_dir());
dir = *filename ? g_strdup(filename)
: QRegisters::globals["$HOME"]->get_string();
diff --git a/src/qregisters.cpp b/src/qregisters.cpp
index facf212..5283ce7 100644
--- a/src/qregisters.cpp
+++ b/src/qregisters.cpp
@@ -431,8 +431,8 @@ QRegisterWorkingDir::set_string(const gchar *str, gsize len)
void
QRegisterWorkingDir::undo_set_string(void)
{
- /* passes ownership of string to undo token object */
- undo.push(new UndoTokenChangeDir(g_get_current_dir()));
+ /* pass ownership of current dir string */
+ undo.push_own<UndoTokenChangeDir>(g_get_current_dir());
}
gchar *
@@ -706,7 +706,7 @@ QRegisterStack::push(QRegister &reg)
entry->set_integer(reg.get_integer());
SLIST_INSERT_HEAD(&head, entry, entries);
- undo.push(new UndoTokenPop(this));
+ undo.push<UndoTokenPop>(this);
}
bool
@@ -725,8 +725,8 @@ QRegisterStack::pop(QRegister &reg)
reg.exchange_string(*entry);
SLIST_REMOVE_HEAD(&head, entries);
- /* pass entry ownership to undo stack */
- undo.push(new UndoTokenPush(this, entry));
+ /* Pass entry ownership to undo stack. */
+ undo.push_own<UndoTokenPush>(this, entry);
return true;
}
diff --git a/src/qregisters.h b/src/qregisters.h
index 3227bc8..563a774 100644
--- a/src/qregisters.h
+++ b/src/qregisters.h
@@ -248,7 +248,7 @@ public:
undo_remove(QRegister *reg)
{
if (must_undo)
- undo.push(new UndoTokenRemove(this, reg));
+ undo.push<UndoTokenRemove>(this, reg);
}
inline QRegister *
diff --git a/src/ring.cpp b/src/ring.cpp
index ac3a09d..702807d 100644
--- a/src/ring.cpp
+++ b/src/ring.cpp
@@ -252,8 +252,8 @@ Ring::close(void)
QRegisters::hook(QRegisters::HOOK_CLOSE);
close(buffer);
current = buffer->next() ? : buffer->prev();
- /* transfer responsibility to UndoToken object */
- undo.push(new UndoTokenEdit(this, buffer));
+ /* Transfer responsibility to UndoToken object. */
+ undo.push_own<UndoTokenEdit>(this, buffer);
if (current) {
current->edit();
diff --git a/src/ring.h b/src/ring.h
index 31fda65..9b8b9d7 100644
--- a/src/ring.h
+++ b/src/ring.h
@@ -53,7 +53,7 @@ class Buffer : private IOView {
inline void
undo_close(void)
{
- undo.push(new UndoTokenClose(this));
+ undo.push<UndoTokenClose>(this);
}
public:
diff --git a/src/undo.cpp b/src/undo.cpp
index b6c510c..e189e23 100644
--- a/src/undo.cpp
+++ b/src/undo.cpp
@@ -67,10 +67,12 @@ UndoStack::set_memory_limit(gsize new_limit)
void
UndoStack::push(UndoToken *token)
{
- if (!enabled) {
- delete token;
- return;
- }
+ /*
+ * All undo token allocations should take place using the
+ * variadic template version of UndoStack::push(), so we
+ * don't have to check `enabled` here.
+ */
+ g_assert(enabled == true);
if (memory_limit) {
gsize token_size = token->get_size();
diff --git a/src/undo.h b/src/undo.h
index 325aee9..1e92b16 100644
--- a/src/undo.h
+++ b/src/undo.h
@@ -175,6 +175,8 @@ extern class UndoStack {
*/
gsize memory_usage;
+ void push(UndoToken *token);
+
public:
bool enabled;
@@ -194,13 +196,50 @@ public:
void set_memory_limit(gsize new_limit = 0);
- void push(UndoToken *token);
+ /**
+ * Allocate and push undo token.
+ *
+ * This does nothing if undo is disabled and should
+ * not be used when ownership of some data is to be
+ * passed to the undo token.
+ */
+ template <class TokenType, typename... Params>
+ inline void
+ push(Params && ... params)
+ {
+ if (enabled)
+ push(new TokenType(params...));
+ }
+
+ /**
+ * Allocate and push undo token, passing ownership.
+ *
+ * This creates and deletes the undo token cheaply
+ * if undo is disabled, so that data whose ownership
+ * is passed to the undo token is correctly reclaimed.
+ *
+ * @bug We must know which version of push to call
+ * depending on the token type. This could be hidden
+ * if UndoTokens had static push methods that take care
+ * of reclaiming memory.
+ */
+ template <class TokenType, typename... Params>
+ inline void
+ push_own(Params && ... params)
+ {
+ if (enabled) {
+ push(new TokenType(params...));
+ } else {
+ /* ensures that all memory is reclaimed */
+ TokenType dummy(params...);
+ }
+ }
template <typename Type>
inline Type &
push_var(Type &variable, Type value)
{
- push(new UndoTokenVariable<Type>(variable, value));
+ push<UndoTokenVariable<Type>>(variable, value);
return variable;
}
@@ -214,7 +253,7 @@ public:
inline gchar *&
push_str(gchar *&variable, gchar *str)
{
- push(new UndoTokenString(variable, str));
+ push<UndoTokenString>(variable, str);
return variable;
}
inline gchar *&
@@ -227,7 +266,8 @@ public:
inline Type *&
push_obj(Type *&variable, Type *obj)
{
- push(new UndoTokenObject<Type>(variable, obj));
+ /* pass ownership of original object */
+ push_own<UndoTokenObject<Type>>(variable, obj);
return variable;
}