From 1852a71f95a1bdfb1445b4ba27d45ffb00b60ec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Tue, 18 Sep 2018 15:24:12 +0200 Subject: [PATCH] [poincare] Remove TreePool::insert method, use Helper::Rotate instead This removes the use of an enormous buffer on the stack and uses instead a linear algorithm with one temporary data. --- poincare/Makefile | 1 + poincare/include/poincare/helpers.h | 18 +++++ poincare/include/poincare/tree_pool.h | 1 - poincare/src/helpers.cpp | 94 +++++++++++++++++++++++++++ poincare/src/tree_pool.cpp | 59 ++++------------- 5 files changed, 124 insertions(+), 49 deletions(-) create mode 100644 poincare/include/poincare/helpers.h create mode 100644 poincare/src/helpers.cpp diff --git a/poincare/Makefile b/poincare/Makefile index ff5cdc01d..e409014cf 100644 --- a/poincare/Makefile +++ b/poincare/Makefile @@ -34,6 +34,7 @@ objs += $(addprefix poincare/src/,\ objs += $(addprefix poincare/src/,\ init.o\ exception_checkpoint.o\ + helpers.o\ ) objs += $(addprefix poincare/src/,\ diff --git a/poincare/include/poincare/helpers.h b/poincare/include/poincare/helpers.h new file mode 100644 index 000000000..1195d6004 --- /dev/null +++ b/poincare/include/poincare/helpers.h @@ -0,0 +1,18 @@ +#ifndef POINCARE_HELPERS_H +#define POINCARE_HELPERS_H + +#include +#include + +namespace Poincare { + +namespace Helpers { + +size_t Gcd(size_t a, size_t b); +bool Rotate(uint32_t * dst, uint32_t * src, size_t len); + +} + +} + +#endif diff --git a/poincare/include/poincare/tree_pool.h b/poincare/include/poincare/tree_pool.h index e38cbf497..db577cf24 100644 --- a/poincare/include/poincare/tree_pool.h +++ b/poincare/include/poincare/tree_pool.h @@ -108,7 +108,6 @@ private: // Pool memory void * alloc(size_t size); void dealloc(TreeNode * ptr, size_t size); - bool insert(char * destination, char * source, size_t length); void moveNodes(TreeNode * destination, TreeNode * source, size_t moveLength); // Identifiers diff --git a/poincare/src/helpers.cpp b/poincare/src/helpers.cpp new file mode 100644 index 000000000..bceba5715 --- /dev/null +++ b/poincare/src/helpers.cpp @@ -0,0 +1,94 @@ +#include +#include + +namespace Poincare { + +namespace Helpers { + +size_t Gcd(size_t a, size_t b) { + int i = a; + int j = b; + do { + if (i == 0) { + return j; + } + if (j == 0) { + return i; + } + if (i > j) { + i = i - j*(i/j); + } else { + j = j - i*(j/i); + } + } while(true); +} + + +bool Rotate(uint32_t * dst, uint32_t * src, size_t len) { + /* This method "rotates" an array to insert data at src with length len at + * address dst. + * + * For instance, rotate(2, 4, 1) is: + * + * | 0 | 1 | 2 | 3 | 4 | 5 | 6 | + * ^¨¨¨¨¨¨¨^---^ + * Dst Src Len = 1 + * + * After rotation : + * | 0 | 1 | 4 | 2 | 3 | 5 | 6 | + * --- ¨¨¨¨¨¨¨ + */ + + if (len == 0 || src == dst || (dst > src && dst < src + len)) { + return false; + } + + /* We start with the first data to move at address a0 (we chose src but this + * does not matter), move it to its final address (a1 = a0 + len). We then + * move the data that was in a1 to its final address (a2) and so on, until we + * set the data at the starting address a0. + * This is a cycle of changes, and we have to make GCD(len, |dst-src]) such + * cycles, each starting at the previous cycle starting address + 1. + * This is a one-move per data algorithm, so it is O(dst - src) if dst > src, + * else O(src+len - dst). */ + + size_t numberOfCycles = Gcd(len, dst < src ? src - dst : dst - src); + size_t dstAddressOffset = dst < src ? len : dst - len - src; + + // We need the limit addresses of the data that will change + uint32_t * insertionZoneStart = dst < src ? dst : src; + uint32_t * insertionZoneEnd = dst < src ? src + len - 1 : dst - 1; + + uint32_t * cycleStartAddress; + uint32_t * moveSrcAddress; + uint32_t * moveDstAddress; + uint32_t tmpData; + uint32_t nextTmpData; + + for (size_t i = 0; i < numberOfCycles; i++) { + // Set the cycle starting source + cycleStartAddress = src + i; + assert(cycleStartAddress <= insertionZoneEnd); + moveSrcAddress = cycleStartAddress; + // Set the cycle starting destination, dstAddressOffset after the source + moveDstAddress = moveSrcAddress + dstAddressOffset; + if (moveDstAddress > insertionZoneEnd) { + moveDstAddress = insertionZoneStart + (moveDstAddress - insertionZoneEnd - 1); + } + tmpData = *moveSrcAddress; + do { + nextTmpData = *moveDstAddress; + *moveDstAddress = tmpData; + tmpData = nextTmpData; + moveSrcAddress = moveDstAddress; + moveDstAddress = moveSrcAddress + dstAddressOffset; + if (moveDstAddress > insertionZoneEnd) { + moveDstAddress = insertionZoneStart + (moveDstAddress - insertionZoneEnd - 1); + } + } while (moveSrcAddress != cycleStartAddress); + } + return true; +} + +} +} diff --git a/poincare/src/tree_pool.cpp b/poincare/src/tree_pool.cpp index 1bdb502d9..35ab533b0 100644 --- a/poincare/src/tree_pool.cpp +++ b/poincare/src/tree_pool.cpp @@ -1,6 +1,7 @@ #include -#include #include +#include +#include #include #include #include @@ -11,21 +12,6 @@ namespace Poincare { TreePool * TreePool::SharedStaticPool; -static void memmove32(uint32_t * dst, uint32_t * src, size_t len) { - if (src < dst && dst < src + len) { - /* Copy backwards to avoid overwrites */ - src += len; - dst += len; - while (len--) { - *--dst = *--src; - } - } else { - while (len--) { - *dst++ = *src++; - } - } -} - void TreePool::freeIdentifier(int identifier) { if (identifier >= 0 && identifier < MaxNumberOfNodes) { m_nodeForIdentifier[identifier] = nullptr; @@ -86,14 +72,16 @@ void TreePool::removeChildrenAndDestroy(TreeNode * nodeToDestroy, int nodeNumber } void TreePool::moveNodes(TreeNode * destination, TreeNode * source, size_t moveSize) { - if (source == destination || moveSize == 0) { - return; - } + assert(moveSize % 4 == 0); + assert((long)source % 4 == 0); + assert((long)destination % 4 == 0); - char * destinationAddress = reinterpret_cast(destination); - char * sourceAddress = reinterpret_cast(source); - if (insert(destinationAddress, sourceAddress, moveSize)) { - updateNodeForIdentifierFromNode(destinationAddress < sourceAddress ? destination : source); + uint32_t * src = reinterpret_cast(source); + uint32_t * dst = reinterpret_cast(destination); + size_t len = moveSize/4; + + if (Helpers::Rotate(dst, src, len)) { + updateNodeForIdentifierFromNode(dst < src ? destination : source); } } @@ -155,31 +143,6 @@ void TreePool::dealloc(TreeNode * node, size_t size) { updateNodeForIdentifierFromNode(node); } -bool TreePool::insert(char * destination, char * source, size_t length) { - if (source == destination || (destination > source && destination < source + length)) { - return false; - } - - assert(length % 4 == 0); - assert((long)source % 4 == 0); - assert((long)destination % 4 == 0); - - uint32_t * src = reinterpret_cast(source); - uint32_t * dst = reinterpret_cast(destination); - size_t len = length/4; - char tempBuffer[BufferSize]; - uint32_t * tmp = reinterpret_cast(tempBuffer); - memmove32(reinterpret_cast(tmp), src, len); - if (dst < src) { - memmove32(dst + len, dst, src - dst); - memmove32(dst, tmp, len); - } else { - memmove32(src, src + len, dst - (src + len)); - memmove32(dst - len, tmp, len); - } - return true; -} - void TreePool::addGhostChildrenAndRename(TreeNode * node) { /* Ensure the pool is syntaxically correct by creating ghost children for * nodes that have a fixed, non-zero number of children. */