less malloc and less copying for skiplist index elements

arangod/SkipLists/skiplistIndex.cpp

@@ -65,21 +65,22 @@
 ////////////////////////////////////////////////////////////////////////////////
 
 static int CompareKeyElement (TRI_shaped_json_t const* left,
-                              TRI_skiplist_index_element_t* right,
+                              TRI_skiplist_index_element_t const* right,
                               size_t rightPosition,
                               TRI_shaper_t* shaper) {
-  int result;
-
   TRI_ASSERT(nullptr != left);
   TRI_ASSERT(nullptr != right);
-  result = TRI_CompareShapeTypes(nullptr,
+
-                                 nullptr,
+  auto rightSubobjects = SkiplistIndex_Subobjects(right);
-                                 left,
+
-                                 shaper,
+  int result = TRI_CompareShapeTypes(nullptr,
-                                 right->_document->getShapedJsonPtr(),
+                                     nullptr,
-                                 &right->_subObjects[rightPosition],
+                                     left,
-                                 nullptr,
+                                     shaper,
-                                 shaper);
+                                     right->_document->getShapedJsonPtr(),
+                                     &rightSubobjects[rightPosition],
+                                     nullptr,
+                                     shaper);
 
   // ...........................................................................
   // In the above function CompareShapeTypes we use strcmp which may
@@ -102,20 +103,23 @@ static int CompareKeyElement (TRI_shaped_json_t const* left,
 /// @brief compares elements, version with proper types
 ////////////////////////////////////////////////////////////////////////////////
 
-static int CompareElementElement (TRI_skiplist_index_element_t* left,
+static int CompareElementElement (TRI_skiplist_index_element_t const* left,
                                   size_t leftPosition,
-                                  TRI_skiplist_index_element_t* right,
+                                  TRI_skiplist_index_element_t const* right,
                                   size_t rightPosition,
                                   TRI_shaper_t* shaper) {
   TRI_ASSERT(nullptr != left);
   TRI_ASSERT(nullptr != right);
+  
+  auto leftSubobjects = SkiplistIndex_Subobjects(left);
+  auto rightSubobjects = SkiplistIndex_Subobjects(right);
 
   int result = TRI_CompareShapeTypes(left->_document->getShapedJsonPtr(),
-                                     &left->_subObjects[leftPosition],
+                                     &leftSubobjects[leftPosition],
                                      nullptr,
                                      shaper,
                                      right->_document->getShapedJsonPtr(),
-                                     &right->_subObjects[rightPosition],
+                                     &rightSubobjects[rightPosition],
                                      nullptr,
                                      shaper);
 
@@ -145,8 +149,8 @@ static int CmpElmElm (void* sli,
                       void* right,
                       triagens::basics::SkipListCmpType cmptype) {
 
-  TRI_skiplist_index_element_t* leftElement = static_cast<TRI_skiplist_index_element_t*>(left);
+  auto leftElement = static_cast<TRI_skiplist_index_element_t const*>(left);
-  TRI_skiplist_index_element_t* rightElement = static_cast<TRI_skiplist_index_element_t*>(right);
+  auto rightElement = static_cast<TRI_skiplist_index_element_t const*>(right);
   TRI_shaper_t* shaper;
 
   TRI_ASSERT(nullptr != left);
@@ -163,14 +167,12 @@ static int CmpElmElm (void* sli,
 
   SkiplistIndex* skiplistindex = static_cast<SkiplistIndex*>(sli);
   shaper = skiplistindex->_collection->getShaper();  // ONLY IN INDEX, PROTECTED by RUNTIME
-  int compareResult;
-
   for (size_t j = 0;  j < skiplistindex->_numFields;  j++) {
-    compareResult = CompareElementElement(leftElement,
+    int compareResult = CompareElementElement(leftElement,
-                                          j,
+                                              j,
-                                          rightElement,
+                                              rightElement,
-                                          j,
+                                              j,
-                                          shaper);
+                                              shaper);
 
     if (compareResult != 0) {
       return compareResult;
@@ -190,8 +192,8 @@ static int CmpElmElm (void* sli,
   }
 
   // We break this tie in the key comparison by looking at the key:
-  compareResult = strcmp(TRI_EXTRACT_MARKER_KEY(leftElement->_document),    // ONLY IN INDEX, PROTECTED by RUNTIME
+  int compareResult = strcmp(TRI_EXTRACT_MARKER_KEY(leftElement->_document),    // ONLY IN INDEX, PROTECTED by RUNTIME
-                         TRI_EXTRACT_MARKER_KEY(rightElement->_document));  // ONLY IN INDEX, PROTECTED by RUNTIME
+                             TRI_EXTRACT_MARKER_KEY(rightElement->_document));  // ONLY IN INDEX, PROTECTED by RUNTIME
 
   if (compareResult < 0) {
     return -1;
@@ -209,15 +211,14 @@ static int CmpElmElm (void* sli,
 static int CmpKeyElm (void* sli,
                       void* left,
                       void* right) {
-  TRI_skiplist_index_key_t* leftKey = static_cast<TRI_skiplist_index_key_t*>(left);
+  auto leftKey = static_cast<TRI_skiplist_index_key_t const*>(left);
-  TRI_skiplist_index_element_t* rightElement = static_cast<TRI_skiplist_index_element_t*>(right);
+  auto rightElement = static_cast<TRI_skiplist_index_element_t const*>(right);
-  TRI_shaper_t* shaper;
 
   TRI_ASSERT(nullptr != left);
   TRI_ASSERT(nullptr != right);
 
   SkiplistIndex* skiplistindex = static_cast<SkiplistIndex*>(sli);
-  shaper = skiplistindex->_collection->getShaper();  // ONLY IN INDEX, PROTECTED by RUNTIME
+  TRI_shaper_t* shaper = skiplistindex->_collection->getShaper();  // ONLY IN INDEX, PROTECTED by RUNTIME
 
   // Note that the key might contain fewer fields than there are indexed
   // attributes, therefore we only run the following loop to
@@ -238,29 +239,10 @@ static int CmpKeyElm (void* sli,
 ////////////////////////////////////////////////////////////////////////////////
 
 static void FreeElm (void* e) {
-  TRI_skiplist_index_element_t* element = static_cast<TRI_skiplist_index_element_t*>(e);
+  auto element = static_cast<TRI_skiplist_index_element_t*>(e);
-  TRI_Free(TRI_UNKNOWN_MEM_ZONE, element->_subObjects);
   TRI_Free(TRI_UNKNOWN_MEM_ZONE, element);
 }
 
-static int CopyElement (SkiplistIndex* skiplistindex,
-                        TRI_skiplist_index_element_t* leftElement,
-                        TRI_skiplist_index_element_t* rightElement) {
-  TRI_ASSERT(nullptr != leftElement && nullptr != rightElement);
-
-  leftElement->_document   = rightElement->_document;
-  leftElement->_subObjects = static_cast<TRI_shaped_sub_t*>(TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, sizeof(TRI_shaped_sub_t) * skiplistindex->_numFields, false));
-
-  if (leftElement->_subObjects == nullptr) {
-    return TRI_ERROR_OUT_OF_MEMORY;
-  }
-
-  memcpy(leftElement->_subObjects, rightElement->_subObjects,
-         sizeof(TRI_shaped_sub_t) * skiplistindex->_numFields);
-
-  return TRI_ERROR_NO_ERROR;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief return the current interval that the iterator points at
 ////////////////////////////////////////////////////////////////////////////////
@@ -843,53 +825,40 @@ TRI_skiplist_iterator_t* SkiplistIndex_find (
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-/// @brief inserts a data element into a unique skip list
+/// @brief inserts a data element into the skip list
+/// ownership for the element is transferred to the index
 ////////////////////////////////////////////////////////////////////////////////
 
 int SkiplistIndex_insert (SkiplistIndex* skiplistIndex,
                           TRI_skiplist_index_element_t* element) {
-  TRI_skiplist_index_element_t* copy = static_cast<TRI_skiplist_index_element_t*>(TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, sizeof(TRI_skiplist_index_element_t), false));
+  int res = skiplistIndex->skiplist->insert(element);
-
-  if (nullptr == copy) {
-    return TRI_ERROR_OUT_OF_MEMORY;
-  }
-
-  int res = CopyElement(skiplistIndex, copy, element);
 
   if (res != TRI_ERROR_NO_ERROR) {
-    TRI_Free(TRI_UNKNOWN_MEM_ZONE, copy);
+    TRI_Free(TRI_UNKNOWN_MEM_ZONE, element);
-    return res;
   }
 
-  res = skiplistIndex->skiplist->insert(copy);
+  return res;
-
-  if (res != TRI_ERROR_NO_ERROR) {
-    TRI_Free(TRI_UNKNOWN_MEM_ZONE, copy->_subObjects);
-    TRI_Free(TRI_UNKNOWN_MEM_ZONE, copy);
-    return res;
-  }
-
-  return TRI_ERROR_NO_ERROR;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief removes an entry from the skip list
+/// ownership for the element is transferred to the index
 ////////////////////////////////////////////////////////////////////////////////
 
 int SkiplistIndex_remove (SkiplistIndex* skiplistIndex,
                           TRI_skiplist_index_element_t* element) {
-  int result;
+  int res = skiplistIndex->skiplist->remove(element);
 
-  result = skiplistIndex->skiplist->remove(element);
+  TRI_Free(TRI_UNKNOWN_MEM_ZONE, element);
 
-  if (result == TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND) {
+  if (res == TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND) {
     // This is for the case of a rollback in an aborted transaction.
     // We silently ignore the fact that the document was not there.
     // This could also be useful for the case of a sparse index.
     return TRI_ERROR_NO_ERROR;
   }
 
-  return result;
+  return res;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -905,12 +874,9 @@ uint64_t SkiplistIndex_getNrUsed (SkiplistIndex* skiplistIndex) {
 ////////////////////////////////////////////////////////////////////////////////
 
 size_t SkiplistIndex_memoryUsage (SkiplistIndex const* skiplistIndex) {
-  size_t const elementSize = skiplistIndex->_numFields * 
-                             (sizeof(TRI_skiplist_index_element_t) + sizeof(TRI_shaped_sub_t));
-
   return sizeof(SkiplistIndex) + 
          skiplistIndex->skiplist->memoryUsage() +
-         skiplistIndex->skiplist->getNrUsed() * elementSize;
+         skiplistIndex->skiplist->getNrUsed() * SkiplistIndex_ElementSize(skiplistIndex);
 }
 
 // -----------------------------------------------------------------------------

arangod/SkipLists/skiplistIndex.h

@@ -66,9 +66,9 @@ typedef struct {
 TRI_skiplist_index_key_t;
 
 typedef struct {
-  TRI_shaped_sub_t* _subObjects;    // list of shaped json objects which the
-                                    // collection should know about
   struct TRI_doc_mptr_t* _document; // master document pointer
+  // note: the index element also contains a list of shaped subs as follows
+  // TRI_shaped_sub_t* _subObjects; 
 }
 TRI_skiplist_index_element_t;
 
@@ -166,6 +166,30 @@ uint64_t SkiplistIndex_getNrUsed (SkiplistIndex*);
 
 size_t SkiplistIndex_memoryUsage (SkiplistIndex const*);
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief return the memory size of a skiplist index element
+////////////////////////////////////////////////////////////////////////////////
+
+inline size_t SkiplistIndex_ElementSize (SkiplistIndex const* idx) {
+  return sizeof(TRI_doc_mptr_t*) + (sizeof(TRI_shaped_sub_t) * idx->_numFields);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief return the base address for the shaped subs inside an element
+////////////////////////////////////////////////////////////////////////////////
+  
+inline TRI_shaped_sub_t const* SkiplistIndex_Subobjects (TRI_skiplist_index_element_t const* element) {
+  return reinterpret_cast<TRI_shaped_sub_t const*>(reinterpret_cast<char const*>(element) + sizeof(TRI_doc_mptr_t*));
+}
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief return the base address for the shaped subs inside an element
+////////////////////////////////////////////////////////////////////////////////
+  
+inline TRI_shaped_sub_t* SkiplistIndex_Subobjects (TRI_skiplist_index_element_t* element) {
+  return reinterpret_cast<TRI_shaped_sub_t*>(reinterpret_cast<char*>(element) + sizeof(TRI_doc_mptr_t*));
+}
+
 #endif
 
 // -----------------------------------------------------------------------------

arangod/VocBase/index.cpp

@@ -1222,6 +1222,8 @@ static int SkiplistIndexHelper (TRI_skiplist_index_t const* skiplistIndex,
   skiplistElement->_document = const_cast<TRI_doc_mptr_t*>(document);
   char const* ptr = skiplistElement->_document->getShapedJsonPtr();  // ONLY IN INDEX, PROTECTED by RUNTIME
 
+  auto subObjects = SkiplistIndex_Subobjects(skiplistElement);
+
   for (size_t j = 0; j < skiplistIndex->_paths._length; ++j) {
     TRI_shape_pid_t shape = *((TRI_shape_pid_t*) TRI_AtVector(&skiplistIndex->_paths, j));
 
@@ -1234,7 +1236,7 @@ static int SkiplistIndexHelper (TRI_skiplist_index_t const* skiplistIndex,
     if (acc == nullptr || acc->_resultSid == TRI_SHAPE_ILLEGAL) {
       // OK, the document does not contain the attributed needed by 
       // the index, are we sparse?
-      skiplistElement->_subObjects[j]._sid = BasicShapes::TRI_SHAPE_SID_NULL; 
+      subObjects[j]._sid = BasicShapes::TRI_SHAPE_SID_NULL; 
 
       res = TRI_ERROR_ARANGO_INDEX_DOCUMENT_ATTRIBUTE_MISSING;
 
@@ -1267,7 +1269,7 @@ static int SkiplistIndexHelper (TRI_skiplist_index_t const* skiplistIndex,
     // Store the field
     // .........................................................................
 
-    TRI_FillShapedSub(&skiplistElement->_subObjects[j], &shapedObject, ptr);
+    TRI_FillShapedSub(&subObjects[j], &shapedObject, ptr);
   }
 
   return res;
@@ -1288,14 +1290,13 @@ static int InsertSkiplistIndex (TRI_index_t* idx,
   // These will be used for comparisions
   // ...........................................................................
 
-  TRI_skiplist_index_element_t skiplistElement;
+  auto skiplistElement = static_cast<TRI_skiplist_index_element_t*>(TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, SkiplistIndex_ElementSize(skiplistIndex->_skiplistIndex), false));
-  skiplistElement._subObjects = static_cast<TRI_shaped_sub_t*>(TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, sizeof(TRI_shaped_sub_t) * skiplistIndex->_paths._length, false));
 
-  if (skiplistElement._subObjects == nullptr) {
+  if (skiplistElement == nullptr) {
     return TRI_ERROR_OUT_OF_MEMORY;
   }
 
-  int res = SkiplistIndexHelper(skiplistIndex, &skiplistElement, doc);
+  int res = SkiplistIndexHelper(skiplistIndex, skiplistElement, doc);
   // ...........................................................................
   // most likely the cause of this error is that the index is sparse
   // and not all attributes the index needs are set -- so the document
@@ -1314,7 +1315,7 @@ static int InsertSkiplistIndex (TRI_index_t* idx,
 
   if (res == TRI_ERROR_ARANGO_INDEX_DOCUMENT_ATTRIBUTE_MISSING) {
     if (idx->_sparse) {
-      TRI_Free(TRI_UNKNOWN_MEM_ZONE, skiplistElement._subObjects);
+      TRI_Free(TRI_UNKNOWN_MEM_ZONE, skiplistElement);
       return TRI_ERROR_NO_ERROR;
     }
 
@@ -1322,23 +1323,13 @@ static int InsertSkiplistIndex (TRI_index_t* idx,
   }
 
   if (res != TRI_ERROR_NO_ERROR) {
+    TRI_Free(TRI_UNKNOWN_MEM_ZONE, skiplistElement);
     return res;
   }
 
-  // ...........................................................................
+  // insert into the index. the memory for the element will be owned or freed
-  // Fill the json field list from the document for skiplist index
+  // by the index
-  // ...........................................................................
+  return SkiplistIndex_insert(skiplistIndex->_skiplistIndex, skiplistElement);
-
-  res = SkiplistIndex_insert(skiplistIndex->_skiplistIndex, &skiplistElement);
-
-  // ...........................................................................
-  // Memory which has been allocated to skiplistElement.fields remains allocated
-  // contents of which are stored in the hash array.
-  // ...........................................................................
-
-  TRI_Free(TRI_UNKNOWN_MEM_ZONE, skiplistElement._subObjects);
-
-  return res;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1425,14 +1416,9 @@ static int RemoveSkiplistIndex (TRI_index_t* idx,
 
   TRI_skiplist_index_t* skiplistIndex = (TRI_skiplist_index_t*) idx;
 
-  // ...........................................................................
+  auto skiplistElement = static_cast<TRI_skiplist_index_element_t*>(TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, SkiplistIndex_ElementSize(skiplistIndex->_skiplistIndex), false));
-  // Allocate some memory for the SkiplistIndexElement structure
-  // ...........................................................................
 
-  TRI_skiplist_index_element_t skiplistElement;
+  if (skiplistElement == nullptr) {
-  skiplistElement._subObjects = static_cast<TRI_shaped_sub_t*>(TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, sizeof(TRI_shaped_sub_t) * skiplistIndex->_paths._length, false));
-
-  if (skiplistElement._subObjects == nullptr) {
     return TRI_ERROR_OUT_OF_MEMORY;
   }
 
@@ -1440,7 +1426,7 @@ static int RemoveSkiplistIndex (TRI_index_t* idx,
   // Fill the json field list from the document
   // ..........................................................................
 
-  int res = SkiplistIndexHelper(skiplistIndex, &skiplistElement, doc);
+  int res = SkiplistIndexHelper(skiplistIndex, skiplistElement, doc);
 
   // ..........................................................................
   // Error returned generally implies that the document never was part of the
@@ -1448,7 +1434,7 @@ static int RemoveSkiplistIndex (TRI_index_t* idx,
   // ..........................................................................
   if (res == TRI_ERROR_ARANGO_INDEX_DOCUMENT_ATTRIBUTE_MISSING) {
     if (idx->_sparse) {
-      TRI_Free(TRI_UNKNOWN_MEM_ZONE, skiplistElement._subObjects);
+      TRI_Free(TRI_UNKNOWN_MEM_ZONE, skiplistElement);
       return TRI_ERROR_NO_ERROR;
     }
 
@@ -1456,22 +1442,14 @@ static int RemoveSkiplistIndex (TRI_index_t* idx,
   }
 
   if (res != TRI_ERROR_NO_ERROR) {
+    TRI_Free(TRI_UNKNOWN_MEM_ZONE, skiplistElement);
     return res;
   }
 
-  // ...........................................................................
+  // attempt the removal for skiplist indexes
-  // Attempt the removal for skiplist indexes
+  // ownership for the index element is transferred to the index
-  // ...........................................................................
 
-  res = SkiplistIndex_remove(skiplistIndex->_skiplistIndex, &skiplistElement);
+  return SkiplistIndex_remove(skiplistIndex->_skiplistIndex, skiplistElement);
-
-  // ...........................................................................
-  // Deallocate memory allocated to skiplistElement.fields above
-  // ...........................................................................
-
-  TRI_Free(TRI_UNKNOWN_MEM_ZONE, skiplistElement._subObjects);
-
-  return res;
 }
 
 ////////////////////////////////////////////////////////////////////////////////

arangod/VocBase/voc-shaper.cpp

@@ -1153,11 +1153,11 @@ static void DestroyAttributesVector (TRI_vector_t* vector) {
 ////////////////////////////////////////////////////////////////////////////////
 
 int TRI_CompareShapeTypes (char const* leftDocument,
-                           TRI_shaped_sub_t* leftObject,
+                           TRI_shaped_sub_t const* leftObject,
                            TRI_shaped_json_t const* leftShaped,
                            TRI_shaper_t* leftShaper,
                            char const* rightDocument,
-                           TRI_shaped_sub_t* rightObject,
+                           TRI_shaped_sub_t const* rightObject,
                            TRI_shaped_json_t const* rightShaped,
                            TRI_shaper_t* rightShaper) {
 

arangod/VocBase/voc-shaper.h

@@ -126,11 +126,11 @@ bool TRI_ExtractShapedJsonVocShaper (TRI_shaper_t* s,
 ////////////////////////////////////////////////////////////////////////////////
 
 int TRI_CompareShapeTypes (char const* leftDocument,
-                           TRI_shaped_sub_t* leftObject,
+                           TRI_shaped_sub_t const* leftObject,
                            TRI_shaped_json_t const* leftShaped,
                            TRI_shaper_t* leftShaper,
                            char const* rightDocument,
-                           TRI_shaped_sub_t* rightObject,
+                           TRI_shaped_sub_t const* rightObject,
                            TRI_shaped_json_t const* rightShaped,
                            TRI_shaper_t* rightShaper);