From 71ba490cf8bdc32ace52e2765c6a02f2b8094416 Mon Sep 17 00:00:00 2001 From: Stephen Atherton Date: Tue, 2 Jul 2019 00:58:43 -0700 Subject: [PATCH] Removed use of the C "struct hack" as it is not valid C++. Replaced zero-length members with functions returning a pointer for arrays or a reference for single members. --- fdbserver/DeltaTree.h | 56 ++++++++++++++++++------------ fdbserver/VersionedBTree.actor.cpp | 50 ++++++++++++++++---------- 2 files changed, 66 insertions(+), 40 deletions(-) diff --git a/fdbserver/DeltaTree.h b/fdbserver/DeltaTree.h index 4a9bee5c98..6797d87a77 100644 --- a/fdbserver/DeltaTree.h +++ b/fdbserver/DeltaTree.h @@ -69,6 +69,7 @@ // // Retrieves the previously stored boolean // bool getPrefixSource() const; // +#pragma pack(push,1) template struct DeltaTree { @@ -76,36 +77,47 @@ struct DeltaTree { return std::numeric_limits::max(); }; -#pragma pack(push,1) struct Node { OffsetT leftChildOffset; OffsetT rightChildOffset; - DeltaT delta[0]; + + inline DeltaT & delta() { + return *(DeltaT *)(this + 1); + }; + + inline const DeltaT & delta() const { + return *(const DeltaT *)(this + 1); + }; Node * rightChild() const { - //printf("Node(%p): leftOffset=%d rightOffset=%d deltaSize=%d\n", this, (int)leftChildOffset, (int)rightChildOffset, (int)delta->size()); - return rightChildOffset == 0 ? nullptr : (Node *)((uint8_t *)delta + rightChildOffset); + //printf("Node(%p): leftOffset=%d rightOffset=%d deltaSize=%d\n", this, (int)leftChildOffset, (int)rightChildOffset, (int)delta().size()); + return rightChildOffset == 0 ? nullptr : (Node *)((uint8_t *)&delta() + rightChildOffset); } Node * leftChild() const { - //printf("Node(%p): leftOffset=%d rightOffset=%d deltaSize=%d\n", this, (int)leftChildOffset, (int)rightChildOffset, (int)delta->size()); - return leftChildOffset == 0 ? nullptr : (Node *)((uint8_t *)delta + leftChildOffset); + //printf("Node(%p): leftOffset=%d rightOffset=%d deltaSize=%d\n", this, (int)leftChildOffset, (int)rightChildOffset, (int)delta().size()); + return leftChildOffset == 0 ? nullptr : (Node *)((uint8_t *)&delta() + leftChildOffset); } int size() const { - return sizeof(Node) + delta->size(); + return sizeof(Node) + delta().size(); } }; -#pragma pack(pop) -#pragma pack(push,1) struct { OffsetT nodeBytes; // Total size of all Nodes including the root uint8_t initialDepth; // Levels in the tree as of the last rebuild - Node root[0]; }; #pragma pack(pop) + inline Node & root() { + return *(Node *)(this + 1); + } + + inline const Node & root() const { + return *(const Node *)(this + 1); + } + int size() const { return sizeof(DeltaTree) + nodeBytes; } @@ -119,18 +131,18 @@ public: struct DecodedNode { DecodedNode(Node *raw, const T *prev, const T *next, Arena &arena) : raw(raw), parent(nullptr), left(nullptr), right(nullptr), prev(prev), next(next), - item(raw->delta->apply(raw->delta->getPrefixSource() ? *prev : *next, arena)) + item(raw->delta().apply(raw->delta().getPrefixSource() ? *prev : *next, arena)) { - //printf("DecodedNode1 raw=%p delta=%s\n", raw, raw->delta->toString().c_str()); + //printf("DecodedNode1 raw=%p delta=%s\n", raw, raw->delta().toString().c_str()); } DecodedNode(Node *raw, DecodedNode *parent, bool left, Arena &arena) : parent(parent), raw(raw), left(nullptr), right(nullptr), prev(left ? parent->prev : &parent->item), next(left ? &parent->item : parent->next), - item(raw->delta->apply(raw->delta->getPrefixSource() ? *prev : *next, arena)) + item(raw->delta().apply(raw->delta().getPrefixSource() ? *prev : *next, arena)) { - //printf("DecodedNode2 raw=%p delta=%s\n", raw, raw->delta->toString().c_str()); + //printf("DecodedNode2 raw=%p delta=%s\n", raw, raw->delta().toString().c_str()); } Node *raw; @@ -175,7 +187,7 @@ public: lower = new(arena) T(arena, *lower); upper = new(arena) T(arena, *upper); - root = (tree->nodeBytes == 0) ? nullptr : new (arena) DecodedNode(tree->root, lower, upper, arena); + root = (tree->nodeBytes == 0) ? nullptr : new (arena) DecodedNode(&tree->root(), lower, upper, arena); } const T *lowerBound() const { @@ -330,7 +342,7 @@ public: // The boundary leading to the new page acts as the last time we branched right if(begin != end) { - nodeBytes = build(*root, begin, end, prev, next); + nodeBytes = build(root(), begin, end, prev, next); } else { nodeBytes = 0; @@ -341,7 +353,7 @@ public: private: static OffsetT build(Node &root, const T *begin, const T *end, const T *prev, const T *next) { //printf("build: %s to %s\n", begin->toString().c_str(), (end - 1)->toString().c_str()); - //printf("build: root at %p sizeof(Node) %d delta at %p \n", &root, sizeof(Node), root.delta); + //printf("build: root at %p sizeof(Node) %d delta at %p \n", &root, sizeof(Node), &root.delta()); ASSERT(end != begin); int count = end - begin; @@ -370,12 +382,12 @@ private: base = next; } - int deltaSize = item.writeDelta(*root.delta, *base, commonPrefix); - root.delta->setPrefixSource(prefixSourcePrev); - //printf("Serialized %s to %p\n", item.toString().c_str(), root.delta); + int deltaSize = item.writeDelta(root.delta(), *base, commonPrefix); + root.delta().setPrefixSource(prefixSourcePrev); + //printf("Serialized %s to %p\n", item.toString().c_str(), &root.delta()); // Continue writing after the serialized Delta. - uint8_t *wptr = (uint8_t *)root.delta + deltaSize; + uint8_t *wptr = (uint8_t *)&root.delta() + deltaSize; // Serialize left child if(count > 1) { @@ -388,7 +400,7 @@ private: // Serialize right child if(count > 2) { - root.rightChildOffset = wptr - (uint8_t *)root.delta; + root.rightChildOffset = wptr - (uint8_t *)&root.delta(); wptr += build(*(Node *)wptr, begin + mid + 1, end, &item, next); } else { diff --git a/fdbserver/VersionedBTree.actor.cpp b/fdbserver/VersionedBTree.actor.cpp index 5834687548..a926926d5a 100644 --- a/fdbserver/VersionedBTree.actor.cpp +++ b/fdbserver/VersionedBTree.actor.cpp @@ -431,7 +431,14 @@ struct RedwoodRecordRef { }; uint8_t flags; - byte data[]; + + inline byte * data() { + return (byte *)(this + 1); + } + + inline const byte * data() const { + return (const byte *)(this + 1); + } void setPrefixSource(bool val) { if(val) { @@ -447,7 +454,7 @@ struct RedwoodRecordRef { } RedwoodRecordRef apply(const RedwoodRecordRef &base, Arena &arena) const { - Reader r(data); + Reader r(data()); int intFieldSuffixLen = flags & INT_FIELD_SUFFIX_BITS; int prefixLen = r.readVarInt(); @@ -501,19 +508,19 @@ struct RedwoodRecordRef { } int size() const { - Reader r(data); + Reader r(data()); int intFieldSuffixLen = flags & INT_FIELD_SUFFIX_BITS; r.readVarInt(); // prefixlen int valueLen = (flags & HAS_VALUE) ? r.read() : 0; int keySuffixLen = (flags & HAS_KEY_SUFFIX) ? r.readVarInt() : 0; - return sizeof(Delta) + r.rptr - data + intFieldSuffixLen + valueLen + keySuffixLen; + return sizeof(Delta) + r.rptr - data() + intFieldSuffixLen + valueLen + keySuffixLen; } // Delta can't be determined without the RedwoodRecordRef upon which the Delta is based. std::string toString() const { - Reader r(data); + Reader r(data()); std::string flagString = " "; if(flags & PREFIX_SOURCE) flagString += "prefixSource "; @@ -638,7 +645,7 @@ struct RedwoodRecordRef { commonPrefix = getCommonPrefixLen(base, 0); } - Writer w(d.data); + Writer w(d.data()); // prefixLen w.writeVarInt(commonPrefix); @@ -688,7 +695,7 @@ struct RedwoodRecordRef { w.writeString(value.get()); } - return w.wptr - d.data + sizeof(Delta); + return w.wptr - d.data() + sizeof(Delta); } template @@ -737,10 +744,17 @@ struct BTreePage { uint16_t count; uint32_t kvBytes; uint8_t extensionPageCount; - LogicalPageID extensionPages[0]; }; #pragma pack(pop) + inline LogicalPageID * extensionPages() { + return (LogicalPageID *)(this + 1); + } + + inline const LogicalPageID * extensionPages() const { + return (const LogicalPageID *)(this + 1); + } + int size() const { const BinaryTree *t = &tree(); return (uint8_t *)t - (uint8_t *)this + t->size(); @@ -751,15 +765,15 @@ struct BTreePage { } BinaryTree & tree() { - return *(BinaryTree *)(extensionPages + extensionPageCount); + return *(BinaryTree *)(extensionPages() + extensionPageCount); } const BinaryTree & tree() const { - return *(const BinaryTree *)(extensionPages + extensionPageCount); + return *(const BinaryTree *)(extensionPages() + extensionPageCount); } static inline int GetHeaderSize(int extensionPages = 0) { - return sizeof(BTreePage) + extensionPages + sizeof(LogicalPageID); + return sizeof(BTreePage) + (extensionPages * sizeof(LogicalPageID)); } std::string toString(bool write, LogicalPageID id, Version ver, const RedwoodRecordRef *lowerBound, const RedwoodRecordRef *upperBound) const { @@ -1603,7 +1617,7 @@ private: for(int e = 0, eEnd = extPages.size(); e < eEnd; ++e) { LogicalPageID eid = m_pager->allocateLogicalPage(); debug_printf("%p: writePages(): Writing extension page op=write id=%u @%" PRId64 " (%d of %lu) referencePageID=%u\n", actor_debug, eid, version, e + 1, extPages.size(), id); - newPage->extensionPages[e] = bigEndian32(eid); + newPage->extensionPages()[e] = bigEndian32(eid); // If replacing the primary page below (version == 0) then pass the primary page's ID as the reference page ID m_pager->writePage(eid, extPages[e], version, (version == 0) ? id : invalidLogicalPageID); ++counts.extPageWrites; @@ -1620,8 +1634,8 @@ private: // Free the old extension pages now that all replacement pages have been written for(int i = 0; i < originalPage->extensionPageCount; ++i) { - //debug_printf("%p: writePages(): Freeing old extension op=del id=%u @latest\n", actor_debug, bigEndian32(originalPage->extensionPages[i])); - //m_pager->freeLogicalPage(bigEndian32(originalPage->extensionPages[i]), version); + //debug_printf("%p: writePages(): Freeing old extension op=del id=%u @latest\n", actor_debug, bigEndian32(originalPage->extensionPages()[i])); + //m_pager->freeLogicalPage(bigEndian32(originalPage->extensionPages()[i]), version); } return primaryLogicalPageIDs; @@ -1684,8 +1698,8 @@ private: pageGets.push_back(std::move(result)); for(int i = 0; i < pTreePage->extensionPageCount; ++i) { - debug_printf("readPage() Reading extension page op=read id=%u @%" PRId64 " ext=%d/%d\n", bigEndian32(pTreePage->extensionPages[i]), snapshot->getVersion(), i + 1, (int)pTreePage->extensionPageCount); - pageGets.push_back(snapshot->getPhysicalPage(bigEndian32(pTreePage->extensionPages[i]))); + debug_printf("readPage() Reading extension page op=read id=%u @%" PRId64 " ext=%d/%d\n", bigEndian32(pTreePage->extensionPages()[i]), snapshot->getVersion(), i + 1, (int)pTreePage->extensionPageCount); + pageGets.push_back(snapshot->getPhysicalPage(bigEndian32(pTreePage->extensionPages()[i]))); } std::vector> pages = wait(getAll(pageGets)); @@ -3561,12 +3575,12 @@ TEST_CASE("!/redwood/correctness/unit/deltaTree/RedwoodRecordRef") { while(1) { if(fwd.get() != items[i]) { printf("forward iterator i=%d\n %s found\n %s expected\n", i, fwd.get().toString().c_str(), items[i].toString().c_str()); - printf("Delta: %s\n", fwd.node->raw->delta->toString().c_str()); + printf("Delta: %s\n", fwd.node->raw->delta().toString().c_str()); ASSERT(false); } if(rev.get() != items[items.size() - 1 - i]) { printf("reverse iterator i=%d\n %s found\n %s expected\n", i, rev.get().toString().c_str(), items[items.size() - 1 - i].toString().c_str()); - printf("Delta: %s\n", rev.node->raw->delta->toString().c_str()); + printf("Delta: %s\n", rev.node->raw->delta().toString().c_str()); ASSERT(false); } ++i;