From b7ff56db631be7416cf228dff89cb23d753e4ec8 Mon Sep 17 00:00:00 2001 From: Robin Haberkorn Date: Sun, 20 Nov 2016 09:00:50 +0100 Subject: fixed glib warnings about using g_mem_set_vtable() and revised memory limiting * we were basing the glib allocators on throwing std::bad_alloc just like the C++ operators. However, this always was unsafe since we were throwing exceptions across plain-C frames (Glib). Also, the memory vtable has been deprecated in Glib, resulting in ugly warnings. * Instead, we now let the C++ new/delete operators work like Glib by basing them on g_malloc/g_slice. This means they will assert and the application will terminate abnormally in case of OOM. OOMs cannot be handled properly anyway, so it is more important to have a good memory limiting mechanism. * Memory limiting has been completely revised. Instead of approximating undo stack sizes using virtual methods (which is unprecise and comes with a performance penalty), we now use a common base class SciTECO::Object to count the memory required by all objects allocated within SciTECO. This is less precise than using global replacement new/deletes which would allow us to control allocations in all C++ code including Scintilla, but they are only supported as of C++14 (GCC 5) and adding compile-time checks would be cumbersome. In any case, we're missing Glib allocations (esp. strings). * As a platform-specific extension, on Linux/glibc we use mallinfo() to count the exact memory usage of the process. On Windows, we use GetProcessMemoryInfo() -- the latter implementation is currently UNTESTED. * We use g_malloc() for new/delete operators when there is malloc_trim() since g_slice does not free heap chunks properly (probably does its own mmap()ing), rendering malloc_trim() ineffective. We've also benchmarked g_slice on Linux/glib (malloc_trim() shouldn't be available elsewhere) and found that it brings no significant performance benefit. On all other platforms, we use g_slice since it is assumed that it at least does not hurt. The new g_slice based allocators should be tested on MSVCRT since I assume that they bring a significant performance benefit on Windows. * Memory limiting does now work in batch mode as well and is still enabled by default. * The old UndoTokenWithSize CRTP hack could be removed. UndoStack operations should be a bit faster now. But on the other hand, there will be an overhead due to repeated memory limit checking on every processed character. --- TODO | 26 -------------------------- 1 file changed, 26 deletions(-) (limited to 'TODO') diff --git a/TODO b/TODO index 7708132..e32bf31 100644 --- a/TODO +++ b/TODO @@ -39,19 +39,6 @@ Known Bugs: regex support, UNIX regex (unportable) or some other library. Perhaps allowing us to interpret the SciTECO matching language directly. - * the glib allocators are fundamentally broken: - throwing exceptions is unsafe from C-linkage callbacks. - We should abandon the custom allocators and rely on - SciTECO's memory limiting. - All C++ allocations could be based on g_malloc/g_slice so we - assert on OOM. Instead we improve memory limiting using platform-specific - API like malloc_info(). Since the remaining platforms are only obscure - ones, the overloaded C++ operators should be sufficient to count the - bulk of memory used. Since the necessary sized delete operators are - only available beginning with C++14, there'd have to be yet another - fallback that stores the memory chunk size at the beginning of the - heap object. - The UndoToken::get_size() workaround can be removed. * It is still possible to crash SciTECO using recursive functions, since they map to the C++ program's call stack. It is perhaps best to use another ValueStack as a stack of @@ -257,19 +244,6 @@ Optimizations: However, this would mean to make some more Cmdline methods public. The implementations of the States' commandline editing handlers could all be concentrated in cmdline.cpp. - * C++14 is supported by GCC 5 and supports new() and delete() - operators with a size argument. Replacing these operators - with versions using g_slice_alloc() and g_slice_free() should - speed up things, especially Q-Register handling and the undo - stack. - This compiler capability should be checked by the build system. - C++11 already allows sized allocators in a class. - Testing shows that this does not speed up things on Linux - and prevents freeing memory on command line termination - (it would be glibc-specific), so it should probably depend on - HAVE_MALLOC_TRIM. On all other platforms, it could be assumed - to be beneficial or at least not hurt. - Best is to test its effect on MSVCRT. * String::append() could be optimized by ORing a padding into the realloc() size (e.g. 0xFF). However, this has not proven effective on Linux/glibc -- cgit v1.2.3