diff --git a/arangod/SkipLists/skiplistIndex.cpp b/arangod/SkipLists/skiplistIndex.cpp index 8ebbc81b12..5585a28c60 100644 --- a/arangod/SkipLists/skiplistIndex.cpp +++ b/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, - left, - shaper, - right->_document->getShapedJsonPtr(), - &right->_subObjects[rightPosition], - nullptr, - shaper); + + auto rightSubobjects = SkiplistIndex_Subobjects(right); + + int result = TRI_CompareShapeTypes(nullptr, + nullptr, + left, + 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(left); - TRI_skiplist_index_element_t* rightElement = static_cast(right); + auto leftElement = static_cast(left); + auto rightElement = static_cast(right); TRI_shaper_t* shaper; TRI_ASSERT(nullptr != left); @@ -163,14 +167,12 @@ static int CmpElmElm (void* sli, SkiplistIndex* skiplistindex = static_cast(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, - j, - rightElement, - j, - shaper); + int compareResult = CompareElementElement(leftElement, + j, + rightElement, + j, + 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 - TRI_EXTRACT_MARKER_KEY(rightElement->_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 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(left); - TRI_skiplist_index_element_t* rightElement = static_cast(right); - TRI_shaper_t* shaper; + auto leftKey = static_cast(left); + auto rightElement = static_cast(right); TRI_ASSERT(nullptr != left); TRI_ASSERT(nullptr != right); SkiplistIndex* skiplistindex = static_cast(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(e); - TRI_Free(TRI_UNKNOWN_MEM_ZONE, element->_subObjects); + auto element = static_cast(e); 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_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_Allocate(TRI_UNKNOWN_MEM_ZONE, sizeof(TRI_skiplist_index_element_t), false)); - - if (nullptr == copy) { - return TRI_ERROR_OUT_OF_MEMORY; - } - - int res = CopyElement(skiplistIndex, copy, element); + int res = skiplistIndex->skiplist->insert(element); if (res != TRI_ERROR_NO_ERROR) { - TRI_Free(TRI_UNKNOWN_MEM_ZONE, copy); - return res; + TRI_Free(TRI_UNKNOWN_MEM_ZONE, element); } - res = skiplistIndex->skiplist->insert(copy); - - 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; + return res; } //////////////////////////////////////////////////////////////////////////////// /// @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); } // ----------------------------------------------------------------------------- diff --git a/arangod/SkipLists/skiplistIndex.h b/arangod/SkipLists/skiplistIndex.h index f15ec12f14..68aee437ce 100644 --- a/arangod/SkipLists/skiplistIndex.h +++ b/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(reinterpret_cast(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(reinterpret_cast(element) + sizeof(TRI_doc_mptr_t*)); +} + #endif // ----------------------------------------------------------------------------- diff --git a/arangod/VocBase/index.cpp b/arangod/VocBase/index.cpp index 991b7331a5..369497f3bb 100644 --- a/arangod/VocBase/index.cpp +++ b/arangod/VocBase/index.cpp @@ -1222,6 +1222,8 @@ static int SkiplistIndexHelper (TRI_skiplist_index_t const* skiplistIndex, skiplistElement->_document = const_cast(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; - skiplistElement._subObjects = static_cast(TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, sizeof(TRI_shaped_sub_t) * skiplistIndex->_paths._length, false)); + auto skiplistElement = static_cast(TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, SkiplistIndex_ElementSize(skiplistIndex->_skiplistIndex), 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; } - // ........................................................................... - // Fill the json field list from the document for skiplist index - // ........................................................................... - - 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; + // insert into the index. the memory for the element will be owned or freed + // by the index + return SkiplistIndex_insert(skiplistIndex->_skiplistIndex, skiplistElement); } //////////////////////////////////////////////////////////////////////////////// @@ -1425,14 +1416,9 @@ static int RemoveSkiplistIndex (TRI_index_t* idx, TRI_skiplist_index_t* skiplistIndex = (TRI_skiplist_index_t*) idx; - // ........................................................................... - // Allocate some memory for the SkiplistIndexElement structure - // ........................................................................... + auto skiplistElement = static_cast(TRI_Allocate(TRI_UNKNOWN_MEM_ZONE, SkiplistIndex_ElementSize(skiplistIndex->_skiplistIndex), false)); - TRI_skiplist_index_element_t skiplistElement; - skiplistElement._subObjects = static_cast(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; } @@ -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); - - // ........................................................................... - // Deallocate memory allocated to skiplistElement.fields above - // ........................................................................... - - TRI_Free(TRI_UNKNOWN_MEM_ZONE, skiplistElement._subObjects); - - return res; + return SkiplistIndex_remove(skiplistIndex->_skiplistIndex, skiplistElement); } //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/VocBase/voc-shaper.cpp b/arangod/VocBase/voc-shaper.cpp index ead8c3e712..e8e4d1c4ea 100644 --- a/arangod/VocBase/voc-shaper.cpp +++ b/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) { diff --git a/arangod/VocBase/voc-shaper.h b/arangod/VocBase/voc-shaper.h index 47ff18d09d..964130cbea 100644 --- a/arangod/VocBase/voc-shaper.h +++ b/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);