From 0a1c6a3d1debcfe34d11ac1765316785f1f61236 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?L=C3=A9a=20Saviot?= <lea.saviot@numworks.com>
Date: Tue, 26 Nov 2019 16:33:26 +0100
Subject: [PATCH] [poincare/tree_pool] Store node offsets, not node pointers

This saves place because we store uint16_t, not uint32_t
---
 liba/include/stdint.h                 |  2 ++
 poincare/include/poincare/tree_pool.h |  9 ++++++---
 poincare/src/tree_pool.cpp            | 14 ++++++++++----
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/liba/include/stdint.h b/liba/include/stdint.h
index ef904e710..1b6397153 100644
--- a/liba/include/stdint.h
+++ b/liba/include/stdint.h
@@ -26,6 +26,8 @@ typedef int64_t int_fast64_t;
 
 typedef uint8_t uint_least8_t;
 
+#define UINT16_MAX 0xffff
+
 #define INT16_MAX 0x7fff
 #define INT16_MIN (-INT16_MAX-1)
 
diff --git a/poincare/include/poincare/tree_pool.h b/poincare/include/poincare/tree_pool.h
index 36bce5366..60cd9a70d 100644
--- a/poincare/include/poincare/tree_pool.h
+++ b/poincare/include/poincare/tree_pool.h
@@ -28,8 +28,7 @@ public:
   // Node
   TreeNode * node(int identifier) const {
     assert(identifier >= 0 && identifier < MaxNumberOfNodes);
-    assert(m_nodeForIdentifier[identifier] != nullptr);
-    return m_nodeForIdentifier[identifier];
+    return const_cast<TreeNode *>(reinterpret_cast<const TreeNode *>(m_alignedBuffer + m_nodeForIdentifierOffset[identifier]));
   }
 
   // Pool memory
@@ -52,6 +51,8 @@ public:
 private:
   constexpr static int BufferSize = 32768;
   constexpr static int MaxNumberOfNodes = BufferSize/sizeof(TreeNode);
+  constexpr static int k_maxNodeOffset = BufferSize/ByteAlignment;
+
   static TreePool * SharedStaticPool;
 
   // TreeNode
@@ -144,7 +145,9 @@ private:
   AlignedNodeBuffer m_alignedBuffer[BufferSize/ByteAlignment];
   char * m_cursor;
   IdentifierStack m_identifiers;
-  TreeNode * m_nodeForIdentifier[MaxNumberOfNodes];
+  uint16_t m_nodeForIdentifierOffset[MaxNumberOfNodes];
+  static_assert(k_maxNodeOffset < UINT16_MAX && sizeof(m_nodeForIdentifierOffset[0]) == sizeof(uint16_t),
+        "The tree pool node offsets in m_nodeForIdentifierOffset cannot be written with the chosen data size (uint16_t)");
 };
 
 }
diff --git a/poincare/src/tree_pool.cpp b/poincare/src/tree_pool.cpp
index 476c102ab..360588443 100644
--- a/poincare/src/tree_pool.cpp
+++ b/poincare/src/tree_pool.cpp
@@ -15,7 +15,11 @@ TreePool * TreePool::SharedStaticPool = nullptr;
 
 void TreePool::freeIdentifier(int identifier) {
   if (identifier >= 0 && identifier < MaxNumberOfNodes) {
-    m_nodeForIdentifier[identifier] = nullptr; // TODO: We do not really need to do this, but it cleaner...
+    /* We could clean m_nodeForIdentifierOffset[identifier] to a default offset
+     * (for instance BufferSize) to be able to return nullptr when we access an
+     * inexisting cleaned node. However, transforming a default offset to a
+     * nullptr tree node adds one check per "get node", which is quite
+     * unefficient. We thus do nothing. */
     m_identifiers.push(identifier);
   }
 }
@@ -135,7 +139,7 @@ void TreePool::dealloc(TreeNode * node, size_t size) {
   );
   m_cursor -= size;
 
-  // Step 2: Update m_nodeForIdentifier for all nodes downstream
+  // Step 2: Update m_nodeForIdentifierOffset for all nodes downstream
   updateNodeForIdentifierFromNode(node);
 }
 
@@ -151,13 +155,15 @@ void TreePool::registerNode(TreeNode * node) {
   int nodeID = node->identifier();
   if (nodeID >= 0) {
     assert(nodeID < MaxNumberOfNodes);
-    m_nodeForIdentifier[nodeID] = node;
+    assert((((char *)node) - ((char *)m_alignedBuffer)) / ByteAlignment < k_maxNodeOffset); // Check that the offset can be stored in a uint16_t
+    m_nodeForIdentifierOffset[nodeID] = (((char *)node) - (char *)m_alignedBuffer)/ByteAlignment;
   }
 }
 
 void TreePool::updateNodeForIdentifierFromNode(TreeNode * node) {
   for (TreeNode * n : Nodes(node)) {
-    m_nodeForIdentifier[n->identifier()] = n;
+    assert((((char *)node) - ((char *)m_alignedBuffer))/ByteAlignment < k_maxNodeOffset); // Check that the offset can be stored in a uint16_t
+    m_nodeForIdentifierOffset[n->identifier()] = (((char *)n) - (char *)m_alignedBuffer)/ByteAlignment;
   }
 }