diff options
author | Andrew Morrow <acm@10gen.com> | 2014-02-05 16:08:31 -0500 |
---|---|---|
committer | Andrew Morrow <acm@mongodb.com> | 2014-04-06 21:17:27 -0400 |
commit | d35fef6dddf7bf0c7b0837ea802bb6e9ab679a83 (patch) | |
tree | 207f5ac5fa1f992d5e0077bca7057f1b675faef6 /src | |
parent | 1d50eac6f00016a3366827df2e851531f8c56d3b (diff) | |
download | mongo-d35fef6dddf7bf0c7b0837ea802bb6e9ab679a83.tar.gz |
SERVER-12578 Permit in-place mutation for all types when possible
Diffstat (limited to 'src')
-rw-r--r-- | src/mongo/bson/mutable/document.cpp | 114 | ||||
-rw-r--r-- | src/mongo/db/ops/modifier_compare_test.cpp | 1 |
2 files changed, 57 insertions, 58 deletions
diff --git a/src/mongo/bson/mutable/document.cpp b/src/mongo/bson/mutable/document.cpp index 5080abf01c4..fb5ee9f7cd1 100644 --- a/src/mongo/bson/mutable/document.cpp +++ b/src/mongo/bson/mutable/document.cpp @@ -1002,23 +1002,37 @@ namespace mutablebson { } } - // Not all types are currently permitted to be updated in-place. - bool canUpdateInPlace(const ElementRep& rep) { - const BSONType type = getType(rep); - switch(type) { - case mongo::NumberDouble: - case mongo::String: - case mongo::BinData: - case mongo::jstOID: - case mongo::Bool: - case mongo::Date: - case mongo::NumberInt: - case mongo::Timestamp: - case mongo::NumberLong: - return true; - default: + // Check all preconditions on doing an in-place update, except for size match. + bool canUpdateInPlace(const ElementRep& sourceRep, const ElementRep& targetRep) { + + // NOTE: CodeWScope might arguably be excluded since it has substructure, but + // mutable doesn't permit navigation into its document, so we can handle it. + + // We can only do an in-place update to an element that is serialized and is not in + // the leaf heap. + // + // TODO: In the future, we can replace values in the leaf heap if they are of the + // same size as the origin was. For now, we don't support that. + if (!hasValue(targetRep) || (targetRep.objIdx == kLeafObjIdx)) return false; + + // sourceRep should be newly created, so it must have a value representation. + dassert(hasValue(sourceRep)); + + // For a target that has substructure, we only permit in-place updates if there + // cannot be ElementReps that reference data within the target. We don't need to + // worry about ElementReps for source, since it is newly created. The only way + // there can be ElementReps referring into substructure is if the Element has + // non-empty non-opaque child references. + if (!isLeaf(targetRep)) { + if (((targetRep.child.left != Element::kOpaqueRepIdx) && + (targetRep.child.left != Element::kInvalidRepIdx)) || + ((targetRep.child.right != Element::kOpaqueRepIdx) && + (targetRep.child.right != Element::kInvalidRepIdx))) + return false; } + + return true; } template<typename Builder> @@ -1983,61 +1997,47 @@ namespace mutablebson { ElementRep& thisRep = impl.getElementRep(_repIdx); ElementRep& valueRep = impl.getElementRep(newValueIdx); - bool inPlace = false; - if (impl.canUpdateInPlace(valueRep) && impl.isInPlaceModeEnabled()) { + if (impl.isInPlaceModeEnabled() && impl.canUpdateInPlace(valueRep, thisRep)) { - // In place updates are currently enabled. We can do an in-place update to an - // element that is serialized and is not in the leaf heap. - const bool inLeafHeap = (thisRep.objIdx == kLeafObjIdx); - const bool hasValue = impl.hasValue(thisRep); + // Get the BSONElement representations of the existing and new value, so we can + // check if they are size compatible. + BSONElement thisElt = impl.getSerializedElement(thisRep); + BSONElement valueElt = impl.getSerializedElement(valueRep); - // TODO: In the future, we can replace values in the leaf heap if they are of the - // same size as the origin was. For now, we don't support that. - if (hasValue && !inLeafHeap) { - - // See if the new Element can be recorded as an in-place update. - dassert(impl.hasValue(valueRep)); - - // Get the BSONElement representations of the existing and new value, so we can - // check if they are size compatible. - BSONElement thisElt = impl.getSerializedElement(thisRep); - BSONElement valueElt = impl.getSerializedElement(valueRep); + if (thisElt.size() == valueElt.size()) { - if (thisElt.size() == valueElt.size()) { + // The old and new elements are size compatible. Compute the base offsets + // of each BSONElement in the object in which it resides. We use these to + // calculate the source and target offsets in the damage entries we are + // going to write. - // The old and new elements are size compatible. Compute the base offsets - // of each BSONElement in the object in which it resides. We use these to - // calculate the source and target offsets in the damage entries we are - // going to write. + const DamageEvent::OffsetSizeType targetBaseOffset = + getElementOffset(impl.getObject(thisRep.objIdx), thisElt); - const DamageEvent::OffsetSizeType targetBaseOffset = - getElementOffset(impl.getObject(thisRep.objIdx), thisElt); + const DamageEvent::OffsetSizeType sourceBaseOffset = + getElementOffset(impl.getObject(valueRep.objIdx), valueElt); - const DamageEvent::OffsetSizeType sourceBaseOffset = - getElementOffset(impl.getObject(valueRep.objIdx), valueElt); + // If this is a type change, record a damage event for the new type. + if (thisElt.type() != valueElt.type()) { + impl.recordDamageEvent(targetBaseOffset, sourceBaseOffset, 1); + } - // If this is a type change, record a damage event for the new type. - if (thisElt.type() != valueElt.type()) { - impl.recordDamageEvent(targetBaseOffset, sourceBaseOffset, 1); - } + dassert(thisElt.fieldNameSize() == valueElt.fieldNameSize()); + dassert(thisElt.valuesize() == valueElt.valuesize()); - dassert(thisElt.fieldNameSize() == valueElt.fieldNameSize()); - dassert(thisElt.valuesize() == valueElt.valuesize()); + // Record a damage event for the new value data. + impl.recordDamageEvent( + targetBaseOffset + thisElt.fieldNameSize() + 1, + sourceBaseOffset + thisElt.fieldNameSize() + 1, + thisElt.valuesize()); + } else { - // Record a damage event for the new value data. - impl.recordDamageEvent( - targetBaseOffset + thisElt.fieldNameSize() + 1, - sourceBaseOffset + thisElt.fieldNameSize() + 1, - thisElt.valuesize()); + // We couldn't do it in place, so disable future in-place updates. + impl.disableInPlaceUpdates(); - inPlace = true; - } } } - if (!inPlace) - getDocument().disableInPlaceUpdates(); - // If we are not rootish, then wire in the new value among our relations. if (thisRep.parent != kInvalidRepIdx) { valueRep.parent = thisRep.parent; diff --git a/src/mongo/db/ops/modifier_compare_test.cpp b/src/mongo/db/ops/modifier_compare_test.cpp index eba04776b1f..ba5c9b6d7b8 100644 --- a/src/mongo/db/ops/modifier_compare_test.cpp +++ b/src/mongo/db/ops/modifier_compare_test.cpp @@ -275,7 +275,6 @@ namespace { ASSERT_OK(mod.apply()); ASSERT_EQUALS(fromjson("{a: {b: 3}}}"), doc); - ASSERT_FALSE(doc.isInPlaceModeEnabled()); Document logDoc; LogBuilder logBuilder(logDoc.root()); |