diff --git a/src/mongo/db/exec/collection_scan.cpp b/src/mongo/db/exec/collection_scan.cpp index a1f416f97fd..866683e3418 100644 --- a/src/mongo/db/exec/collection_scan.cpp +++ b/src/mongo/db/exec/collection_scan.cpp @@ -210,7 +210,7 @@ PlanStage::StageState CollectionScan::doWork(WorkingSetID* out) { WorkingSetID id = _workingSet->allocate(); WorkingSetMember* member = _workingSet->get(id); - member->recordId = record->id; + member->recordId = std::move(record->id); member->resetDocument(opCtx()->recoveryUnit()->getSnapshotId(), record->data.releaseToBson()); _workingSet->transitionToRecordIdAndObj(id); diff --git a/src/mongo/db/record_id.h b/src/mongo/db/record_id.h index 6788ca62dcd..65a42a8649a 100644 --- a/src/mongo/db/record_id.h +++ b/src/mongo/db/record_id.h @@ -119,14 +119,17 @@ public: */ template auto withFormat(OnNull&& onNull, OnLong&& onLong, OnStr&& onStr) const { - switch (auto f = _format) { + switch (_format) { case Format::kNull: return onNull(Null()); case Format::kLong: - return onLong(getLong()); - case Format::kSmallStr: + return onLong(_getLongNoCheck()); + case Format::kSmallStr: { + auto str = _getSmallStrNoCheck(); + return onStr(str.rawData(), str.size()); + } case Format::kBigStr: { - auto str = getStr(); + auto str = _getBigStrNoCheck(); return onStr(str.rawData(), str.size()); } default: @@ -155,9 +158,7 @@ public: } invariant(isLong(), fmt::format("expected RecordID long format, got: {}", _formatToString(_format))); - int64_t val; - memcpy(&val, _buffer, sizeof(val)); - return val; + return _getLongNoCheck(); } /** @@ -169,17 +170,9 @@ public: isStr(), fmt::format("expected RecordID string format, got: {}", _formatToString(_format))); if (_format == Format::kSmallStr) { - char size = _buffer[0]; - invariant(size > 0); - invariant(size <= kSmallStrMaxSize); - return StringData(_buffer + 1, size); + return _getSmallStrNoCheck(); } else if (_format == Format::kBigStr) { - // We use a ConstSharedBuffer that is only allocated once and assume the string size is - // just the originally allocated capacity. - size_t size = _sharedBuffer.capacity(); - invariant(size > kSmallStrMaxSize); - invariant(size <= kBigStrMaxSize); - return StringData(_sharedBuffer.get(), size); + return _getBigStrNoCheck(); } MONGO_UNREACHABLE; } @@ -224,13 +217,19 @@ public: if (rhs._format == Format::kNull) { return 1; } - return getLong() == rhs.getLong() ? 0 : (getLong() > rhs.getLong()) ? 1 : -1; + return _getLongNoCheck() == rhs.getLong() + ? 0 + : (_getLongNoCheck() > rhs.getLong()) ? 1 : -1; case Format::kSmallStr: + if (rhs._format == Format::kNull) { + return 1; + } + return _getSmallStrNoCheck().compare(rhs.getStr()); case Format::kBigStr: if (rhs._format == Format::kNull) { return 1; } - return getStr().compare(rhs.getStr()); + return _getBigStrNoCheck().compare(rhs.getStr()); } MONGO_UNREACHABLE; } @@ -351,6 +350,28 @@ private: MONGO_UNREACHABLE; } + int64_t _getLongNoCheck() const { + int64_t val; + memcpy(&val, _buffer, sizeof(val)); + return val; + } + + StringData _getSmallStrNoCheck() const { + char size = _buffer[0]; + invariant(size > 0); + invariant(size <= kSmallStrMaxSize); + return StringData(_buffer + 1, size); + } + + StringData _getBigStrNoCheck() const { + // We use a ConstSharedBuffer that is only allocated once and assume the string size is + // just the originally allocated capacity. + size_t size = _sharedBuffer.capacity(); + invariant(size > kSmallStrMaxSize); + invariant(size <= kBigStrMaxSize); + return StringData(_sharedBuffer.get(), size); + } + Format _format = Format::kNull; // An extra byte of space is required to store the size for the // kSmallStr Format. Zero the buffer so we don't need to write diff --git a/src/mongo/db/storage/record_id_bm.cpp b/src/mongo/db/storage/record_id_bm.cpp index 63ca93486c4..8e830aa8999 100644 --- a/src/mongo/db/storage/record_id_bm.cpp +++ b/src/mongo/db/storage/record_id_bm.cpp @@ -37,12 +37,39 @@ namespace mongo { namespace { +void BM_RecordIdCompareLong(benchmark::State& state) { + RecordId rid(1 << 31); + RecordId tmp(1); + for (auto _ : state) { + benchmark::DoNotOptimize(rid > tmp); + benchmark::ClobberMemory(); + } +} + +void BM_RecordIdCompareSmallStr(benchmark::State& state) { + std::string str1(20, 'x'); + RecordId rid1(str1.c_str(), str1.size()); + RecordId rid2(str1.c_str(), str1.size()); + for (auto _ : state) { + benchmark::DoNotOptimize(rid1 > rid2); + benchmark::ClobberMemory(); + } +} + +void BM_RecordIdIsValidLong(benchmark::State& state) { + RecordId rid(1 << 31); + for (auto _ : state) { + benchmark::DoNotOptimize(rid.isValid()); + benchmark::ClobberMemory(); + } +} + void BM_RecordIdCopyLong(benchmark::State& state) { RecordId rid(1 << 31); for (auto _ : state) { RecordId tmp; - benchmark::ClobberMemory(); benchmark::DoNotOptimize(tmp = rid); + benchmark::ClobberMemory(); } } @@ -50,8 +77,8 @@ void BM_RecordIdCopyOID(benchmark::State& state) { RecordId rid = record_id_helpers::keyForOID(OID::gen()); for (auto _ : state) { RecordId tmp; - benchmark::ClobberMemory(); benchmark::DoNotOptimize(tmp = rid); + benchmark::ClobberMemory(); } } @@ -63,8 +90,8 @@ void BM_RecordIdCopyMedString(benchmark::State& state) { RecordId rid = RecordId(buf, bufLen); for (auto _ : state) { RecordId tmp; - benchmark::ClobberMemory(); benchmark::DoNotOptimize(tmp = rid); + benchmark::ClobberMemory(); } } @@ -76,28 +103,28 @@ void BM_RecordIdCopyBigString(benchmark::State& state) { RecordId rid = RecordId(buf, bufLen); for (auto _ : state) { RecordId tmp; - benchmark::ClobberMemory(); benchmark::DoNotOptimize(tmp = rid); + benchmark::ClobberMemory(); } } void BM_RecordIdFormatLong(benchmark::State& state) { RecordId rid(1 << 31); for (auto _ : state) { - benchmark::ClobberMemory(); benchmark::DoNotOptimize(rid.withFormat([](RecordId::Null) { return false; }, [](std::int64_t val) { return false; }, [](const char* str, int size) { return false; })); + benchmark::ClobberMemory(); } } void BM_RecordIdFormatString(benchmark::State& state) { RecordId rid = record_id_helpers::keyForOID(OID::gen()); for (auto _ : state) { - benchmark::ClobberMemory(); benchmark::DoNotOptimize(rid.withFormat([](RecordId::Null) { return false; }, [](std::int64_t val) { return false; }, [](const char* str, int size) { return false; })); + benchmark::ClobberMemory(); } } @@ -106,6 +133,10 @@ BENCHMARK(BM_RecordIdCopyOID); BENCHMARK(BM_RecordIdCopyMedString); BENCHMARK(BM_RecordIdCopyBigString); +BENCHMARK(BM_RecordIdCompareLong); +BENCHMARK(BM_RecordIdCompareSmallStr); +BENCHMARK(BM_RecordIdIsValidLong); + BENCHMARK(BM_RecordIdFormatLong); BENCHMARK(BM_RecordIdFormatString); diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp index e40cba963ed..14cfea1e6e3 100644 --- a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp +++ b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp @@ -694,7 +694,8 @@ public: auto& metricsCollector = ResourceConsumption::MetricsCollector::get(_opCtx); metricsCollector.incrementOneDocRead(value.size); - return {{id, {static_cast(value.data), static_cast(value.size)}}}; + return { + {std::move(id), {static_cast(value.data), static_cast(value.size)}}}; } void save() final { @@ -2062,7 +2063,7 @@ boost::optional WiredTigerRecordStoreCursorBase::next() { metricsCollector.incrementOneDocRead(value.size); _lastReturnedId = id; - return {{id, {static_cast(value.data), static_cast(value.size)}}}; + return {{std::move(id), {static_cast(value.data), static_cast(value.size)}}}; } boost::optional WiredTigerRecordStoreCursorBase::seekExact(const RecordId& id) { @@ -2218,13 +2219,11 @@ bool WiredTigerRecordStoreCursorBase::restore() { int cmp; int ret = wiredTigerPrepareConflictRetry(_opCtx, [&] { return c->search_near(c, &cmp); }); - RecordId id; if (ret == WT_NOTFOUND) { _eof = true; return !_rs._isCapped; } invariantWTOK(ret); - id = getKey(c); if (cmp == 0) return true; // Landed right where we left off.