Conversation
| decrementParent(node->parent, key); | ||
| if (node->parent && node->parent->refCounter == 0) { | ||
| persistent_ptr<Node256> nodeParent; | ||
| pmemobj_free(nodeParent->children[0].raw_ptr()); |
There was a problem hiding this comment.
nodeParent is declared above, but not initialized. How can this work? Do I miss something?
| size_t keyCalc = key[tree->treeRoot->keySize - node->depth - 1]; | ||
| std::bitset<8> x(keyCalc); | ||
| node->children[keyCalc] = nullptr; | ||
| node->refCounter--; |
There was a problem hiding this comment.
I can see refCounter is atomic, shouldn't we initialize it when loading an existing tree?
| @@ -272,27 +271,50 @@ void ARTree::Put(const char *key, int32_t keyBytes, const char *value, | |||
| * TODO: decrement value std::atomic<int> refCounter in parent and free it if | |||
There was a problem hiding this comment.
Todo can be removed? Also the one in line 378?
| std::bitset<8> x(keyCalc); | ||
| node->children[keyCalc] = nullptr; | ||
| node->refCounter--; | ||
| if (node->refCounter == 0) { |
There was a problem hiding this comment.
Shouldn't this be an compare and swap instruction? Otherwise we can have a situation in which two threads decrement refCounter in parallel before checking for zero in next line. In this situation, none will decrement the parent
| void ARTree::decrementParent(persistent_ptr<Node> node) {} | ||
| void ARTree::decrementParent(persistent_ptr<Node256> node, const char *key) { | ||
| size_t keyCalc = key[tree->treeRoot->keySize - node->depth - 1]; | ||
| std::bitset<8> x(keyCalc); |
There was a problem hiding this comment.
looks like x is not used for anything
| decrementParent(valPrstPtr->parent); | ||
| void ARTree::removeFromTree(persistent_ptr<NodeLeafCompressed> compressed, | ||
| const char *key) { | ||
| persistent_ptr<Node256> current = compressed->parent; |
There was a problem hiding this comment.
name "current" is confusing, maybe this variable should be called "parent"?
| // remove ValueWrapper | ||
| pmemobj_free(valPrstPtr.raw_ptr()); | ||
| } else if (valPrstPtr->location == DISK) { | ||
| // TODO: jradtke need to confirm if no extra action required here |
There was a problem hiding this comment.
I suppose that comment should be updated, for sure extra actions are needed, right?
3 participants
Initial implementation of removal from ARTree using pmemobj_free.