From d3cf455cba32ef3f5280634470858e80761d8450 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 15 Jan 2020 15:17:07 +0000 Subject: Unify towards nbostream --- storage/src/tests/visiting/visitormanagertest.cpp | 2 +- .../src/vespa/storage/persistence/mergehandler.cpp | 24 ++++++++-------------- .../src/vespa/storage/visiting/recoveryvisitor.cpp | 4 ++-- 3 files changed, 11 insertions(+), 19 deletions(-) (limited to 'storage') diff --git a/storage/src/tests/visiting/visitormanagertest.cpp b/storage/src/tests/visiting/visitormanagertest.cpp index b7eb7fee3ec..3f111bb53f7 100644 --- a/storage/src/tests/visiting/visitormanagertest.cpp +++ b/storage/src/tests/visiting/visitormanagertest.cpp @@ -337,7 +337,7 @@ int getTotalSerializedSize(const std::vector& docs) { int total = 0; for (size_t i = 0; i < docs.size(); ++i) { - total += int(docs[i]->serialize()->getLength()); + total += int(docs[i]->getSerializedSize()); } return total; } diff --git a/storage/src/vespa/storage/persistence/mergehandler.cpp b/storage/src/vespa/storage/persistence/mergehandler.cpp index 37e1d818bb8..1ef26a969ed 100644 --- a/storage/src/vespa/storage/persistence/mergehandler.cpp +++ b/storage/src/vespa/storage/persistence/mergehandler.cpp @@ -506,18 +506,11 @@ MergeHandler::fetchLocalData( assert(doc != 0); assertContainedInBucket(doc->getId(), bucket, idFactory); e._docName = doc->getId().toString(); - { - vespalib::nbostream stream; - doc->serializeHeader(stream); - e._headerBlob.resize(stream.size()); - memcpy(&e._headerBlob[0], stream.peek(), stream.size()); - } - { - vespalib::nbostream stream; - doc->serializeBody(stream); - e._bodyBlob.resize(stream.size()); - memcpy(&e._bodyBlob[0], stream.peek(), stream.size()); - } + vespalib::nbostream stream; + doc->serialize(stream); + e._headerBlob.resize(stream.size()); + memcpy(&e._headerBlob[0], stream.peek(), stream.size()); + e._bodyBlob.clear(); } else { const DocumentId* docId = docEntry.getDocumentId(); assert(docId != 0); @@ -556,11 +549,10 @@ MergeHandler::deserializeDiffDocument( const api::ApplyBucketDiffCommand::Entry& e, const document::DocumentTypeRepo& repo) const { - Document::UP doc(new Document); - using document::ByteBuffer; - ByteBuffer hbuf(&e._headerBlob[0], e._headerBlob.size()); + auto doc = std::make_unique(); + vespalib::nbostream hbuf(&e._headerBlob[0], e._headerBlob.size()); if (e._bodyBlob.size() > 0) { - ByteBuffer bbuf(&e._bodyBlob[0], e._bodyBlob.size()); + vespalib::nbostream bbuf(&e._bodyBlob[0], e._bodyBlob.size()); doc->deserialize(repo, hbuf, bbuf); } else { doc->deserialize(repo, hbuf); diff --git a/storage/src/vespa/storage/visiting/recoveryvisitor.cpp b/storage/src/vespa/storage/visiting/recoveryvisitor.cpp index 7c69a232af0..f9e0fa17d66 100644 --- a/storage/src/vespa/storage/visiting/recoveryvisitor.cpp +++ b/storage/src/vespa/storage/visiting/recoveryvisitor.cpp @@ -39,7 +39,7 @@ RecoveryVisitor::handleDocuments(const document::BucketId& bid, LOG(debug, "Visitor %s handling block of %zu documents.", _id.c_str(), entries.size()); - documentapi::DocumentListMessage* cmd = NULL; + documentapi::DocumentListMessage* cmd = nullptr; { CommandMap::iterator iter = _activeCommands.find(bid); @@ -71,7 +71,7 @@ RecoveryVisitor::handleDocuments(const document::BucketId& bid, } } - hitCounter.addHit(doc->getId(), doc->serialize()->getLength()); + hitCounter.addHit(doc->getId(), doc->getSerializedSize()); int64_t timestamp = doc->getLastModified(); cmd->getDocuments().push_back(documentapi::DocumentListMessage::Entry( -- cgit v1.2.3 From b1bc742e8ac18dd41ea8f19919005257fbfa277b Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 15 Jan 2020 23:14:32 +0000 Subject: Remove complicated option for slicing as it is not used anywhere. --- document/src/tests/testbytebuffer.cpp | 72 ------- .../document/fieldvalue/serializablearray.cpp | 48 ++--- .../vespa/document/fieldvalue/serializablearray.h | 17 +- .../vespa/document/fieldvalue/structfieldvalue.cpp | 6 +- document/src/vespa/document/util/bytebuffer.cpp | 236 +-------------------- document/src/vespa/document/util/bytebuffer.h | 61 +----- storage/src/vespa/storage/common/storagelink.cpp | 1 + .../maintenance/simplemaintenancescanner.cpp | 7 +- .../distributor/throttlingoperationstarter.cpp | 4 +- vespalib/src/vespa/vespalib/util/alloc.h | 4 +- 10 files changed, 50 insertions(+), 406 deletions(-) (limited to 'storage') diff --git a/document/src/tests/testbytebuffer.cpp b/document/src/tests/testbytebuffer.cpp index 17807fb4ff5..feb3252a48c 100644 --- a/document/src/tests/testbytebuffer.cpp +++ b/document/src/tests/testbytebuffer.cpp @@ -134,78 +134,6 @@ TEST(ByteBuffer_Test, test_copy_constructor) } } -TEST(ByteBuffer_Test, test_slice) -{ - ByteBuffer* newBuf=ByteBuffer::copyBuffer("hei der",8); - - ByteBuffer* slice = new ByteBuffer; - slice->sliceFrom(*newBuf, 0,newBuf->getLength()); - delete newBuf; - newBuf = NULL; - - EXPECT_TRUE(strcmp(slice->getBufferAtPos(),"hei der")==0); - - ByteBuffer* slice2 = new ByteBuffer; - slice2->sliceFrom(*slice, 4, slice->getLength()); - delete slice; - slice = NULL; - - EXPECT_TRUE(strcmp(slice2->getBufferAtPos(),"der")==0); - EXPECT_TRUE(strcmp(slice2->getBuffer(),"hei der")==0); - delete slice2; - slice2 = NULL; - - ByteBuffer* newBuf2=new ByteBuffer("hei der", 8); - ByteBuffer* slice3=new ByteBuffer; - ByteBuffer* slice4=new ByteBuffer; - - slice3->sliceFrom(*newBuf2, 4, newBuf2->getLength()); - slice4->sliceFrom(*newBuf2, 0, newBuf2->getLength()); - delete newBuf2; - newBuf2 = NULL; - - EXPECT_TRUE(strcmp(slice3->getBufferAtPos(),"der")==0); - EXPECT_TRUE(strcmp(slice4->getBuffer(),"hei der")==0); - - delete slice3; - slice3 = NULL; - - EXPECT_TRUE(strcmp(slice4->getBuffer(),"hei der")==0); - - delete slice4; - slice4 = NULL; -} - -TEST(ByteBuffer_Test, test_slice2) -{ - ByteBuffer* newBuf=ByteBuffer::copyBuffer("hei der",8); - - ByteBuffer slice; - slice.sliceFrom(*newBuf, 0, newBuf->getLength()); - - delete newBuf; - newBuf = NULL; - - EXPECT_TRUE(strcmp(slice.getBufferAtPos(),"hei der")==0); - - ByteBuffer slice2; - slice2.sliceFrom(slice, 4, slice.getLength()); - - EXPECT_TRUE(strcmp(slice2.getBufferAtPos(),"der")==0); - EXPECT_TRUE(strcmp(slice2.getBuffer(),"hei der")==0); - - ByteBuffer* newBuf2=new ByteBuffer("hei der", 8); - - slice.sliceFrom(*newBuf2, 4, newBuf2->getLength()); - slice2.sliceFrom(*newBuf2, 0, newBuf2->getLength()); - delete newBuf2; - newBuf2 = NULL; - - EXPECT_TRUE(strcmp(slice.getBufferAtPos(),"der")==0); - EXPECT_TRUE(strcmp(slice2.getBuffer(),"hei der")==0); -} - - TEST(ByteBuffer_Test, test_putGetFlip) { ByteBuffer* newBuf=new ByteBuffer(100); diff --git a/document/src/vespa/document/fieldvalue/serializablearray.cpp b/document/src/vespa/document/fieldvalue/serializablearray.cpp index 5dfd8eff891..ff0fe3dbdf0 100644 --- a/document/src/vespa/document/fieldvalue/serializablearray.cpp +++ b/document/src/vespa/document/fieldvalue/serializablearray.cpp @@ -34,6 +34,22 @@ SerializableArray::SerializableArray() { } +SerializableArray::SerializableArray(EntryMap entries, ByteBuffer::UP buffer, + CompressionConfig::Type comp_type,uint32_t uncompressed_length) + : _entries(std::move(entries)), + _owned(), + _serializedCompression(comp_type) +{ + + if (CompressionConfig::isCompressed(_serializedCompression)) { + _compSerData = std::move(buffer); + _uncompressedLength = uncompressed_length; + } else { + _uncompressedLength = buffer->getRemaining(); + _uncompSerData = std::move(buffer); + } +} + serializablearray::BufferMap & ensure(std::unique_ptr & owned) { if (!owned) { owned = std::make_unique(); @@ -45,8 +61,8 @@ SerializableArray::SerializableArray(const SerializableArray& other) : Cloneable(), _entries(other._entries), _owned(), - _uncompSerData(other._uncompSerData.get() ? new ByteBuffer(*other._uncompSerData) : NULL), - _compSerData(other._compSerData.get() ? new ByteBuffer(*other._compSerData) : NULL), + _uncompSerData(other._uncompSerData.get() ? new ByteBuffer(*other._uncompSerData) : nullptr), + _compSerData(other._compSerData.get() ? new ByteBuffer(*other._compSerData) : nullptr), _serializedCompression(other._serializedCompression), _uncompressedLength(other._uncompressedLength) { @@ -86,9 +102,7 @@ void SerializableArray::clear() _uncompressedLength = 0; } -SerializableArray::~SerializableArray() -{ -} +SerializableArray::~SerializableArray() = default; void SerializableArray::invalidate() @@ -102,7 +116,7 @@ SerializableArray::set(int id, ByteBuffer::UP buffer) maybeDecompress(); Entry e(id, buffer->getRemaining(), buffer->getBuffer()); ensure(_owned)[id] = std::move(buffer); - EntryMap::iterator it = find(id); + auto it = find(id); if (it == _entries.end()) { _entries.push_back(e); } else { @@ -139,7 +153,7 @@ SerializableArray::get(int id) const { vespalib::ConstBufferRef buf; if ( !maybeDecompressAndCatch() ) { - EntryMap::const_iterator found = find(id); + auto found = find(id); if (found != _entries.end()) { const Entry& entry = *found; @@ -168,7 +182,7 @@ void SerializableArray::clear(int id) { maybeDecompress(); - EntryMap::iterator it = find(id); + auto it = find(id); if (it != _entries.end()) { _entries.erase(it); if (_owned) { @@ -221,24 +235,6 @@ SerializableArray::deCompress() // throw (DeserializeException) } } -void SerializableArray::assign(EntryMap & entries, - ByteBuffer::UP buffer, - CompressionConfig::Type comp_type, - uint32_t uncompressed_length) -{ - _serializedCompression = comp_type; - - _entries.clear(); - _entries.swap(entries); - if (CompressionConfig::isCompressed(_serializedCompression)) { - _compSerData.reset(buffer.release()); - _uncompressedLength = uncompressed_length; - } else { - _uncompressedLength = buffer->getRemaining(); - _uncompSerData.reset(buffer.release()); - } -} - vespalib::compression::CompressionInfo SerializableArray::getCompressionInfo() const { return CompressionInfo(_uncompressedLength, _compSerData->getRemaining()); diff --git a/document/src/vespa/document/fieldvalue/serializablearray.h b/document/src/vespa/document/fieldvalue/serializablearray.h index 1e2bf5706bf..7ee09f592b2 100644 --- a/document/src/vespa/document/fieldvalue/serializablearray.h +++ b/document/src/vespa/document/fieldvalue/serializablearray.h @@ -110,7 +110,9 @@ public: using CompressionInfo = vespalib::compression::CompressionInfo; SerializableArray(); - virtual ~SerializableArray(); + SerializableArray(EntryMap entries, ByteBufferUP buffer, + CompressionConfig::Type comp_type, uint32_t uncompressed_length); + ~SerializableArray() override; void swap(SerializableArray& other); @@ -140,9 +142,6 @@ public: /** @return Returns true if the given ID is Set in the array. */ bool has(int id) const; - /** @return Number of elements in array */ - bool hasAnyElems() const { return !_entries.empty(); } - /** * clears an attribute. * @@ -156,16 +155,6 @@ public: CompressionConfig::Type getCompression() const { return _serializedCompression; } CompressionInfo getCompressionInfo() const; - /** - * Sets the serialized data that is the basis for this object's - * content. This is used by deserialization. Any existing entries - * are cleared. - */ - void assign(EntryMap &entries, - ByteBufferUP buffer, - CompressionConfig::Type comp_type, - uint32_t uncompressed_length); - bool empty() const { return _entries.empty(); } const ByteBuffer* getSerializedBuffer() const { diff --git a/document/src/vespa/document/fieldvalue/structfieldvalue.cpp b/document/src/vespa/document/fieldvalue/structfieldvalue.cpp index a011f9d8949..1d74e99208a 100644 --- a/document/src/vespa/document/fieldvalue/structfieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/structfieldvalue.cpp @@ -81,13 +81,11 @@ StructFieldValue::lazyDeserialize(const FixedTypeRepo &repo, _doc_type = &repo.getDocumentType(); _version = version; - _chunks.push_back(std::make_unique()); - _chunks.back().assign(fm, std::move(buffer), comp_type, uncompressed_length); + _chunks.push_back(std::make_unique(std::move(fm), std::move(buffer), comp_type, uncompressed_length)); _hasChanged = false; } -bool StructFieldValue::serializeField(int field_id, uint16_t version, - FieldValueWriter &writer) const { +bool StructFieldValue::serializeField(int field_id, uint16_t version, FieldValueWriter &writer) const { if (version == _version) { for (int i = _chunks.size() - 1; i >= 0; --i) { vespalib::ConstBufferRef buf = _chunks[i].get(field_id); diff --git a/document/src/vespa/document/util/bytebuffer.cpp b/document/src/vespa/document/util/bytebuffer.cpp index ccbc2bc7790..1b586a448cf 100644 --- a/document/src/vespa/document/util/bytebuffer.cpp +++ b/document/src/vespa/document/util/bytebuffer.cpp @@ -12,14 +12,6 @@ #include #include -#define LOG_DEBUG1(a) -// Enable this macros instead to see what bytebuffer calls come -//#define LOG_DEBUG1(a) std::cerr << "ByteBuffer(" << ((void*) this) << " " << a << ")\n"; - -#define LOG_DEBUG2(a,b) LOG_DEBUG1(vespalib::make_string(a,b)); -#define LOG_DEBUG3(a,b,c) LOG_DEBUG1(vespalib::make_string(a,b,c)); -#define LOG_DEBUG4(a,b,c,d) LOG_DEBUG1(vespalib::make_string(a,b,c,d)); - using vespalib::alloc::Alloc; namespace document { @@ -46,15 +38,13 @@ InputOutOfRangeException::InputOutOfRangeException( } ByteBuffer::ByteBuffer() : - _buffer(NULL), + _buffer(nullptr), _len(0), _pos(0), _limit(0), - _bufHolder(NULL), _ownedBuffer() { - set(NULL, 0); - LOG_DEBUG1("Created empty bytebuffer"); + set(nullptr, 0); } ByteBuffer::ByteBuffer(size_t len) : @@ -63,11 +53,10 @@ ByteBuffer::ByteBuffer(size_t len) : } ByteBuffer::ByteBuffer(const char* buffer, size_t len) : - _buffer(NULL), + _buffer(nullptr), _len(0), _pos(0), _limit(0), - _bufHolder(NULL), _ownedBuffer() { set(buffer, len); @@ -78,33 +67,17 @@ ByteBuffer::ByteBuffer(Alloc buffer, size_t len) : _len(len), _pos(0), _limit(len), - _bufHolder(NULL), _ownedBuffer(std::move(buffer)) { } -ByteBuffer::ByteBuffer(BufferHolder* buf, size_t pos, size_t len, size_t limit) : - _buffer(NULL), - _len(0), - _pos(0), - _limit(0), - _bufHolder(NULL), - _ownedBuffer() -{ - set(buf, pos, len, limit); - LOG_DEBUG3("Created copy of byte buffer of length %" PRIu64 " with " - "limit %" PRIu64 ".", len, limit); -} - ByteBuffer::ByteBuffer(const ByteBuffer& bb) : _buffer(0), _len(0), _pos(0), _limit(0), - _bufHolder(NULL), _ownedBuffer() { - LOG_DEBUG1("Created empty byte buffer to assign to."); *this = bb; } @@ -121,99 +94,17 @@ ByteBuffer& ByteBuffer::operator=(const ByteBuffer & org) _len = org._len; _pos = org._pos; _limit = org._limit; - LOG_DEBUG4("Assignment created new buffer of size %" PRIu64 " at pos " - "%" PRIu64 " with limit %" PRIu64 ".", - _len, _pos, _limit); } return *this; } -void -ByteBuffer::set(BufferHolder* buf, size_t pos, size_t len, size_t limit) -{ - cleanUp(); - _bufHolder = buf; - _bufHolder->addRef(); - _buffer = static_cast(_bufHolder->_buffer.get()); - _pos=pos; - _len=len; - _limit=limit; - LOG_DEBUG4("set() created new buffer of size %" PRIu64 " at pos " - "%" PRIu64 " with limit %" PRIu64 ".", - _len, _pos, _limit); -} - -ByteBuffer::~ByteBuffer() -{ - if (_bufHolder) { - _bufHolder->subRef(); - } -} - -std::unique_ptr -ByteBuffer::sliceCopy() const -{ - ByteBuffer* buf = new ByteBuffer; - buf->sliceFrom(*this, _pos, _limit); - - LOG_DEBUG3("Created slice at pos %" PRIu64 " with limit %" PRIu64 ".", - _pos, _limit); - return std::unique_ptr(buf); -} +ByteBuffer::~ByteBuffer() = default; void ByteBuffer::throwOutOfBounds(size_t want, size_t has) { - LOG_DEBUG1("Throwing out of bounds exception"); throw BufferOutOfBoundsException(want, has, VESPA_STRLOC); } -void -ByteBuffer::sliceFrom(const ByteBuffer& buf, size_t from, size_t to) // throw (BufferOutOfBoundsException) -{ - LOG_DEBUG3("Created slice from buffer from %" PRIu64 " to %" PRIu64 ".", - from, to); - if (from > buf._len) { - throwOutOfBounds(from, buf._len); - } else if (to > buf._len) { - throwOutOfBounds(to, buf._len); - } else if (to < from) { - throwOutOfBounds(to, from); - } else { - - if (!buf._buffer) { - clear(); - return; - } - - // Slicing from someone that doesn't own their buffer, must make own copy. - if (( buf._ownedBuffer.get() == NULL ) && (buf._bufHolder == NULL)) { - cleanUp(); - Alloc::alloc(to-from + 1).swap(_ownedBuffer); - _buffer = static_cast(_ownedBuffer.get()); - memcpy(_buffer, buf._buffer + from, to-from); - _buffer[to-from] = 0; - _pos = 0; - _len = _limit = to-from; - return; - } - - // Slicing from someone that owns, but hasn't made a reference counter yet. - if (!buf._bufHolder) { - buf._bufHolder=new BufferHolder(std::move(const_cast(buf._ownedBuffer))); - } - - // Slicing from refcounter. - cleanUp(); - - _bufHolder = buf._bufHolder; - _bufHolder->addRef(); - _buffer = static_cast(_bufHolder->_buffer.get()); - _pos=from; - _len=to; - _limit=to; - } -} - ByteBuffer* ByteBuffer::copyBuffer(const char* buffer, size_t len) { if (buffer && len) { @@ -222,15 +113,13 @@ ByteBuffer* ByteBuffer::copyBuffer(const char* buffer, size_t len) static_cast(newBuf.get())[len] = 0; return new ByteBuffer(std::move(newBuf), len); } else { - return NULL; + return nullptr; } } void ByteBuffer::setPos(size_t pos) // throw (BufferOutOfBoundsException) { - LOG_DEBUG3("Setting pos to be %" PRIu64 ", limit is %" PRIu64 ".", - pos, _limit); if (pos>_limit) { throwOutOfBounds(pos, _limit); } else { @@ -241,7 +130,6 @@ ByteBuffer::setPos(size_t pos) // throw (BufferOutOfBoundsException) void ByteBuffer::setLimit(size_t limit) // throw (BufferOutOfBoundsException) { - LOG_DEBUG3("Setting limit to %" PRIu64 ", (size is %" PRIu64 ").", limit, _len); if (limit>_len) { throwOutOfBounds(limit, _len); } else { @@ -249,29 +137,8 @@ ByteBuffer::setLimit(size_t limit) // throw (BufferOutOfBoundsException) } } - -ByteBuffer::BufferHolder::BufferHolder(Alloc buffer) - : _buffer(std::move(buffer)) -{ -} - -ByteBuffer::BufferHolder::~BufferHolder() = default; -void ByteBuffer::dump() const -{ - fprintf(stderr, "ByteBuffer: Length %lu, Pos %lu, Limit %lu\n", - _len, _pos, _limit); - for (size_t i=0; i<_len; i++) { - if (_buffer[i]>32 && _buffer[i]<126) { - fprintf(stderr, "%c", _buffer[i]); - } else { - fprintf(stderr, "[%d]",_buffer[i]); - } - } -} - void ByteBuffer::incPos(size_t pos) { - LOG_DEBUG2("incPos(%" PRIu64 ")", pos); if (_pos + pos > _limit) { throwOutOfBounds(_pos + pos, _limit); } else { @@ -283,7 +150,6 @@ void ByteBuffer::incPos(size_t pos) } void ByteBuffer::getNumeric(uint8_t & v) { - LOG_DEBUG2("getNumeric8(%d)", (int) v); if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { @@ -293,7 +159,6 @@ void ByteBuffer::getNumeric(uint8_t & v) { } void ByteBuffer::putNumeric(uint8_t v) { - LOG_DEBUG2("putNumeric8(%d)", (int) v); if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { @@ -330,7 +195,6 @@ size_t ByteBuffer::forceValgrindPos2Lim() const void ByteBuffer::getNumericNetwork(int16_t & v) { - LOG_DEBUG2("getNumericNetwork16(%d)", (int) v); if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { @@ -340,18 +204,7 @@ void ByteBuffer::getNumericNetwork(int16_t & v) { } } -void ByteBuffer::getNumeric(int16_t & v) { - LOG_DEBUG2("getNumeric16(%d)", (int) v); - if (__builtin_expect(getRemaining() < sizeof(v), 0)) { - throwOutOfBounds(getRemaining(), sizeof(v)); - } else { - v = *(int16_t *) (void *) getBufferAtPos(); - incPosNoCheck(sizeof(v)); - } -} - void ByteBuffer::putNumericNetwork(int16_t v) { - LOG_DEBUG2("putNumericNetwork16(%d)", (int) v); if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { @@ -361,18 +214,7 @@ void ByteBuffer::putNumericNetwork(int16_t v) { } } -void ByteBuffer::putNumeric(int16_t v) { - LOG_DEBUG2("putNumeric16(%d)", (int) v); - if (__builtin_expect(getRemaining() < sizeof(v), 0)) { - throwOutOfBounds(getRemaining(), sizeof(v)); - } else { - *(int16_t *) (void *) getBufferAtPos() = v; - incPosNoCheck(sizeof(v)); - } -} - void ByteBuffer::getNumericNetwork(int32_t & v) { - LOG_DEBUG2("getNumericNetwork32(%d)", (int) v); if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { @@ -383,7 +225,6 @@ void ByteBuffer::getNumericNetwork(int32_t & v) { } void ByteBuffer::getNumeric(int32_t & v) { - LOG_DEBUG2("getNumeric32(%d)", (int) v); if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { @@ -394,7 +235,6 @@ void ByteBuffer::getNumeric(int32_t & v) { void ByteBuffer::putNumericNetwork(int32_t v) { - LOG_DEBUG2("putNumericNetwork32(%d)", (int) v); if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { @@ -405,7 +245,6 @@ void ByteBuffer::putNumericNetwork(int32_t v) { } void ByteBuffer::putNumeric(int32_t v) { - LOG_DEBUG2("putNumeric32(%d)", (int) v); if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { @@ -414,17 +253,7 @@ void ByteBuffer::putNumeric(int32_t v) { } } -void ByteBuffer::getNumericNetwork(float & v) { - LOG_DEBUG2("getNumericNetworkFloat(%f)", v); - // XXX depends on sizeof(float) == sizeof(uint32_t) == 4 - // and endianness same for float and ints - int32_t val; - getIntNetwork(val); - memcpy(&v, &val, sizeof(v)); -} - void ByteBuffer::getNumeric(float & v) { - LOG_DEBUG2("getNumericFloat(%f)", v); if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { @@ -433,26 +262,7 @@ void ByteBuffer::getNumeric(float & v) { } } -void ByteBuffer::putNumericNetwork(float v) { - LOG_DEBUG2("putNumericNetworkFloat(%f)", v); - // XXX depends on sizeof(float) == sizeof(int32_t) == 4 - // and endianness same for float and ints - int32_t val; - memcpy(&val, &v, sizeof(val)); - putIntNetwork(val); -} - -void ByteBuffer::putNumeric(float v) { - LOG_DEBUG2("putNumericFloat(%f)", v); - if (__builtin_expect(getRemaining() < sizeof(v), 0)) { - throwOutOfBounds(getRemaining(), sizeof(v)); - } else { - *(float *) (void *) getBufferAtPos() = v; - incPosNoCheck(sizeof(v)); - } -} void ByteBuffer::getNumeric(int64_t& v) { - LOG_DEBUG2("getNumeric64(%" PRId64 ")", v); if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { @@ -460,17 +270,8 @@ void ByteBuffer::getNumeric(int64_t& v) { incPosNoCheck(sizeof(v)); } } -void ByteBuffer::putNumeric(int64_t v) { - LOG_DEBUG2("putNumeric64(%" PRId64 ")", v); - if (__builtin_expect(getRemaining() < sizeof(v), 0)) { - throwOutOfBounds(getRemaining(), sizeof(v)); - } else { - *(int64_t *) (void *) getBufferAtPos() = v; - incPosNoCheck(sizeof(v)); - } -} + void ByteBuffer::getNumeric(double& v) { - LOG_DEBUG2("getNumericDouble(%f)", v); if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { @@ -479,7 +280,6 @@ void ByteBuffer::getNumeric(double& v) { } } void ByteBuffer::putNumeric(double v) { - LOG_DEBUG2("putNumericDouble(%f)", v); if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { @@ -489,24 +289,19 @@ void ByteBuffer::putNumeric(double v) { } void ByteBuffer::getNumericNetwork(double & v) { - LOG_DEBUG2("getNumericNetworkDouble(%f)", v); getDoubleLongNetwork(v); } void ByteBuffer::putNumericNetwork(int64_t v) { - LOG_DEBUG2("putNumericNetwork64(%" PRId64 ")", v); putDoubleLongNetwork(v); } void ByteBuffer::putNumericNetwork(double v) { - LOG_DEBUG2("putNumericNetworkDouble(%f)", v); putDoubleLongNetwork(v); } void ByteBuffer::getNumericNetwork(int64_t & v) { - LOG_DEBUG2("getNumericNetwork64(%" PRId64 ")", v); getDoubleLongNetwork(v); } void ByteBuffer::putInt2_4_8Bytes(int64_t number, size_t len) { - LOG_DEBUG3("putInt2_4_8(%" PRId64 ", %" PRIu64 ")", number, len); if (number < 0ll) { throw InputOutOfRangeException(vespalib::make_string( "Cannot encode negative number."), VESPA_STRLOC); @@ -542,7 +337,6 @@ void ByteBuffer::putInt2_4_8Bytes(int64_t number, size_t len) { } void ByteBuffer::getInt2_4_8Bytes(int64_t & v) { - LOG_DEBUG2("getInt2_4_8(%" PRId64 ")", v); if (getRemaining() >= 2) { uint8_t flagByte = peekByte(); @@ -588,7 +382,6 @@ size_t ByteBuffer::getSerializedSize2_4_8Bytes(int64_t number) { } void ByteBuffer::putInt1_2_4Bytes(int32_t number) { - LOG_DEBUG2("putInt1_2_4Bytes(%i)", number); if (number < 0) { throw InputOutOfRangeException(vespalib::make_string( "Cannot encode negative number."), VESPA_STRLOC); @@ -607,7 +400,6 @@ void ByteBuffer::putInt1_2_4Bytes(int32_t number) { } void ByteBuffer::getInt1_2_4Bytes(int32_t & v) { - LOG_DEBUG2("getInt1_2_4Bytes(%i)", v); if (getRemaining() >= 1) { unsigned char flagByte = peekByte(); @@ -650,7 +442,6 @@ size_t ByteBuffer::getSerializedSize1_2_4Bytes(int32_t number) { } } void ByteBuffer::putInt1_4Bytes(int32_t number) { - LOG_DEBUG2("putInt1_4Bytes(%i)", number); if (number < 0) { throw InputOutOfRangeException(vespalib::make_string( "Cannot encode negative number."), VESPA_STRLOC); @@ -666,7 +457,6 @@ void ByteBuffer::putInt1_4Bytes(int32_t number) { } } void ByteBuffer::getInt1_4Bytes(int32_t & v) { - LOG_DEBUG2("getInt1_4Bytes(%i)", v); if (getRemaining() >= 1) { unsigned char flagByte = peekByte(); @@ -700,13 +490,11 @@ size_t ByteBuffer::getSerializedSize1_4Bytes(int32_t number) { } void ByteBuffer::getBytes(void *buffer, size_t count) { - LOG_DEBUG3("getBytes(%p, %" PRIu64 ")", buffer, count); const char *v = getBufferAtPos(); incPos(count); memcpy(buffer, v, count); } void ByteBuffer::putBytes(const void *buf, size_t count) { - LOG_DEBUG3("putBytes(%p, %" PRIu64 ")", buf, count); if (__builtin_expect(getRemaining() < count, 0)) { throwOutOfBounds(getRemaining(), sizeof(count)); } else { @@ -721,8 +509,6 @@ std::string ByteBuffer::toString() { } void ByteBuffer::swap(ByteBuffer& other) { - LOG_DEBUG2("swap(%p)", &other); - std::swap(_bufHolder, other._bufHolder); std::swap(_buffer, other._buffer); std::swap(_len, other._len); std::swap(_pos, other._pos); @@ -730,14 +516,8 @@ void ByteBuffer::swap(ByteBuffer& other) { } void ByteBuffer::cleanUp() { - LOG_DEBUG1("cleanUp()"); - if (_bufHolder) { - _bufHolder->subRef(); - _bufHolder = NULL; - } else { - Alloc().swap(_ownedBuffer); - } - _buffer = NULL; + Alloc().swap(_ownedBuffer); + _buffer = nullptr; } } // document diff --git a/document/src/vespa/document/util/bytebuffer.h b/document/src/vespa/document/util/bytebuffer.h index 6467e6d8bf0..a49bd55f8fe 100644 --- a/document/src/vespa/document/util/bytebuffer.h +++ b/document/src/vespa/document/util/bytebuffer.h @@ -15,7 +15,6 @@ #pragma once #include -#include namespace document { @@ -31,6 +30,8 @@ public: ByteBuffer(const ByteBuffer &); ByteBuffer& operator=(const ByteBuffer &); + ByteBuffer(ByteBuffer &&) = default; + ByteBuffer& operator=(ByteBuffer &&) = default; ~ByteBuffer(); @@ -66,7 +67,7 @@ public: } /** Clear this buffer, and set free the underlying BufferHolder. */ - void reset() { set(NULL, 0); } + void reset() { set(nullptr, 0); } /** * Creates a ByteBuffer object from another buffer. allocates @@ -75,30 +76,17 @@ public: * @param buffer The buffer to copy. * @param len The length of the buffer. * - * @return Returns a newly created bytebuffer object, or NULL - * if buffer was NULL, or len was <=0. + * @return Returns a newly created bytebuffer object, or nullptr + * if buffer was nullptr, or len was <=0. */ static ByteBuffer* copyBuffer(const char* buffer, size_t len); - std::unique_ptr sliceCopy() const; - - /** - * @throws BufferOutOfBoundsException If faulty range is given. - */ - void sliceFrom(const ByteBuffer& buf, size_t from, size_t to); - /** @return Returns the buffer pointed to by this object (at position 0) */ char* getBuffer() const { return _buffer; } /** @return Returns the length of the buffer pointed to by this object. */ size_t getLength() const { return _len; } - /** - * Adjust the length of the buffer. Only sane to shorten it, as you do not - * know what is ahead. - */ - void setLength(size_t len) { _len = len; } - /** @return Returns a pointer to the current position in the buffer. */ char* getBufferAtPos() const { return _buffer + _pos; } @@ -169,26 +157,19 @@ public: _limit=_len; } - void getNumericNetwork(uint8_t & v) { getNumeric(v); } void getNumeric(uint8_t & v); - void putNumericNetwork(uint8_t v) { putNumeric(v); } void putNumeric(uint8_t v); void getNumericNetwork(int16_t & v); - void getNumeric(int16_t & v); void putNumericNetwork(int16_t v); - void putNumeric(int16_t v); void getNumericNetwork(int32_t & v); void getNumeric(int32_t & v); void putNumericNetwork(int32_t v); void putNumeric(int32_t v); - void getNumericNetwork(float & v); void getNumeric(float & v); - void putNumericNetwork(float v); - void putNumeric(float v); + void getNumericNetwork(int64_t & v); void getNumeric(int64_t& v); void putNumericNetwork(int64_t v); - void putNumeric(int64_t v); void getNumericNetwork(double & v); void getNumeric(double& v); void putNumericNetwork(double v); @@ -197,21 +178,15 @@ public: void getByte(uint8_t & v) { getNumeric(v); } void putByte(uint8_t v) { putNumeric(v); } void getShortNetwork(int16_t & v) { getNumericNetwork(v); } - void getShort(int16_t & v) { getNumeric(v); } void putShortNetwork(int16_t v) { putNumericNetwork(v); } - void putShort(int16_t v) { putNumeric(v); } void getIntNetwork(int32_t & v) { getNumericNetwork(v); } void getInt(int32_t & v) { getNumeric(v); } void putIntNetwork(int32_t v) { putNumericNetwork(v); } void putInt(int32_t v) { putNumeric(v); } - void getFloatNetwork(float & v) { getNumericNetwork(v); } void getFloat(float & v) { getNumeric(v); } - void putFloatNetwork(float v) { putNumericNetwork(v); } - void putFloat(float v) { putNumeric(v); } void getLongNetwork(int64_t & v) { getNumericNetwork(v); } void getLong(int64_t& v) { getNumeric(v); } void putLongNetwork(int64_t v) { putNumericNetwork(v); } - void putLong(int64_t v) { putNumeric(v); } void getDoubleNetwork(double & v) { getNumericNetwork(v); } void getDouble(double& v) { getNumeric(v); } void putDoubleNetwork(double v) { putNumericNetwork(v); } @@ -343,7 +318,6 @@ public: * reached */ void getChar(char & val) { unsigned char t;getByte(t); val=t; } - void putChar(char val) { putByte(static_cast(val)); } /** * Reads the given number of bytes into the given pointer, and updates the @@ -365,39 +339,16 @@ public: */ void putBytes(const void *buf, size_t count); - /** Debug */ - void dump() const; - - class BufferHolder : public vespalib::ReferenceCounter - { - private: - BufferHolder(const BufferHolder &); - BufferHolder& operator=(const BufferHolder &); - - public: - BufferHolder(vespalib::alloc::Alloc buffer); - virtual ~BufferHolder(); - - vespalib::alloc::Alloc _buffer; - }; - - ByteBuffer(BufferHolder* buf, size_t pos, size_t len, size_t limit); - - void set(BufferHolder* buf, size_t pos, size_t len, size_t limit); - private: char * _buffer; size_t _len; size_t _pos; size_t _limit; - mutable BufferHolder * _bufHolder; vespalib::alloc::Alloc _ownedBuffer; public: std::string toString(); - void swap(ByteBuffer& other); - void cleanUp(); }; diff --git a/storage/src/vespa/storage/common/storagelink.cpp b/storage/src/vespa/storage/common/storagelink.cpp index f73eb3ea36d..065f0b0b750 100644 --- a/storage/src/vespa/storage/common/storagelink.cpp +++ b/storage/src/vespa/storage/common/storagelink.cpp @@ -4,6 +4,7 @@ #include "bucketmessages.h" #include #include +#include #include LOG_SETUP(".application.link"); diff --git a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp index e143f4d8570..15a57c1e7ee 100644 --- a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp +++ b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp @@ -2,6 +2,7 @@ #include "simplemaintenancescanner.h" #include #include +#include namespace storage::distributor { @@ -16,10 +17,10 @@ SimpleMaintenanceScanner::SimpleMaintenanceScanner(BucketPriorityDatabase& bucke { } -SimpleMaintenanceScanner::~SimpleMaintenanceScanner() {} +SimpleMaintenanceScanner::~SimpleMaintenanceScanner() = default; -SimpleMaintenanceScanner::PendingMaintenanceStats::PendingMaintenanceStats() {} -SimpleMaintenanceScanner::PendingMaintenanceStats::~PendingMaintenanceStats() {} +SimpleMaintenanceScanner::PendingMaintenanceStats::PendingMaintenanceStats() = default; +SimpleMaintenanceScanner::PendingMaintenanceStats::~PendingMaintenanceStats() = default; SimpleMaintenanceScanner::PendingMaintenanceStats::PendingMaintenanceStats(const PendingMaintenanceStats &) = default; SimpleMaintenanceScanner::PendingMaintenanceStats & SimpleMaintenanceScanner::PendingMaintenanceStats::operator = (const PendingMaintenanceStats &) = default; diff --git a/storage/src/vespa/storage/distributor/throttlingoperationstarter.cpp b/storage/src/vespa/storage/distributor/throttlingoperationstarter.cpp index abd9778d72c..9e3230a0f34 100644 --- a/storage/src/vespa/storage/distributor/throttlingoperationstarter.cpp +++ b/storage/src/vespa/storage/distributor/throttlingoperationstarter.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "throttlingoperationstarter.h" +#include namespace storage::distributor { @@ -10,8 +11,7 @@ ThrottlingOperationStarter::ThrottlingOperation::~ThrottlingOperation() } bool -ThrottlingOperationStarter::canStart(uint32_t currentOperationCount, - Priority priority) const +ThrottlingOperationStarter::canStart(uint32_t currentOperationCount, Priority priority) const { uint32_t variablePending(_maxPending - _minPending); uint32_t maxPendingForPri(_minPending + variablePending*((255.0 - priority) / 255.0)); diff --git a/vespalib/src/vespa/vespalib/util/alloc.h b/vespalib/src/vespa/vespalib/util/alloc.h index b52cace45a5..03ebc2807f9 100644 --- a/vespalib/src/vespa/vespalib/util/alloc.h +++ b/vespalib/src/vespa/vespalib/util/alloc.h @@ -59,13 +59,13 @@ public: bool resize_inplace(size_t newSize); Alloc(const Alloc &) = delete; Alloc & operator = (const Alloc &) = delete; - Alloc(Alloc && rhs) : + Alloc(Alloc && rhs) noexcept : _alloc(rhs._alloc), _allocator(rhs._allocator) { rhs.clear(); } - Alloc & operator=(Alloc && rhs) { + Alloc & operator=(Alloc && rhs) noexcept { if (this != & rhs) { if (_alloc.first != nullptr) { _allocator->free(_alloc); -- cgit v1.2.3 From d092c8e45fe74f0fe7808a6d4fa363fb0a009181 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 17 Jan 2020 09:25:04 +0000 Subject: GC unused code and simplify StructFieldValue. --- document/src/tests/documenttestcase.cpp | 15 +++++-- .../document/fieldvalue/serializablearray.cpp | 1 - .../vespa/document/fieldvalue/serializablearray.h | 25 ------------ .../vespa/document/fieldvalue/structfieldvalue.cpp | 11 +++-- .../vespa/document/fieldvalue/structfieldvalue.h | 9 ++--- .../storage/storageserver/storagemetricsset.cpp | 47 ---------------------- .../storage/storageserver/storagemetricsset.h | 21 ++-------- 7 files changed, 22 insertions(+), 107 deletions(-) (limited to 'storage') diff --git a/document/src/tests/documenttestcase.cpp b/document/src/tests/documenttestcase.cpp index 93bcd61ddb0..9a3d9c94a91 100644 --- a/document/src/tests/documenttestcase.cpp +++ b/document/src/tests/documenttestcase.cpp @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include -#include #include #include #include @@ -9,6 +8,9 @@ #include #include #include +#include +#include + #include #include #include @@ -28,10 +30,15 @@ using namespace fieldvalue; TEST(DocumentTest, testSizeOf) { + EXPECT_EQ(24u, sizeof(std::vector)); + EXPECT_EQ(24u, sizeof(vespalib::alloc::Alloc)); + EXPECT_EQ(56u, sizeof(ByteBuffer)); + EXPECT_EQ(32u, sizeof(vespalib::GrowableByteBuffer)); + EXPECT_EQ(56u, sizeof(ByteBuffer)); EXPECT_EQ(88ul, sizeof(IdString)); EXPECT_EQ(104ul, sizeof(DocumentId)); - EXPECT_EQ(208ul, sizeof(Document)); - EXPECT_EQ(72ul, sizeof(StructFieldValue)); + EXPECT_EQ(200ul, sizeof(Document)); + EXPECT_EQ(64ul, sizeof(StructFieldValue)); EXPECT_EQ(24ul, sizeof(StructuredFieldValue)); EXPECT_EQ(64ul, sizeof(SerializableArray)); } @@ -63,7 +70,7 @@ TEST(DocumentTest, testFieldPath) class Handler : public fieldvalue::IteratorHandler { public: Handler(); - ~Handler(); + ~Handler() override; const std::string & getResult() const { return _result; } private: void onPrimitive(uint32_t, const Content&) override { diff --git a/document/src/vespa/document/fieldvalue/serializablearray.cpp b/document/src/vespa/document/fieldvalue/serializablearray.cpp index ff0fe3dbdf0..89b54b3e51a 100644 --- a/document/src/vespa/document/fieldvalue/serializablearray.cpp +++ b/document/src/vespa/document/fieldvalue/serializablearray.cpp @@ -26,7 +26,6 @@ public: } -SerializableArray::Statistics SerializableArray::_stats; SerializableArray::SerializableArray() : _serializedCompression(CompressionConfig::NONE), diff --git a/document/src/vespa/document/fieldvalue/serializablearray.h b/document/src/vespa/document/fieldvalue/serializablearray.h index 7ee09f592b2..102b4519c79 100644 --- a/document/src/vespa/document/fieldvalue/serializablearray.h +++ b/document/src/vespa/document/fieldvalue/serializablearray.h @@ -35,25 +35,6 @@ namespace serializablearray { class SerializableArray : public vespalib::Cloneable { public: - // Counts set during serialization, in order to provide metrics for how - // often we use cached version, and how often we compress. - struct Statistics { - uint64_t _usedCachedSerializationCount; - uint64_t _compressedDocumentCount; - uint64_t _compressionDidntHelpCount; - uint64_t _uncompressableCount; - uint64_t _serializedUncompressed; - uint64_t _inputWronglySerialized; - - Statistics() - : _usedCachedSerializationCount(0), - _compressedDocumentCount(0), - _compressionDidntHelpCount(0), - _uncompressableCount(0), - _serializedUncompressed(0), - _inputWronglySerialized(0) {} - }; - /** * Contains the id of a field, the size and a buffer reference that is either * a relative offset to a common buffer, or the buffer itself it it is not. @@ -97,12 +78,6 @@ public: static const uint32_t ReservedId = 100; static const uint32_t ReservedIdUpper = 128; - -private: - static Statistics _stats; - -public: - static Statistics& getStatistics() { return _stats; } using CP = vespalib::CloneablePtr; using UP = std::unique_ptr; using ByteBufferUP = std::unique_ptr; diff --git a/document/src/vespa/document/fieldvalue/structfieldvalue.cpp b/document/src/vespa/document/fieldvalue/structfieldvalue.cpp index 1d74e99208a..fbdc08bee0e 100644 --- a/document/src/vespa/document/fieldvalue/structfieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/structfieldvalue.cpp @@ -48,15 +48,14 @@ StructFieldValue::Chunks::~Chunks() = default; void StructFieldValue::Chunks::push_back(SerializableArray::UP item) { - assert(_sz < 2); - _chunks[_sz++].reset(item.release()); + assert(size() < 2); + _chunks[size()] = std::move(item); } void StructFieldValue::Chunks::clear() { _chunks[0].reset(); _chunks[1].reset(); - _sz = 0; } const StructDataType & @@ -182,8 +181,8 @@ StructFieldValue::getFieldValue(const Field& field) const nbostream stream(buf.c_str(), buf.size()); FieldValue::UP value(field.getDataType().createFieldValue()); if ((_repo == nullptr) && (_doc_type != nullptr)) { - std::unique_ptr tmpRepo(new DocumentTypeRepo(*_doc_type)); - createFV(*value, *tmpRepo, stream, *_doc_type, _version); + DocumentTypeRepo tmpRepo(*_doc_type); + createFV(*value, tmpRepo, stream, *_doc_type, _version); } else { createFV(*value, *_repo, stream, *_doc_type, _version); } @@ -262,7 +261,7 @@ StructFieldValue::setFieldValue(const Field& field, FieldValue::UP value) _chunks.push_back(std::make_unique()); } - _chunks.back().set(fieldId, std::move(serialized)); + _chunks[0].set(fieldId, std::move(serialized)); _hasChanged = true; } diff --git a/document/src/vespa/document/fieldvalue/structfieldvalue.h b/document/src/vespa/document/fieldvalue/structfieldvalue.h index b5b15e9dfce..30500229813 100644 --- a/document/src/vespa/document/fieldvalue/structfieldvalue.h +++ b/document/src/vespa/document/fieldvalue/structfieldvalue.h @@ -27,19 +27,16 @@ class StructFieldValue : public StructuredFieldValue public: class Chunks { public: - Chunks() : _sz(0) { } + Chunks() { } ~Chunks(); SerializableArray & operator [] (size_t i) { return *_chunks[i]; } const SerializableArray & operator [] (size_t i) const { return *_chunks[i]; } VESPA_DLL_LOCAL void push_back(SerializableArray::UP item); - SerializableArray & back() { return *_chunks[_sz-1]; } - const SerializableArray & back() const { return *_chunks[_sz-1]; } - size_t size() const { return _sz; } - bool empty() const { return _sz == 0; } + size_t size() const { return _chunks[1] ? 2 : _chunks[0] ? 1 : 0; } + bool empty() const { return !_chunks[0]; } VESPA_DLL_LOCAL void clear(); private: SerializableArray::CP _chunks[2]; - size_t _sz; }; private: Chunks _chunks; diff --git a/storage/src/vespa/storage/storageserver/storagemetricsset.cpp b/storage/src/vespa/storage/storageserver/storagemetricsset.cpp index f0e64f0dfd1..f3240f0663b 100644 --- a/storage/src/vespa/storage/storageserver/storagemetricsset.cpp +++ b/storage/src/vespa/storage/storageserver/storagemetricsset.cpp @@ -16,37 +16,6 @@ MessageMemoryUseMetricSet::MessageMemoryUseMetricSet(metrics::MetricSet* owner) MessageMemoryUseMetricSet::~MessageMemoryUseMetricSet() = default; -DocumentSerializationMetricSet::DocumentSerializationMetricSet(metrics::MetricSet* owner) - : metrics::MetricSet("document_serialization", {{"docserialization"}}, - "Counts of document serialization of various types", owner), - usedCachedSerializationCount( - "cached_serialization_count", {{"docserialization"}}, - "Number of times we didn't need to serialize the document as " - "we already had serialized version cached", this), - compressedDocumentCount( - "compressed_serialization_count", {{"docserialization"}}, - "Number of times we compressed document when serializing", - this), - compressionDidntHelpCount( - "compressed_didnthelp_count", {{"docserialization"}}, - "Number of times we compressed document when serializing, but " - "the compressed version was bigger, so it was dumped", this), - uncompressableCount( - "uncompressable_serialization_count", {{"docserialization"}}, - "Number of times we didn't attempt compression as document " - "had already been tagged uncompressable", this), - serializedUncompressed( - "uncompressed_serialization_count", {{"docserialization"}}, - "Number of times we serialized a document uncompressed", this), - inputWronglySerialized( - "input_wrongly_serialized_count", {{"docserialization"}}, - "Number of times we reserialized a document because the " - "compression it had in cache did not match what was configured", - this) -{} - -DocumentSerializationMetricSet::~DocumentSerializationMetricSet() = default; - StorageMetricSet::StorageMetricSet() : metrics::MetricSet("server", {{"memory"}}, "Metrics for VDS applications"), @@ -54,28 +23,12 @@ StorageMetricSet::StorageMetricSet() memoryUse_messages(this), memoryUse_visiting("memoryusage_visiting", {{"memory"}}, "Message use from visiting", this), - documentSerialization(this), tls_metrics(this) {} StorageMetricSet::~StorageMetricSet() = default; void StorageMetricSet::updateMetrics() { - document::SerializableArray::Statistics stats( - document::SerializableArray::getStatistics()); - - documentSerialization.usedCachedSerializationCount.set( - stats._usedCachedSerializationCount); - documentSerialization.compressedDocumentCount.set( - stats._compressedDocumentCount); - documentSerialization.compressionDidntHelpCount.set( - stats._compressionDidntHelpCount); - documentSerialization.uncompressableCount.set( - stats._uncompressableCount); - documentSerialization.serializedUncompressed.set( - stats._serializedUncompressed); - documentSerialization.inputWronglySerialized.set( - stats._inputWronglySerialized); // Delta snapshotting is destructive, so if an explicit snapshot is triggered // (instead of just regular periodic snapshots), some events will effectively diff --git a/storage/src/vespa/storage/storageserver/storagemetricsset.h b/storage/src/vespa/storage/storageserver/storagemetricsset.h index e9378010540..49795c63324 100644 --- a/storage/src/vespa/storage/storageserver/storagemetricsset.h +++ b/storage/src/vespa/storage/storageserver/storagemetricsset.h @@ -3,7 +3,6 @@ #pragma once #include "tls_statistics_metrics_wrapper.h" - #include namespace storage { @@ -17,21 +16,8 @@ public: metrics::LongValueMetric highpri; metrics::LongValueMetric veryhighpri; - MessageMemoryUseMetricSet(metrics::MetricSet* owner); - ~MessageMemoryUseMetricSet(); -}; - -struct DocumentSerializationMetricSet : public metrics::MetricSet -{ - metrics::LongCountMetric usedCachedSerializationCount; - metrics::LongCountMetric compressedDocumentCount; - metrics::LongCountMetric compressionDidntHelpCount; - metrics::LongCountMetric uncompressableCount; - metrics::LongCountMetric serializedUncompressed; - metrics::LongCountMetric inputWronglySerialized; - - DocumentSerializationMetricSet(metrics::MetricSet* owner); - ~DocumentSerializationMetricSet(); + explicit MessageMemoryUseMetricSet(metrics::MetricSet* owner); + ~MessageMemoryUseMetricSet() override; }; struct StorageMetricSet : public metrics::MetricSet @@ -39,12 +25,11 @@ struct StorageMetricSet : public metrics::MetricSet metrics::LongValueMetric memoryUse; MessageMemoryUseMetricSet memoryUse_messages; metrics::LongValueMetric memoryUse_visiting; - DocumentSerializationMetricSet documentSerialization; TlsStatisticsMetricsWrapper tls_metrics; StorageMetricSet(); - ~StorageMetricSet(); + ~StorageMetricSet() override; void updateMetrics(); }; -- cgit v1.2.3 From 1d3fe1bedb648cfd497eeee61478fa45f332255b Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 20 Jan 2020 13:04:49 +0000 Subject: GC a load of unused code. ByteBuffer towards read only. --- document/src/tests/documenttestcase.cpp | 7 +- document/src/tests/testbytebuffer.cpp | 264 +---------------- .../src/vespa/document/fieldvalue/fieldvalue.cpp | 5 - .../src/vespa/document/fieldvalue/fieldvalue.h | 9 +- document/src/vespa/document/util/CMakeLists.txt | 2 +- .../src/vespa/document/util/bufferexceptions.h | 8 - document/src/vespa/document/util/bytebuffer.cpp | 315 +++------------------ document/src/vespa/document/util/bytebuffer.h | 165 +---------- document/src/vespa/document/util/serializable.cpp | 72 ----- document/src/vespa/document/util/serializable.h | 97 ------- .../vespa/document/util/serializableexceptions.cpp | 21 ++ .../vespa/document/util/serializableexceptions.h | 8 - .../documentapi/messagebus/documentprotocol.h | 1 - .../messagebus/messages/queryresultmessage.cpp | 8 +- .../messagebus/messages/queryresultmessage.h | 2 +- .../documentapi/messagebus/messages/visitor.cpp | 57 +--- .../documentapi/messagebus/messages/visitor.h | 4 +- .../documentapi/messagebus/routablefactories60.cpp | 83 ++---- .../searchlib/tensor/generic_tensor_store.cpp | 9 +- .../src/vespa/vespalib/util/growablebytebuffer.cpp | 2 +- .../src/vespa/vespalib/util/growablebytebuffer.h | 2 +- storage/src/tests/common/message_sender_stub.cpp | 10 +- storage/src/tests/visiting/visitormanagertest.cpp | 2 +- .../src/vespa/storage/common/bucketmessages.cpp | 22 +- storage/src/vespa/storage/common/storagelink.cpp | 4 +- .../operations/external/statbucketoperation.cpp | 13 +- .../persistence/filestorage/filestormanager.h | 3 - .../src/vespa/storage/visiting/recoveryvisitor.cpp | 3 +- .../src/vespa/storage/visiting/recoveryvisitor.h | 12 +- storage/src/vespa/storage/visiting/visitor.cpp | 24 +- .../mbusprot/protocolserialization4_2.cpp | 135 +++------ .../mbusprot/protocolserialization5_1.cpp | 50 ++-- .../src/vespa/storageapi/mbusprot/storagereply.cpp | 6 +- storageapi/src/vespa/storageapi/message/bucket.h | 7 +- .../src/vespa/storageapi/message/visitor.cpp | 19 +- .../src/vespa/storageapi/messageapi/returncode.cpp | 56 +--- .../src/vespa/storageapi/messageapi/returncode.h | 28 +- .../vespa/storageapi/messageapi/storagereply.cpp | 11 +- .../src/vespa/storageapi/messageapi/storagereply.h | 9 +- vdslib/src/tests/container/parameterstest.cpp | 16 +- .../src/vespa/vdslib/container/documentsummary.cpp | 14 +- .../src/vespa/vdslib/container/documentsummary.h | 5 +- vdslib/src/vespa/vdslib/container/parameters.cpp | 24 +- vdslib/src/vespa/vdslib/container/parameters.h | 27 +- vdslib/src/vespa/vdslib/container/searchresult.cpp | 42 +-- vdslib/src/vespa/vdslib/container/searchresult.h | 9 +- 46 files changed, 331 insertions(+), 1361 deletions(-) delete mode 100644 document/src/vespa/document/util/serializable.cpp delete mode 100644 document/src/vespa/document/util/serializable.h create mode 100644 document/src/vespa/document/util/serializableexceptions.cpp (limited to 'storage') diff --git a/document/src/tests/documenttestcase.cpp b/document/src/tests/documenttestcase.cpp index 39a92352a5e..a8d4829d355 100644 --- a/document/src/tests/documenttestcase.cpp +++ b/document/src/tests/documenttestcase.cpp @@ -891,14 +891,11 @@ TEST(DocumentTest, testGenerateSerializedFile) CompressionConfig newCfg(CompressionConfig::LZ4, 9, 95); const_cast(doc.getType().getFieldsType()).setCompressionConfig(newCfg); - ByteBuffer lz4buf(getSerializedSize(doc)); - - doc.serialize(lz4buf); - lz4buf.flip(); + nbostream lz4buf = doc.serialize(); fd = open((serializedDir + "/serializecpp-lz4-level9.dat").c_str(), O_WRONLY | O_TRUNC | O_CREAT, 0644); - if (write(fd, lz4buf.getBufferAtPos(), lz4buf.getRemaining()) != (ssize_t)lz4buf.getRemaining()) { + if (write(fd, lz4buf.c_str(), lz4buf.size()) != (ssize_t)lz4buf.size()) { throw vespalib::Exception("write failed"); } close(fd); diff --git a/document/src/tests/testbytebuffer.cpp b/document/src/tests/testbytebuffer.cpp index b90db980683..b4ad558b4ad 100644 --- a/document/src/tests/testbytebuffer.cpp +++ b/document/src/tests/testbytebuffer.cpp @@ -3,12 +3,14 @@ #include #include #include -#include -#include #include +#include +#include #include + using namespace document; +using vespalib::GrowableByteBuffer; namespace { @@ -29,9 +31,10 @@ TEST(ByteBuffer_Test, test_constructors) TEST(ByteBuffer_Test, test_copy_constructor) { try { - ByteBuffer b1(100); - b1.putInt(1); - b1.putInt(2); + GrowableByteBuffer gb(100); + gb.putInt(1); + gb.putInt(2); + ByteBuffer b1(gb.getBuffer(), gb.position()); ByteBuffer b2(b1); @@ -40,10 +43,9 @@ TEST(ByteBuffer_Test, test_copy_constructor) EXPECT_EQ(b1.getRemaining(),b2.getRemaining()); int test = 0; - b2.flip(); - b2.getInt(test); + b2.getIntNetwork(test); EXPECT_EQ(1,test); - b2.getInt(test); + b2.getIntNetwork(test); EXPECT_EQ(2,test); } catch (std::exception &e) { @@ -51,252 +53,6 @@ TEST(ByteBuffer_Test, test_copy_constructor) } } -TEST(ByteBuffer_Test, test_putGetFlip) -{ - ByteBuffer newBuf(100); - - try { - newBuf.putInt(10); - int test; - newBuf.flip(); - - newBuf.getInt(test); - EXPECT_EQ(test, 10); - - newBuf.clear(); - newBuf.putDouble(3.35); - newBuf.flip(); - EXPECT_EQ(newBuf.getRemaining(), 100); - double test2; - newBuf.getDouble(test2); - EXPECT_TRUE(test2==3.35); - - newBuf.clear(); - newBuf.putBytes("heisann",8); - newBuf.putInt(4); - EXPECT_EQ(newBuf.getPos(), 12); - EXPECT_EQ(newBuf.getLength(), 100); - newBuf.flip(); - EXPECT_EQ(newBuf.getRemaining(), 100); - - char testStr[12]; - newBuf.getBytes(testStr, 8); - EXPECT_TRUE(strcmp(testStr,"heisann")==0); - newBuf.getInt(test); - EXPECT_TRUE(test==4); - } catch (std::exception &e) { - FAIL() << "Unexpected exception at " << VESPA_STRLOC << ": \"" << e.what() << "\""; - } -} - - -TEST(ByteBuffer_Test, test_NumberEncodings) -{ - ByteBuffer buf(1024); - - // Check 0 - buf.putInt1_2_4Bytes(124); - buf.putInt2_4_8Bytes(124); - buf.putInt1_4Bytes(124); - // Check 1 - buf.putInt1_2_4Bytes(127); - buf.putInt2_4_8Bytes(127); - buf.putInt1_4Bytes(127); - // Check 2 - buf.putInt1_2_4Bytes(128); - buf.putInt2_4_8Bytes(128); - buf.putInt1_4Bytes(128); - // Check 3 - buf.putInt1_2_4Bytes(255); - buf.putInt2_4_8Bytes(255); - buf.putInt1_4Bytes(255); - // Check 4 - buf.putInt1_2_4Bytes(256); - buf.putInt2_4_8Bytes(256); - buf.putInt1_4Bytes(256); - // Check 5 - buf.putInt1_2_4Bytes(0); - buf.putInt2_4_8Bytes(0); - buf.putInt1_4Bytes(0); - // Check 6 - buf.putInt1_2_4Bytes(1); - buf.putInt2_4_8Bytes(1); - buf.putInt1_4Bytes(1); - - // Check 7 - try { - buf.putInt1_2_4Bytes(0x7FFFFFFF); - FAIL() << "Expected input out of range exception"; - } catch (InputOutOfRangeException& e) { } - buf.putInt2_4_8Bytes(0x7FFFFFFFll); - buf.putInt1_4Bytes(0x7FFFFFFF); - - try { - buf.putInt2_4_8Bytes(0x7FFFFFFFFFFFFFFFll); - FAIL() << "Expected input out of range exception"; - } catch (InputOutOfRangeException& e) { } - - buf.putInt1_2_4Bytes(0x7FFF); - // Check 8 - buf.putInt2_4_8Bytes(0x7FFFll); - buf.putInt1_4Bytes(0x7FFF); - buf.putInt1_2_4Bytes(0x7F); - // Check 9 - buf.putInt2_4_8Bytes(0x7Fll); - buf.putInt1_4Bytes(0x7F); - - try { - buf.putInt1_2_4Bytes(-1); - FAIL() << "Expected input out of range exception"; - } catch (InputOutOfRangeException& e) { } - try { - buf.putInt2_4_8Bytes(-1); - FAIL() << "Expected input out of range exception"; - } catch (InputOutOfRangeException& e) { } - try { - buf.putInt1_4Bytes(-1); - FAIL() << "Expected input out of range exception"; - } catch (InputOutOfRangeException& e) { } - - try { - buf.putInt1_2_4Bytes(-0x7FFFFFFF); - FAIL() << "Expected input out of range exception"; - } catch (InputOutOfRangeException& e) { } - try { - buf.putInt2_4_8Bytes(-0x7FFFFFFF); - FAIL() << "Expected input out of range exception"; - } catch (InputOutOfRangeException& e) { } - try { - buf.putInt1_4Bytes(-0x7FFFFFFF); - FAIL() << "Expected input out of range exception"; - } catch (InputOutOfRangeException& e) { } - - try { - buf.putInt2_4_8Bytes(-0x7FFFFFFFFFFFFFFFll); - FAIL() << "Expected input out of range exception"; - } catch (InputOutOfRangeException& e) { } - - uint32_t endWritePos = buf.getPos(); - buf.setPos(0); - - int32_t tmp32; - int64_t tmp64; - - // Check 0 - buf.getInt1_2_4Bytes(tmp32); - EXPECT_EQ(124, tmp32); - buf.getInt2_4_8Bytes(tmp64); - EXPECT_EQ((int64_t)124, tmp64); - buf.getInt1_4Bytes(tmp32); - EXPECT_EQ(124, tmp32); - // Check 1 - buf.getInt1_2_4Bytes(tmp32); - EXPECT_EQ(127, tmp32); - buf.getInt2_4_8Bytes(tmp64); - EXPECT_EQ((int64_t)127, tmp64); - buf.getInt1_4Bytes(tmp32); - EXPECT_EQ(127, tmp32); - // Check 2 - buf.getInt1_2_4Bytes(tmp32); - EXPECT_EQ(128, tmp32); - buf.getInt2_4_8Bytes(tmp64); - EXPECT_EQ((int64_t)128, tmp64); - buf.getInt1_4Bytes(tmp32); - EXPECT_EQ(128, tmp32); - // Check 3 - buf.getInt1_2_4Bytes(tmp32); - EXPECT_EQ(255, tmp32); - buf.getInt2_4_8Bytes(tmp64); - EXPECT_EQ((int64_t)255, tmp64); - buf.getInt1_4Bytes(tmp32); - EXPECT_EQ(255, tmp32); - // Check 4 - buf.getInt1_2_4Bytes(tmp32); - EXPECT_EQ(256, tmp32); - buf.getInt2_4_8Bytes(tmp64); - EXPECT_EQ((int64_t)256, tmp64); - buf.getInt1_4Bytes(tmp32); - EXPECT_EQ(256, tmp32); - // Check 5 - buf.getInt1_2_4Bytes(tmp32); - EXPECT_EQ(0, tmp32); - buf.getInt2_4_8Bytes(tmp64); - EXPECT_EQ((int64_t)0, tmp64); - buf.getInt1_4Bytes(tmp32); - EXPECT_EQ(0, tmp32); - // Check 6 - buf.getInt1_2_4Bytes(tmp32); - EXPECT_EQ(1, tmp32); - buf.getInt2_4_8Bytes(tmp64); - EXPECT_EQ((int64_t)1, tmp64); - buf.getInt1_4Bytes(tmp32); - EXPECT_EQ(1, tmp32); - // Check 7 - buf.getInt2_4_8Bytes(tmp64); - EXPECT_EQ((int64_t)0x7FFFFFFF, tmp64); - buf.getInt1_4Bytes(tmp32); - EXPECT_EQ(0x7FFFFFFF, tmp32); - buf.getInt1_2_4Bytes(tmp32); - EXPECT_EQ(0x7FFF, tmp32); - // Check 8 - buf.getInt2_4_8Bytes(tmp64); - EXPECT_EQ((int64_t)0x7FFF, tmp64); - buf.getInt1_4Bytes(tmp32); - EXPECT_EQ(0x7FFF, tmp32); - buf.getInt1_2_4Bytes(tmp32); - EXPECT_EQ(0x7F, tmp32); - // Check 9 - buf.getInt2_4_8Bytes(tmp64); - EXPECT_EQ((int64_t)0x7F, tmp64); - buf.getInt1_4Bytes(tmp32); - EXPECT_EQ(0x7F, tmp32); - - uint32_t endReadPos = buf.getPos(); - EXPECT_EQ(endWritePos, endReadPos); - -} - -TEST(ByteBuffer_Test, test_NumberLengths) -{ - EXPECT_EQ((size_t) 1, ByteBuffer::getSerializedSize1_4Bytes(0)); - EXPECT_EQ((size_t) 1, ByteBuffer::getSerializedSize1_4Bytes(1)); - EXPECT_EQ((size_t) 1, ByteBuffer::getSerializedSize1_4Bytes(4)); - EXPECT_EQ((size_t) 1, ByteBuffer::getSerializedSize1_4Bytes(31)); - EXPECT_EQ((size_t) 1, ByteBuffer::getSerializedSize1_4Bytes(126)); - EXPECT_EQ((size_t) 1, ByteBuffer::getSerializedSize1_4Bytes(127)); - EXPECT_EQ((size_t) 4, ByteBuffer::getSerializedSize1_4Bytes(128)); - EXPECT_EQ((size_t) 4, ByteBuffer::getSerializedSize1_4Bytes(129)); - EXPECT_EQ((size_t) 4, ByteBuffer::getSerializedSize1_4Bytes(255)); - EXPECT_EQ((size_t) 4, ByteBuffer::getSerializedSize1_4Bytes(256)); - EXPECT_EQ((size_t) 4, ByteBuffer::getSerializedSize1_4Bytes(0x7FFFFFFF)); - - EXPECT_EQ((size_t) 2, ByteBuffer::getSerializedSize2_4_8Bytes(0)); - EXPECT_EQ((size_t) 2, ByteBuffer::getSerializedSize2_4_8Bytes(1)); - EXPECT_EQ((size_t) 2, ByteBuffer::getSerializedSize2_4_8Bytes(4)); - EXPECT_EQ((size_t) 2, ByteBuffer::getSerializedSize2_4_8Bytes(31)); - EXPECT_EQ((size_t) 2, ByteBuffer::getSerializedSize2_4_8Bytes(126)); - EXPECT_EQ((size_t) 2, ByteBuffer::getSerializedSize2_4_8Bytes(127)); - EXPECT_EQ((size_t) 2, ByteBuffer::getSerializedSize2_4_8Bytes(128)); - EXPECT_EQ((size_t) 2, ByteBuffer::getSerializedSize2_4_8Bytes(32767)); - EXPECT_EQ((size_t) 4, ByteBuffer::getSerializedSize2_4_8Bytes(32768)); - EXPECT_EQ((size_t) 4, ByteBuffer::getSerializedSize2_4_8Bytes(32769)); - EXPECT_EQ((size_t) 4, ByteBuffer::getSerializedSize2_4_8Bytes(1030493)); - EXPECT_EQ((size_t) 4, ByteBuffer::getSerializedSize2_4_8Bytes(0x3FFFFFFF)); - EXPECT_EQ((size_t) 8, ByteBuffer::getSerializedSize2_4_8Bytes(0x40000000)); - EXPECT_EQ((size_t) 8, ByteBuffer::getSerializedSize2_4_8Bytes(0x40000001)); - - EXPECT_EQ((size_t) 1, ByteBuffer::getSerializedSize1_2_4Bytes(0)); - EXPECT_EQ((size_t) 1, ByteBuffer::getSerializedSize1_2_4Bytes(1)); - EXPECT_EQ((size_t) 1, ByteBuffer::getSerializedSize1_2_4Bytes(4)); - EXPECT_EQ((size_t) 1, ByteBuffer::getSerializedSize1_2_4Bytes(31)); - EXPECT_EQ((size_t) 1, ByteBuffer::getSerializedSize1_2_4Bytes(126)); - EXPECT_EQ((size_t) 1, ByteBuffer::getSerializedSize1_2_4Bytes(127)); - EXPECT_EQ((size_t) 2, ByteBuffer::getSerializedSize1_2_4Bytes(128)); - EXPECT_EQ((size_t) 2, ByteBuffer::getSerializedSize1_2_4Bytes(16383)); - EXPECT_EQ((size_t) 4, ByteBuffer::getSerializedSize1_2_4Bytes(16384)); - EXPECT_EQ((size_t) 4, ByteBuffer::getSerializedSize1_2_4Bytes(16385)); -} - TEST(ByteBuffer_Test, test_SerializableArray) { SerializableArray array; diff --git a/document/src/vespa/document/fieldvalue/fieldvalue.cpp b/document/src/vespa/document/fieldvalue/fieldvalue.cpp index c41f6cbb616..49efbd83c61 100644 --- a/document/src/vespa/document/fieldvalue/fieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/fieldvalue.cpp @@ -35,11 +35,6 @@ void FieldValue::serialize(nbostream &stream) const { serializer.write(*this); } -void FieldValue::serialize(ByteBuffer& buffer) const { - nbostream stream = serialize(); - buffer.putBytes(stream.peek(), stream.size()); -} - nbostream FieldValue::serialize() const { nbostream stream; diff --git a/document/src/vespa/document/fieldvalue/fieldvalue.h b/document/src/vespa/document/fieldvalue/fieldvalue.h index 4b754f09cd6..9ed8d522b17 100644 --- a/document/src/vespa/document/fieldvalue/fieldvalue.h +++ b/document/src/vespa/document/fieldvalue/fieldvalue.h @@ -19,15 +19,11 @@ #include #include -namespace vespalib { - class nbostream; -} +namespace vespalib { class nbostream; } namespace document { -namespace fieldvalue { - class IteratorHandler; -} +namespace fieldvalue { class IteratorHandler; } class ByteBuffer; class DataType; @@ -73,7 +69,6 @@ public: virtual bool isA(const FieldValue& other) const; void serialize(vespalib::nbostream &stream) const; - void serialize(ByteBuffer& buffer) const; vespalib::nbostream serialize() const; /** diff --git a/document/src/vespa/document/util/CMakeLists.txt b/document/src/vespa/document/util/CMakeLists.txt index 8cb148abe25..48ca7bd36d7 100644 --- a/document/src/vespa/document/util/CMakeLists.txt +++ b/document/src/vespa/document/util/CMakeLists.txt @@ -3,7 +3,7 @@ vespa_add_library(document_util OBJECT SOURCES bytebuffer.cpp printable.cpp - serializable.cpp + serializableexceptions.cpp stringutil.cpp DEPENDS AFTER diff --git a/document/src/vespa/document/util/bufferexceptions.h b/document/src/vespa/document/util/bufferexceptions.h index 8a3215f6c79..aee7f3ae568 100644 --- a/document/src/vespa/document/util/bufferexceptions.h +++ b/document/src/vespa/document/util/bufferexceptions.h @@ -15,13 +15,5 @@ public: VESPA_DEFINE_EXCEPTION_SPINE(BufferOutOfBoundsException) }; -class InputOutOfRangeException : public vespalib::IoException { -public: - InputOutOfRangeException(const vespalib::string& msg, - const vespalib::string& location = ""); - - VESPA_DEFINE_EXCEPTION_SPINE(InputOutOfRangeException) -}; - } diff --git a/document/src/vespa/document/util/bytebuffer.cpp b/document/src/vespa/document/util/bytebuffer.cpp index 415aeebf969..c909ca5fe61 100644 --- a/document/src/vespa/document/util/bytebuffer.cpp +++ b/document/src/vespa/document/util/bytebuffer.cpp @@ -13,11 +13,42 @@ #include using vespalib::alloc::Alloc; +using vespalib::make_string; namespace document { +namespace { + +static void throwOutOfBounds(size_t want, size_t has) __attribute__((noinline, noreturn)); + +void throwOutOfBounds(size_t want, size_t has) +{ + throw BufferOutOfBoundsException(want, has, VESPA_STRLOC); +} + +} + +#if defined(__i386__) || defined(__x86_64__) + +template +void +ByteBuffer::getDoubleLongNetwork(T &val) { + //TODO: Change this if we move to big-endian hardware + if (__builtin_expect(getRemaining() < (int)sizeof(T), 0)) { + throwOutOfBounds(sizeof(T), getRemaining()); + } + + auto * data = reinterpret_cast(&val); + for (int i=sizeof(T)-1; i>=0; --i) { + getByte(data[i]); + } +} + +#else +#error "getDoubleLongNetwork is undefined for this arcitecture" +#endif + VESPA_IMPLEMENT_EXCEPTION_SPINE(BufferOutOfBoundsException); -VESPA_IMPLEMENT_EXCEPTION_SPINE(InputOutOfRangeException); vespalib::string BufferOutOfBoundsException::createMessage(size_t pos, size_t len) { vespalib::asciistream ost; @@ -25,18 +56,11 @@ vespalib::string BufferOutOfBoundsException::createMessage(size_t pos, size_t le return ost.str(); } -BufferOutOfBoundsException::BufferOutOfBoundsException( - size_t pos, size_t len, const vespalib::string& location) +BufferOutOfBoundsException::BufferOutOfBoundsException(size_t pos, size_t len, const vespalib::string& location) : IoException(createMessage(pos, len), IoException::NO_SPACE, location, 1) { } -InputOutOfRangeException::InputOutOfRangeException( - const vespalib::string& msg, const vespalib::string& location) - : IoException(msg, IoException::INTERNAL_FAILURE, location, 1) -{ -} - ByteBuffer::ByteBuffer(size_t len) : ByteBuffer(Alloc::alloc(len), len) { @@ -74,11 +98,6 @@ ByteBuffer::ByteBuffer(const ByteBuffer& rhs) : ByteBuffer::~ByteBuffer() = default; -void ByteBuffer::throwOutOfBounds(size_t want, size_t has) -{ - throw BufferOutOfBoundsException(want, has, VESPA_STRLOC); -} - ByteBuffer* ByteBuffer::copyBuffer(const char* buffer, size_t len) { if (buffer && len) { @@ -119,15 +138,6 @@ void ByteBuffer::getNumeric(uint8_t & v) { } } -void ByteBuffer::putNumeric(uint8_t v) { - if (__builtin_expect(getRemaining() < sizeof(v), 0)) { - throwOutOfBounds(getRemaining(), sizeof(v)); - } else { - *(uint8_t *) getBufferAtPos() = v; - incPosNoCheck(sizeof(v)); - } -} - void ByteBuffer::getNumericNetwork(int16_t & v) { if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); @@ -138,16 +148,6 @@ void ByteBuffer::getNumericNetwork(int16_t & v) { } } -void ByteBuffer::putNumericNetwork(int16_t v) { - if (__builtin_expect(getRemaining() < sizeof(v), 0)) { - throwOutOfBounds(getRemaining(), sizeof(v)); - } else { - uint16_t val = htons(v); - *(uint16_t *) (void *) getBufferAtPos() = val; - incPosNoCheck(sizeof(v)); - } -} - void ByteBuffer::getNumericNetwork(int32_t & v) { if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); @@ -158,44 +158,6 @@ void ByteBuffer::getNumericNetwork(int32_t & v) { } } -void ByteBuffer::getNumeric(int32_t & v) { - if (__builtin_expect(getRemaining() < sizeof(v), 0)) { - throwOutOfBounds(getRemaining(), sizeof(v)); - } else { - v = *(int32_t *) (void *) getBufferAtPos(); - incPosNoCheck(sizeof(v)); - } -} - - -void ByteBuffer::putNumericNetwork(int32_t v) { - if (__builtin_expect(getRemaining() < sizeof(v), 0)) { - throwOutOfBounds(getRemaining(), sizeof(v)); - } else { - uint32_t val = htonl(v); - *(uint32_t *) (void *) getBufferAtPos() = val; - incPosNoCheck(sizeof(v)); - } -} - -void ByteBuffer::putNumeric(int32_t v) { - if (__builtin_expect(getRemaining() < sizeof(v), 0)) { - throwOutOfBounds(getRemaining(), sizeof(v)); - } else { - *(int32_t *) (void *) getBufferAtPos() = v; - incPosNoCheck(sizeof(v)); - } -} - -void ByteBuffer::getNumeric(float & v) { - if (__builtin_expect(getRemaining() < sizeof(v), 0)) { - throwOutOfBounds(getRemaining(), sizeof(v)); - } else { - v = *(float *) (void *) getBufferAtPos(); - incPosNoCheck(sizeof(v)); - } -} - void ByteBuffer::getNumeric(int64_t& v) { if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); @@ -205,223 +167,14 @@ void ByteBuffer::getNumeric(int64_t& v) { } } -void ByteBuffer::getNumeric(double& v) { - if (__builtin_expect(getRemaining() < sizeof(v), 0)) { - throwOutOfBounds(getRemaining(), sizeof(v)); - } else { - v = *(double *) (void *) getBufferAtPos(); - incPosNoCheck(sizeof(v)); - } -} -void ByteBuffer::putNumeric(double v) { - if (__builtin_expect(getRemaining() < sizeof(v), 0)) { - throwOutOfBounds(getRemaining(), sizeof(v)); - } else { - *(double *) (void *) getBufferAtPos() = v; - incPosNoCheck(sizeof(v)); - } -} - void ByteBuffer::getNumericNetwork(double & v) { getDoubleLongNetwork(v); } -void ByteBuffer::putNumericNetwork(int64_t v) { - putDoubleLongNetwork(v); -} -void ByteBuffer::putNumericNetwork(double v) { - putDoubleLongNetwork(v); -} + void ByteBuffer::getNumericNetwork(int64_t & v) { getDoubleLongNetwork(v); } -void ByteBuffer::putInt2_4_8Bytes(int64_t number, size_t len) { - if (number < 0ll) { - throw InputOutOfRangeException(vespalib::make_string( - "Cannot encode negative number."), VESPA_STRLOC); - } else if (number > 0x3FFFFFFFFFFFFFFFll) { - throw InputOutOfRangeException(vespalib::make_string( - "Cannot encode number larger than 2^62."), VESPA_STRLOC); - } - - if (len == 0) { - if (number < 0x8000ll) { - //length 2 bytes - putShortNetwork((int16_t) number); - } else if (number < 0x40000000ll) { - //length 4 bytes - putIntNetwork(((int32_t) number) | 0x80000000); - } else { - //length 8 bytes - putLongNetwork(number | 0xC000000000000000ll); - } - } else if (len == 2) { - //length 2 bytes - putShortNetwork((int16_t) number); - } else if (len == 4) { - //length 4 bytes - putIntNetwork(((int32_t) number) | 0x80000000); - } else if (len == 8) { - //length 8 bytes - putLongNetwork(number | 0xC000000000000000ll); - } else { - throw InputOutOfRangeException(vespalib::make_string( - "Cannot encode number using %d bytes.", (int)len), VESPA_STRLOC); - } -} - -void ByteBuffer::getInt2_4_8Bytes(int64_t & v) { - if (getRemaining() >= 2) { - uint8_t flagByte = peekByte(); - - if (flagByte & 0x80) { - if (flagByte & 0x40) { - //length 8 bytes - int64_t tmp; - getLongNetwork(tmp); - v = tmp & 0x3FFFFFFFFFFFFFFFll; - } else { - //length 4 bytes - int32_t tmp; - getIntNetwork(tmp); - v = (int64_t) (tmp & 0x3FFFFFFF); - } - } else { - //length 2 bytes - int16_t tmp; - getShortNetwork(tmp); - v = (int64_t) tmp; - } - } else { - throwOutOfBounds(getRemaining(), 2); - } -} - -size_t ByteBuffer::getSerializedSize2_4_8Bytes(int64_t number) { - if (number < 0ll) { - throw InputOutOfRangeException(vespalib::make_string( - "Cannot encode negative number."), VESPA_STRLOC); - } else if (number > 0x3FFFFFFFFFFFFFFFll) { - throw InputOutOfRangeException(vespalib::make_string( - "Cannot encode number larger than 2^62."), VESPA_STRLOC); - } - - if (number < 0x8000ll) { - return 2; - } else if (number < 0x40000000ll) { - return 4; - } else { - return 8; - } -} - -void ByteBuffer::putInt1_2_4Bytes(int32_t number) { - if (number < 0) { - throw InputOutOfRangeException(vespalib::make_string( - "Cannot encode negative number."), VESPA_STRLOC); - } else if (number > 0x3FFFFFFF) { - throw InputOutOfRangeException(vespalib::make_string( - "Cannot encode number larger than 2^30."), VESPA_STRLOC); - } - - if (number < 0x80) { - putByte((unsigned char) number); - } else if (number < 0x4000) { - putShortNetwork((int16_t) (((int16_t)number) | ((int16_t) 0x8000))); - } else { - putIntNetwork(number | 0xC0000000); - } -} - -void ByteBuffer::getInt1_2_4Bytes(int32_t & v) { - if (getRemaining() >= 1) { - unsigned char flagByte = peekByte(); - - if (flagByte & 0x80) { - if (flagByte & 0x40) { - //length 4 bytes - int32_t tmp; - getIntNetwork(tmp); - v = tmp & 0x3FFFFFFF; - } else { - //length 2 bytes - int16_t tmp; - getShortNetwork(tmp); - v = (int32_t) (tmp & ((int16_t) 0x3FFF)); - } - } else { - v = (int32_t) flagByte; - incPosNoCheck(1); - } - } else { - throwOutOfBounds(getRemaining(), 1); - } -} - -size_t ByteBuffer::getSerializedSize1_2_4Bytes(int32_t number) { - if (number < 0) { - throw InputOutOfRangeException(vespalib::make_string( - "Cannot encode negative number."), VESPA_STRLOC); - } else if (number > 0x3FFFFFFF) { - throw InputOutOfRangeException(vespalib::make_string( - "Cannot encode number larger than 2^30."), VESPA_STRLOC); - } - - if (number < 0x80) { - return 1; - } else if (number < 0x4000) { - return 2; - } else { - return 4; - } -} -void ByteBuffer::putInt1_4Bytes(int32_t number) { - if (number < 0) { - throw InputOutOfRangeException(vespalib::make_string( - "Cannot encode negative number."), VESPA_STRLOC); - } else if (number > 0x7FFFFFFF) { - throw InputOutOfRangeException(vespalib::make_string( - "Cannot encode number larger than 2^31."), VESPA_STRLOC); - } - - if (number < 0x80) { - putByte((unsigned char) number); - } else { - putIntNetwork(number | 0x80000000); - } -} -void ByteBuffer::getInt1_4Bytes(int32_t & v) { - if (getRemaining() >= 1) { - unsigned char flagByte = peekByte(); - - if (flagByte & 0x80) { - //length 4 bytes - int32_t tmp; - getIntNetwork(tmp); - v = tmp & 0x7FFFFFFF; - } else { - v = (int32_t) flagByte; - incPosNoCheck(1); - } - } else { - throwOutOfBounds(getRemaining(), 1); - } -} -size_t ByteBuffer::getSerializedSize1_4Bytes(int32_t number) { - if (number < 0) { - throw InputOutOfRangeException(vespalib::make_string( - "Cannot encode negative number."), VESPA_STRLOC); - } else if (number > 0x7FFFFFFF) { - throw InputOutOfRangeException(vespalib::make_string( - "Cannot encode number larger than 2^31."), VESPA_STRLOC); - } - - if (number < 0x80) { - return 1; - } else { - return 4; - } -} void ByteBuffer::getBytes(void *buffer, size_t count) { const char *v = getBufferAtPos(); diff --git a/document/src/vespa/document/util/bytebuffer.h b/document/src/vespa/document/util/bytebuffer.h index b97c9ecee1a..ce01231602d 100644 --- a/document/src/vespa/document/util/bytebuffer.h +++ b/document/src/vespa/document/util/bytebuffer.h @@ -98,10 +98,6 @@ public: */ void incPos(size_t pos); - void incPosNoCheck(size_t pos) { - _pos += pos; - } - /** * Resets pos to 0, and sets limit to old pos. Use this before reading * from a buffer you have written to @@ -110,175 +106,21 @@ public: _pos = 0; } - /** - * Sets pos to 0 and limit to length. Use this to start writing from the - * start of the buffer. - */ - void clear() { - _pos=0; - } - void getNumeric(uint8_t & v); - void putNumeric(uint8_t v); void getNumericNetwork(int16_t & v); - void putNumericNetwork(int16_t v); void getNumericNetwork(int32_t & v); - void getNumeric(int32_t & v); - void putNumericNetwork(int32_t v); - void putNumeric(int32_t v); - void getNumeric(float & v); void getNumericNetwork(int64_t & v); void getNumeric(int64_t& v); - void putNumericNetwork(int64_t v); void getNumericNetwork(double & v); - void getNumeric(double& v); - void putNumericNetwork(double v); - void putNumeric(double v); + void getChar(char & val) { unsigned char t;getByte(t); val=t; } void getByte(uint8_t & v) { getNumeric(v); } - void putByte(uint8_t v) { putNumeric(v); } void getShortNetwork(int16_t & v) { getNumericNetwork(v); } - void putShortNetwork(int16_t v) { putNumericNetwork(v); } void getIntNetwork(int32_t & v) { getNumericNetwork(v); } - void getInt(int32_t & v) { getNumeric(v); } - void putIntNetwork(int32_t v) { putNumericNetwork(v); } - void putInt(int32_t v) { putNumeric(v); } - void getFloat(float & v) { getNumeric(v); } void getLongNetwork(int64_t & v) { getNumericNetwork(v); } void getLong(int64_t& v) { getNumeric(v); } - void putLongNetwork(int64_t v) { putNumericNetwork(v); } void getDoubleNetwork(double & v) { getNumericNetwork(v); } - void getDouble(double& v) { getNumeric(v); } - void putDoubleNetwork(double v) { putNumericNetwork(v); } - void putDouble(double v) { putNumeric(v); } - - private: - void throwOutOfBounds(size_t want, size_t has) __attribute__((noinline,noreturn)); - uint8_t peekByte() const { return *getBufferAtPos(); } - -#if defined(__i386__) || defined(__x86_64__) - - template - void putDoubleLongNetwork(T val) { - //TODO: Change this if we move to big-endian hardware - if (__builtin_expect(getRemaining() < (int)sizeof(T), 0)) { - throwOutOfBounds(sizeof(T), getRemaining()); - } - unsigned char* data = reinterpret_cast(&val); - for (int i=sizeof(T)-1; i>=0; --i) { - putByte(data[i]); - } - } - - template - void getDoubleLongNetwork(T &val) { - //TODO: Change this if we move to big-endian hardware - if (__builtin_expect(getRemaining() < (int)sizeof(T), 0)) { - throwOutOfBounds(sizeof(T), getRemaining()); - } - - unsigned char* data = reinterpret_cast(&val); - for (int i=sizeof(T)-1; i>=0; --i) { - getByte(data[i]); - } - } -#else - #error "getDoubleLongNetwork is undefined for this arcitecture" -#endif - - public: - /** - * Writes a 62-bit positive integer to the buffer, using 2, 4, or 8 bytes. - * - * @param number the integer to write - */ - void putInt2_4_8Bytes(int64_t number) { - putInt2_4_8Bytes(number, 0); - } - - /** - * Writes a 62-bit positive integer to the buffer, using 2, 4, or 8 bytes. - * - * @param number the integer to write - * @param len if non-zero, force writing number using len bytes, possibly - * with truncation - */ - void putInt2_4_8Bytes(int64_t number, size_t len); - - /** - * Reads a 62-bit positive integer from the buffer, which was written using - * 2, 4, or 8 bytes. - * - * @param v the integer read - */ - void getInt2_4_8Bytes(int64_t & v); - - /** - * Computes the size used for storing the given integer using 2, 4 or 8 - * bytes. - * - * @param number the integer to check length of - * @return the number of bytes used to store it; 2, 4 or 8 - */ - static size_t getSerializedSize2_4_8Bytes(int64_t number); - - /** - * Writes a 30-bit positive integer to the buffer, using 1, 2, or 4 bytes. - * - * @param number the integer to write - */ - void putInt1_2_4Bytes(int32_t number); - - /** - * Reads a 30-bit positive integer from the buffer, which was written using - * 1, 2, or 4 bytes. - * - * @param v the integer read - */ - void getInt1_2_4Bytes(int32_t & v); - - /** - * Computes the size used for storing the given integer using 1, 2 or 4 - * bytes. - * - * @param number the integer to check length of - * @return the number of bytes used to store it; 1, 2 or 4 - */ - static size_t getSerializedSize1_2_4Bytes(int32_t number); - - /** - * Writes a 31-bit positive integer to the buffer, using 1 or 4 bytes. - * - * @param number the integer to write - */ - void putInt1_4Bytes(int32_t number); - - /** - * Reads a 31-bit positive integer from the buffer, which was written using - * 1 or 4 bytes. - * - * @param v the integer read - */ - void getInt1_4Bytes(int32_t & v); - - /** - * Computes the size used for storing the given integer using 1 or 4 bytes. - * - * @param number the integer to check length of - * @return the number of bytes used to store it; 1 or 4 - */ - static size_t getSerializedSize1_4Bytes(int32_t number); - - /** - * Writes a 8 bit integer to the buffer at the current position, and - * increases the positition accordingly. - * - * @param val the int to store - * @return True if the value could be stored, false if end of buffer is - * reached - */ - void getChar(char & val) { unsigned char t;getByte(t); val=t; } /** * Reads the given number of bytes into the given pointer, and updates the @@ -301,6 +143,11 @@ public: void putBytes(const void *buf, size_t count); private: + template + void getDoubleLongNetwork(T &val); + + void incPosNoCheck(size_t pos) { _pos += pos; } + char * _buffer; size_t _len; size_t _pos; diff --git a/document/src/vespa/document/util/serializable.cpp b/document/src/vespa/document/util/serializable.cpp deleted file mode 100644 index e6881c598ac..00000000000 --- a/document/src/vespa/document/util/serializable.cpp +++ /dev/null @@ -1,72 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "serializable.h" -#include "bufferexceptions.h" -#include "serializableexceptions.h" -#include "bytebuffer.h" - -namespace document { - -IMPLEMENT_IDENTIFIABLE_ABSTRACT(Serializable, vespalib::Identifiable); -IMPLEMENT_IDENTIFIABLE_ABSTRACT(Deserializable, Serializable); -VESPA_IMPLEMENT_EXCEPTION_SPINE(DeserializeException); -VESPA_IMPLEMENT_EXCEPTION_SPINE(SerializeException); - -std::unique_ptr -Serializable::serialize() const -{ - size_t len = getSerializedSize(); - std::unique_ptr retVal(new ByteBuffer(len)); - serialize(*retVal.get()); - return retVal; -} - -DeserializeException::DeserializeException(const vespalib::string& msg, const vespalib::string& location) - : IoException(msg, IoException::CORRUPT_DATA, location, 1) -{ -} - -DeserializeException::DeserializeException( - const vespalib::string& msg, const vespalib::Exception& cause, - const vespalib::string& location) - : IoException(msg, IoException::CORRUPT_DATA, cause, location, 1) -{ -} - -SerializeException::SerializeException(const vespalib::string& msg, const vespalib::string& location) - : IoException(msg, IoException::CORRUPT_DATA, location, 1) -{ -} - -SerializeException::SerializeException( - const vespalib::string& msg, const vespalib::Exception& cause, - const vespalib::string& location) - : IoException(msg, IoException::CORRUPT_DATA, cause, location, 1) -{ -} - -void -Serializable::serialize(ByteBuffer& buffer) const { - int pos = buffer.getPos(); - try{ - onSerialize(buffer); - } catch (...) { - buffer.setPos(pos); - throw; - } -} - -void -Deserializable::deserialize(const DocumentTypeRepo &repo, ByteBuffer& buffer) { - int pos = buffer.getPos(); - try { - onDeserialize(repo, buffer); - } catch (const DeserializeException &) { - buffer.setPos(pos); - throw; - } catch (const BufferOutOfBoundsException &) { - buffer.setPos(pos); - throw; - } -} -} diff --git a/document/src/vespa/document/util/serializable.h b/document/src/vespa/document/util/serializable.h deleted file mode 100644 index 8039b3cd90b..00000000000 --- a/document/src/vespa/document/util/serializable.h +++ /dev/null @@ -1,97 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -/** - * @file serializable.h - * @ingroup document - * - * @brief Interfaces to be used for serializing of objects. - * - * @author Thomas F. Gundersen, H�kon Humberset - * @date 2004-03-15 - * @version $Id$ - */ - -#pragma once - -#include "bytebuffer.h" -#include "identifiableid.h" -#include -#include - -namespace document { -class DocumentTypeRepo; - -/** - * Base class for classes that can be converted into a bytestream, - * normally used later to create a similar instance. - */ - -class Serializable : public vespalib::Identifiable -{ -protected: - virtual void onSerialize(ByteBuffer& buffer) const = 0; -public: - DECLARE_IDENTIFIABLE_ABSTRACT(Serializable); - - virtual ~Serializable() {} - - /** - * @return An upper limit to how many bytes serialization of this instance - * need, providing instance is not altered before serialization. - */ - virtual size_t getSerializedSize() const = 0; - - /** - * Serializes the instance into the buffer given. Use getSerializedSize() - * before calling this method to be sure buffer is big enough. - * On success, the given buffers position will be just past the serialized - * version of this instance, on failure, position will be reset to whatever - * it was prior to calling this function. - * - * @throw SerializeException If for some reason instance cannot be - * serialized. - * @throw BufferOutOfBoundsException If buffer does not have enough space. - */ - void serialize(ByteBuffer& buffer) const; - - /** - * Creates a bytebuffer with enough space to serialize this instance - * and serialize this instance into it. - * - * @return The created bytebuffer, positioned after the serialization. - * - * @throw SerializeException If for some reason instance cannot be - * serialized. - * @throw BufferOutOfBoundsException If buffer does not have enough space. - */ - std::unique_ptr serialize() const; -}; - -/** - * Base class for instances that can be overwritten from a bytestream, - * given that the bytestream is created from a similar instance. - */ -class Deserializable : public vespalib::Cloneable, public Serializable -{ -protected: - virtual void onDeserialize(const DocumentTypeRepo &repo, ByteBuffer& buffer) = 0; - -public: - DECLARE_IDENTIFIABLE_ABSTRACT(Deserializable); - virtual ~Deserializable() {} - - /** - * Overwrite this object with the object represented by the given - * bytestream. On success, buffer will be positioned after the bytestream - * representing the instance we've just deserialized, on failure, bytebuffer - * will be pointing to where it was pointing before calling this function. - * - * @throw DeserializeException If read data doesn't represent a legal object - * of this type. - * @throw BufferOutOfBoundsException If instance wants to read more data - * than is available in the buffer. - */ - void deserialize(const DocumentTypeRepo &repo, ByteBuffer& buffer); -}; - -} - diff --git a/document/src/vespa/document/util/serializableexceptions.cpp b/document/src/vespa/document/util/serializableexceptions.cpp new file mode 100644 index 00000000000..e80e38015e8 --- /dev/null +++ b/document/src/vespa/document/util/serializableexceptions.cpp @@ -0,0 +1,21 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "serializableexceptions.h" + +namespace document { + +VESPA_IMPLEMENT_EXCEPTION_SPINE(DeserializeException); + +DeserializeException::DeserializeException(const vespalib::string& msg, const vespalib::string& location) + : IoException(msg, IoException::CORRUPT_DATA, location, 1) +{ +} + +DeserializeException::DeserializeException( + const vespalib::string& msg, const vespalib::Exception& cause, + const vespalib::string& location) + : IoException(msg, IoException::CORRUPT_DATA, cause, location, 1) +{ +} + +} diff --git a/document/src/vespa/document/util/serializableexceptions.h b/document/src/vespa/document/util/serializableexceptions.h index fcfed810bfc..1b692aa27b7 100644 --- a/document/src/vespa/document/util/serializableexceptions.h +++ b/document/src/vespa/document/util/serializableexceptions.h @@ -24,12 +24,4 @@ public: VESPA_DEFINE_EXCEPTION_SPINE(DeserializeException) }; -class SerializeException : public vespalib::IoException { -public: - SerializeException(const vespalib::string& msg, const vespalib::string& location = ""); - SerializeException(const vespalib::string& msg, const vespalib::Exception& cause, - const vespalib::string& location = ""); - VESPA_DEFINE_EXCEPTION_SPINE(SerializeException) -}; - } diff --git a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.h b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.h index 79f7d7c0ccc..5582c0ea153 100644 --- a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.h +++ b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.h @@ -20,7 +20,6 @@ namespace documentapi { class LoadTypeSet; class RoutingPolicyRepository; class RoutableRepository; -class SystemState; class IRoutingPolicyFactory; class IRoutableFactory; diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/queryresultmessage.cpp b/documentapi/src/vespa/documentapi/messagebus/messages/queryresultmessage.cpp index 411a7237cb5..08d631dc44b 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/queryresultmessage.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/messages/queryresultmessage.cpp @@ -4,11 +4,7 @@ namespace documentapi { -QueryResultMessage::QueryResultMessage() : - VisitorMessage(), - _searchResult(), - _summary() -{} +QueryResultMessage::QueryResultMessage() = default; QueryResultMessage::QueryResultMessage(const vdslib::SearchResult & result, const vdslib::DocumentSummary & summary) : VisitorMessage(), @@ -16,7 +12,7 @@ QueryResultMessage::QueryResultMessage(const vdslib::SearchResult & result, cons _summary(summary) {} -QueryResultMessage::~QueryResultMessage() {} +QueryResultMessage::~QueryResultMessage() = default; DocumentReply::UP QueryResultMessage::doCreateReply() const diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/queryresultmessage.h b/documentapi/src/vespa/documentapi/messagebus/messages/queryresultmessage.h index 239ce6fefd5..6324e2664e4 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/queryresultmessage.h +++ b/documentapi/src/vespa/documentapi/messagebus/messages/queryresultmessage.h @@ -25,7 +25,7 @@ public: * Constructs a new search result message for deserialization. */ QueryResultMessage(); - ~QueryResultMessage(); + ~QueryResultMessage() override; /** * Constructs a new search result message for the given search result. diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/visitor.cpp b/documentapi/src/vespa/documentapi/messagebus/messages/visitor.cpp index c891d9a316d..102a2308768 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/visitor.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/messages/visitor.cpp @@ -1,9 +1,11 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "visitor.h" -#include #include #include +#include +#include +#include using document::FixedBucketSpaces; @@ -79,7 +81,7 @@ DestroyVisitorMessage::~DestroyVisitorMessage() = default; DocumentReply::UP DestroyVisitorMessage::doCreateReply() const { - return DocumentReply::UP(new DocumentReply(DocumentProtocol::REPLY_DESTROYVISITOR)); + return std::make_unique(DocumentProtocol::REPLY_DESTROYVISITOR); } uint32_t @@ -115,11 +117,7 @@ VisitorInfoMessage::getType() const return DocumentProtocol::MESSAGE_VISITORINFO; } -MapVisitorMessage::MapVisitorMessage() : - _data() -{ - // empty -} +MapVisitorMessage::MapVisitorMessage() = default; uint32_t MapVisitorMessage::getApproxSize() const @@ -138,31 +136,17 @@ uint32_t MapVisitorMessage::getType() const return DocumentProtocol::MESSAGE_MAPVISITOR; } -DocumentListMessage::Entry::Entry() -{ - // empty -} +DocumentListMessage::Entry::Entry() = default; -DocumentListMessage::Entry::Entry(int64_t timestamp, - document::Document::SP doc, - bool removeEntry) : +DocumentListMessage::Entry::Entry(int64_t timestamp, document::Document::SP doc, bool removeEntry) : _timestamp(timestamp), _document(std::move(doc)), _removeEntry(removeEntry) -{ - // empty -} +{ } -DocumentListMessage::Entry::Entry(const Entry& other) : - _timestamp(other._timestamp), - _document(other._document), - _removeEntry(other._removeEntry) -{ - // empty -} +DocumentListMessage::Entry::Entry(const Entry& other) = default; -DocumentListMessage::Entry::Entry(const document::DocumentTypeRepo &repo, - document::ByteBuffer& buf) +DocumentListMessage::Entry::Entry(const document::DocumentTypeRepo &repo, document::ByteBuffer& buf) { buf.getLongNetwork(_timestamp); vespalib::nbostream stream(buf.getBufferAtPos(), buf.getRemaining()); @@ -174,26 +158,15 @@ DocumentListMessage::Entry::Entry(const document::DocumentTypeRepo &repo, } void -DocumentListMessage::Entry::serialize(document::ByteBuffer& buf) const +DocumentListMessage::Entry::serialize(vespalib::GrowableByteBuffer& buf) const { - buf.putLongNetwork(_timestamp); - _document->serialize(buf); + buf.putLong(_timestamp); + vespalib::nbostream nbo = _document->serialize(); + buf.putBytes(nbo.c_str(), nbo.size()); buf.putByte(_removeEntry ? 1 : 0); } -uint32_t -DocumentListMessage::Entry::getSerializedSize() const -{ - return sizeof(int64_t) + sizeof(uint8_t) - + _document->getSerializedSize(); -} - -DocumentListMessage::DocumentListMessage() : - _bucketId(), - _documents() -{ - // empty -} +DocumentListMessage::DocumentListMessage() = default; DocumentListMessage::DocumentListMessage(document::BucketId bid) : _bucketId(bid), diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/visitor.h b/documentapi/src/vespa/documentapi/messagebus/messages/visitor.h index b18a4e985f3..f47fa48bd80 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/visitor.h +++ b/documentapi/src/vespa/documentapi/messagebus/messages/visitor.h @@ -52,7 +52,7 @@ public: const string& instanceId, const string& controlDestination, const string& dataDestination); - ~CreateVisitorMessage(); + ~CreateVisitorMessage() override; const string& getLibraryName() const { return _libName; } void setLibraryName(const string& value) { _libName = value; } @@ -250,7 +250,7 @@ public: const document::Document::SP& getDocument() { return _document; } bool isRemoveEntry() { return _removeEntry; } - void serialize(document::ByteBuffer& buf) const; + void serialize(vespalib::GrowableByteBuffer& buf) const; uint32_t getSerializedSize() const; private: int64_t _timestamp; diff --git a/documentapi/src/vespa/documentapi/messagebus/routablefactories60.cpp b/documentapi/src/vespa/documentapi/messagebus/routablefactories60.cpp index 1f950ae166c..2fe9ffa3da9 100644 --- a/documentapi/src/vespa/documentapi/messagebus/routablefactories60.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/routablefactories60.cpp @@ -20,7 +20,7 @@ namespace documentapi { bool RoutableFactories60::DocumentMessageFactory::encode(const mbus::Routable &obj, vespalib::GrowableByteBuffer &out) const { - const DocumentMessage &msg = static_cast(obj); + const auto &msg = static_cast(obj); out.putByte(msg.getPriority()); out.putInt(msg.getLoadType().getId()); return doEncode(msg, out); @@ -93,7 +93,7 @@ RoutableFactories60::CreateVisitorMessageFactory::doDecode(document::ByteBuffer msg->setVisitRemoves(decodeBoolean(buf)); msg->setFieldSet(decodeString(buf)); msg->setVisitInconsistentBuckets(decodeBoolean(buf)); - msg->getParameters().deserialize(_repo, buf); + msg->getParameters().deserialize(buf); msg->setVisitorDispatcherVersion(50); decodeInt(buf); // Unused legacy visitor ordering msg->setMaxBucketsPerVisitor(decodeInt(buf)); @@ -105,7 +105,7 @@ RoutableFactories60::CreateVisitorMessageFactory::doDecode(document::ByteBuffer bool RoutableFactories60::CreateVisitorMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const { - const CreateVisitorMessage &msg = static_cast(obj); + const auto &msg = static_cast(obj); buf.putString(msg.getLibraryName()); buf.putString(msg.getInstanceId()); @@ -126,10 +126,8 @@ RoutableFactories60::CreateVisitorMessageFactory::doEncode(const DocumentMessage buf.putString(msg.getFieldSet()); buf.putBoolean(msg.visitInconsistentBuckets()); - int len = msg.getParameters().getSerializedSize(); - char *tmp = buf.allocate(len); - document::ByteBuffer dbuf(tmp, len); - msg.getParameters().serialize(dbuf); + + msg.getParameters().serialize(buf); buf.putInt(0); // Unused legacy visitor ordering buf.putInt(msg.getMaxBucketsPerVisitor()); @@ -184,7 +182,7 @@ RoutableFactories60::CreateVisitorReplyFactory::doDecode(document::ByteBuffer &b bool RoutableFactories60::CreateVisitorReplyFactory::doEncode(const DocumentReply &obj, vespalib::GrowableByteBuffer &buf) const { - const CreateVisitorReply &reply = static_cast(obj); + const auto &reply = static_cast(obj); buf.putLong(reply.getLastBucket().getRawId()); buf.putInt(reply.getVisitorStatistics().getBucketsVisited()); buf.putLong(reply.getVisitorStatistics().getDocumentsVisited()); @@ -209,19 +207,14 @@ RoutableFactories60::DestroyVisitorReplyFactory::doEncode(const DocumentReply &, } DocumentReply::UP -RoutableFactories60::DocumentIgnoredReplyFactory::doDecode(document::ByteBuffer& buf) const +RoutableFactories60::DocumentIgnoredReplyFactory::doDecode(document::ByteBuffer& ) const { - (void) buf; - return DocumentReply::UP(new DocumentIgnoredReply()); + return std::make_unique(); } bool -RoutableFactories60::DocumentIgnoredReplyFactory::doEncode( - const DocumentReply& obj, - vespalib::GrowableByteBuffer& buf) const +RoutableFactories60::DocumentIgnoredReplyFactory::doEncode(const DocumentReply&, vespalib::GrowableByteBuffer& ) const { - (void) obj; - (void) buf; return true; } @@ -243,15 +236,12 @@ RoutableFactories60::DocumentListMessageFactory::doDecode(document::ByteBuffer & bool RoutableFactories60::DocumentListMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const { - const DocumentListMessage &msg = static_cast(obj); + const auto &msg = static_cast(obj); buf.putLong(msg.getBucketId().getRawId()); buf.putInt(msg.getDocuments().size()); for (const auto & document : msg.getDocuments()) { - int len = document.getSerializedSize(); - char *tmp = buf.allocate(len); - document::ByteBuffer dbuf(tmp, len); - document.serialize(dbuf); + document.serialize(buf); } return true; @@ -283,11 +273,7 @@ bool RoutableFactories60::DocumentSummaryMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const { const DocumentSummaryMessage &msg = static_cast(obj); - - int32_t len = msg.getSerializedSize(); - char *tmp = buf.allocate(len); - document::ByteBuffer dbuf(tmp, len); - msg.serialize(dbuf); + msg.serialize(buf); return true; } @@ -322,7 +308,7 @@ RoutableFactories60::EmptyBucketsMessageFactory::doDecode(document::ByteBuffer & bool RoutableFactories60::EmptyBucketsMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const { - const EmptyBucketsMessage &msg = static_cast(obj); + const auto &msg = static_cast(obj); buf.putInt(msg.getBucketIds().size()); for (const auto & bucketId : msg.getBucketIds()) { @@ -367,7 +353,7 @@ RoutableFactories60::GetBucketListMessageFactory::doDecode(document::ByteBuffer bool RoutableFactories60::GetBucketListMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const { - const GetBucketListMessage &msg = static_cast(obj); + const auto &msg = static_cast(obj); buf.putLong(msg.getBucketId().getRawId()); return encodeBucketSpace(msg.getBucketSpace(), buf); } @@ -392,7 +378,7 @@ RoutableFactories60::GetBucketListReplyFactory::doDecode(document::ByteBuffer &b bool RoutableFactories60::GetBucketListReplyFactory::doEncode(const DocumentReply &obj, vespalib::GrowableByteBuffer &buf) const { - const GetBucketListReply &reply = static_cast(obj); + const auto &reply = static_cast(obj); const std::vector &buckets = reply.getBuckets(); buf.putInt(buckets.size()); @@ -417,7 +403,7 @@ RoutableFactories60::GetBucketStateMessageFactory::doDecode(document::ByteBuffer bool RoutableFactories60::GetBucketStateMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const { - const GetBucketStateMessage &msg = static_cast(obj); + const auto &msg = static_cast(obj); buf.putLong(msg.getBucketId().getRawId()); return true; } @@ -511,7 +497,7 @@ DocumentMessage::UP RoutableFactories60::MapVisitorMessageFactory::doDecode(document::ByteBuffer &buf) const { auto msg = std::make_unique(); - msg->getData().deserialize(_repo, buf); + msg->getData().deserialize(buf); return msg; } @@ -519,11 +505,7 @@ bool RoutableFactories60::MapVisitorMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const { const MapVisitorMessage &msg = static_cast(obj); - - int32_t len = msg.getData().getSerializedSize(); - char *tmp = buf.allocate(len); - document::ByteBuffer dbuf(tmp, len); - msg.getData().serialize(dbuf); + msg.getData().serialize(buf); return true; } @@ -660,12 +642,8 @@ RoutableFactories60::SearchResultMessageFactory::doDecode(document::ByteBuffer & bool RoutableFactories60::SearchResultMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const { - const SearchResultMessage &msg = static_cast(obj); - - int len = msg.getSerializedSize(); - char *tmp = buf.allocate(len); - document::ByteBuffer dbuf(tmp, len); - msg.serialize(dbuf); + const auto & msg = static_cast(obj); + msg.serialize(buf); return true; } @@ -683,13 +661,10 @@ RoutableFactories60::QueryResultMessageFactory::doDecode(document::ByteBuffer &b bool RoutableFactories60::QueryResultMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const { - const QueryResultMessage &msg = static_cast(obj); + const auto &msg = static_cast(obj); - int len = msg.getSearchResult().getSerializedSize() + msg.getDocumentSummary().getSerializedSize(); - char *tmp = buf.allocate(len); - document::ByteBuffer dbuf(tmp, len); - msg.getSearchResult().serialize(dbuf); - msg.getDocumentSummary().serialize(dbuf); + msg.getSearchResult().serialize(buf); + msg.getDocumentSummary().serialize(buf); return true; } @@ -744,7 +719,7 @@ RoutableFactories60::StatBucketMessageFactory::doDecode(document::ByteBuffer &bu bool RoutableFactories60::StatBucketMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const { - const StatBucketMessage &msg = static_cast(obj); + const auto &msg = static_cast(obj); buf.putLong(msg.getBucketId().getRawId()); buf.putString(msg.getDocumentSelection()); @@ -762,7 +737,7 @@ RoutableFactories60::StatBucketReplyFactory::doDecode(document::ByteBuffer &buf) bool RoutableFactories60::StatBucketReplyFactory::doEncode(const DocumentReply &obj, vespalib::GrowableByteBuffer &buf) const { - const StatBucketReply &reply = static_cast(obj); + const auto &reply = static_cast(obj); buf.putString(reply.getResults()); return true; } @@ -802,7 +777,7 @@ RoutableFactories60::UpdateDocumentMessageFactory::decodeInto(UpdateDocumentMess bool RoutableFactories60::UpdateDocumentMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const { - const UpdateDocumentMessage &msg = static_cast(obj); + const auto &msg = static_cast(obj); vespalib::nbostream stream; msg.getDocumentUpdate().serializeHEAD(stream); @@ -826,7 +801,7 @@ RoutableFactories60::UpdateDocumentReplyFactory::doDecode(document::ByteBuffer & bool RoutableFactories60::UpdateDocumentReplyFactory::doEncode(const DocumentReply &obj, vespalib::GrowableByteBuffer &buf) const { - const UpdateDocumentReply &reply = static_cast(obj); + const auto &reply = static_cast(obj); buf.putBoolean(reply.getWasFound()); buf.putLong(reply.getHighestModificationTimestamp()); return true; @@ -852,7 +827,7 @@ RoutableFactories60::VisitorInfoMessageFactory::doDecode(document::ByteBuffer &b bool RoutableFactories60::VisitorInfoMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const { - const VisitorInfoMessage &msg = static_cast(obj); + const auto &msg = static_cast(obj); buf.putInt(msg.getFinishedBuckets().size()); for (const auto & bucketId : msg.getFinishedBuckets()) { @@ -887,7 +862,7 @@ RoutableFactories60::WrongDistributionReplyFactory::doDecode(document::ByteBuffe bool RoutableFactories60::WrongDistributionReplyFactory::doEncode(const DocumentReply &obj, vespalib::GrowableByteBuffer &buf) const { - const WrongDistributionReply &reply = static_cast(obj); + const auto &reply = static_cast(obj); buf.putString(reply.getSystemState()); return true; } diff --git a/searchlib/src/vespa/searchlib/tensor/generic_tensor_store.cpp b/searchlib/src/vespa/searchlib/tensor/generic_tensor_store.cpp index 4e522f27ce2..ff41396f66b 100644 --- a/searchlib/src/vespa/searchlib/tensor/generic_tensor_store.cpp +++ b/searchlib/src/vespa/searchlib/tensor/generic_tensor_store.cpp @@ -3,7 +3,6 @@ #include "generic_tensor_store.h" #include #include -#include #include #include #include @@ -15,9 +14,7 @@ using search::datastore::Handle; using vespalib::tensor::Tensor; using vespalib::tensor::TypedBinaryFormat; -namespace search { - -namespace tensor { +namespace search::tensor { constexpr size_t MIN_BUFFER_ARRAYS = 1024; @@ -118,6 +115,4 @@ GenericTensorStore::setTensor(const Tensor &tensor) return raw.ref; } -} // namespace search::tensor - -} // namespace search +} diff --git a/staging_vespalib/src/vespa/vespalib/util/growablebytebuffer.cpp b/staging_vespalib/src/vespa/vespalib/util/growablebytebuffer.cpp index 57e88873e81..a10cad70579 100644 --- a/staging_vespalib/src/vespa/vespalib/util/growablebytebuffer.cpp +++ b/staging_vespalib/src/vespa/vespalib/util/growablebytebuffer.cpp @@ -27,7 +27,7 @@ GrowableByteBuffer::allocate(uint32_t len) } void -GrowableByteBuffer::putBytes(const char* buffer, uint32_t length) +GrowableByteBuffer::putBytes(const void * buffer, uint32_t length) { char* buf = allocate(length); memcpy(buf, buffer, length); diff --git a/staging_vespalib/src/vespa/vespalib/util/growablebytebuffer.h b/staging_vespalib/src/vespa/vespalib/util/growablebytebuffer.h index d32afeeabee..afbde04fb9b 100644 --- a/staging_vespalib/src/vespa/vespalib/util/growablebytebuffer.h +++ b/staging_vespalib/src/vespa/vespalib/util/growablebytebuffer.h @@ -45,7 +45,7 @@ public: /** Adds the given buffer to this buffer. */ - void putBytes(const char* buffer, uint32_t length); + void putBytes(const void * buffer, uint32_t length); /** Adds a short to the buffer. diff --git a/storage/src/tests/common/message_sender_stub.cpp b/storage/src/tests/common/message_sender_stub.cpp index c127f9071e5..f4e64e07955 100644 --- a/storage/src/tests/common/message_sender_stub.cpp +++ b/storage/src/tests/common/message_sender_stub.cpp @@ -22,9 +22,7 @@ MessageSenderStub::getLastCommand(bool verbose) const } std::string -MessageSenderStub::dumpMessage(const api::StorageMessage& msg, - bool includeAddress, - bool verbose) const +MessageSenderStub::dumpMessage(const api::StorageMessage& msg, bool includeAddress, bool verbose) const { std::ostringstream ost; @@ -38,7 +36,7 @@ MessageSenderStub::dumpMessage(const api::StorageMessage& msg, ost << " => " << msg.getAddress()->getIndex(); } if (verbose && msg.getType().isReply()) { - ost << " " << dynamic_cast(msg).getResult(); + ost << " " << dynamic_cast(msg).getResult().toString(); } return ost.str(); @@ -67,9 +65,7 @@ MessageSenderStub::getLastReply(bool verbose) const throw std::logic_error("Expected reply where there was none"); } - return dumpMessage(*replies.back(), - true, - verbose); + return dumpMessage(*replies.back(),true, verbose); } diff --git a/storage/src/tests/visiting/visitormanagertest.cpp b/storage/src/tests/visiting/visitormanagertest.cpp index 3f111bb53f7..2544fcc9a4b 100644 --- a/storage/src/tests/visiting/visitormanagertest.cpp +++ b/storage/src/tests/visiting/visitormanagertest.cpp @@ -653,7 +653,7 @@ TEST_F(VisitorManagerTest, visitor_cleanup) { if (api::ReturnCode::ILLEGAL_PARAMETERS == reply->getResult().getResult()) { failures++; } else { - std::cerr << reply->getResult() << "\n"; + std::cerr << reply->getResult().toString() << "\n"; } } else { if (api::ReturnCode::BUSY == reply->getResult().getResult()) { diff --git a/storage/src/vespa/storage/common/bucketmessages.cpp b/storage/src/vespa/storage/common/bucketmessages.cpp index e92e2d4c3bf..c80b7bbb17a 100644 --- a/storage/src/vespa/storage/common/bucketmessages.cpp +++ b/storage/src/vespa/storage/common/bucketmessages.cpp @@ -14,7 +14,7 @@ ReadBucketList::ReadBucketList(BucketSpace bucketSpace, spi::PartitionId partiti _partition(partition) { } -ReadBucketList::~ReadBucketList() { } +ReadBucketList::~ReadBucketList() = default; document::Bucket ReadBucketList::getBucket() const @@ -38,7 +38,7 @@ ReadBucketListReply::ReadBucketListReply(const ReadBucketList& cmd) _partition(cmd.getPartition()) { } -ReadBucketListReply::~ReadBucketListReply() { } +ReadBucketListReply::~ReadBucketListReply() = default; document::Bucket ReadBucketListReply::getBucket() const @@ -66,7 +66,7 @@ ReadBucketInfo::ReadBucketInfo(const document::Bucket &bucket) _bucket(bucket) { } -ReadBucketInfo::~ReadBucketInfo() { } +ReadBucketInfo::~ReadBucketInfo() = default; void ReadBucketInfo::print(std::ostream& out, bool verbose, const std::string& indent) const @@ -92,7 +92,7 @@ ReadBucketInfoReply::ReadBucketInfoReply(const ReadBucketInfo& cmd) _bucket(cmd.getBucket()) { } -ReadBucketInfoReply::~ReadBucketInfoReply() { } +ReadBucketInfoReply::~ReadBucketInfoReply() = default; void ReadBucketInfoReply::print(std::ostream& out, bool verbose, const std::string& indent) const { out << "ReadBucketInfoReply()"; @@ -117,7 +117,7 @@ RepairBucketCommand::RepairBucketCommand(const document::Bucket &bucket, uint16_ setPriority(LOW); } -RepairBucketCommand::~RepairBucketCommand() { } +RepairBucketCommand::~RepairBucketCommand() = default; void RepairBucketCommand::print(std::ostream& out, bool verbose, const std::string& indent) const { @@ -153,7 +153,7 @@ RepairBucketReply::RepairBucketReply(const RepairBucketCommand& cmd, const api:: _altered(false) { } -RepairBucketReply::~RepairBucketReply() { } +RepairBucketReply::~RepairBucketReply() = default; void RepairBucketReply::print(std::ostream& out, bool verbose, const std::string& indent) const { @@ -180,7 +180,7 @@ BucketDiskMoveCommand::BucketDiskMoveCommand(const document::Bucket &bucket, setPriority(LOW); } -BucketDiskMoveCommand::~BucketDiskMoveCommand() { } +BucketDiskMoveCommand::~BucketDiskMoveCommand() = default; void BucketDiskMoveCommand::setBucketId(const document::BucketId& id) @@ -208,14 +208,14 @@ BucketDiskMoveReply::BucketDiskMoveReply(const BucketDiskMoveCommand& cmd, _dstDisk(cmd.getDstDisk()) { } -BucketDiskMoveReply::~BucketDiskMoveReply() { } +BucketDiskMoveReply::~BucketDiskMoveReply() = default; void BucketDiskMoveReply::print(std::ostream& out, bool, const std::string&) const { out << "BucketDiskMoveReply(" << _bucket.getBucketId() << ", source " << _srcDisk << ", target " << _dstDisk << ", " << _bucketInfo << ", " - << getResult() << ")"; + << getResult().toString() << ")"; } std::unique_ptr @@ -236,7 +236,7 @@ InternalBucketJoinCommand::InternalBucketJoinCommand(const document::Bucket &buc // them higher than getting more bucket info lists. } -InternalBucketJoinCommand::~InternalBucketJoinCommand() { } +InternalBucketJoinCommand::~InternalBucketJoinCommand() = default; void InternalBucketJoinCommand::print(std::ostream& out, bool verbose, const std::string& indent) const { @@ -255,7 +255,7 @@ InternalBucketJoinReply::InternalBucketJoinReply(const InternalBucketJoinCommand _bucketInfo(info) { } -InternalBucketJoinReply::~InternalBucketJoinReply() { } +InternalBucketJoinReply::~InternalBucketJoinReply() = default; void InternalBucketJoinReply::print(std::ostream& out, bool verbose, const std::string& indent) const diff --git a/storage/src/vespa/storage/common/storagelink.cpp b/storage/src/vespa/storage/common/storagelink.cpp index 065f0b0b750..d9612eb48a0 100644 --- a/storage/src/vespa/storage/common/storagelink.cpp +++ b/storage/src/vespa/storage/common/storagelink.cpp @@ -142,7 +142,7 @@ void StorageLink::sendDown(const StorageMessage::SP& msg) sendUp(reply); } } else { - ost << " Return code: " << static_cast(*msg).getResult(); + ost << " Return code: " << static_cast(*msg).getResult().toString(); LOGBP(warning, "%s", ost.str().c_str()); } } else if (!_down->onDown(msg)) { @@ -182,7 +182,7 @@ void StorageLink::sendUp(const shared_ptr & msg) sendDown(reply); } } else { - ost << " Return code: " << static_cast(*msg).getResult(); + ost << " Return code: " << static_cast(*msg).getResult().toString(); LOGBP(warning, "%s", ost.str().c_str()); } } else if (!_up->onUp(msg)) { diff --git a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp index 8e6494a588d..c19168ca6e1 100644 --- a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp @@ -8,8 +8,7 @@ #include LOG_SETUP(".distributor.callback.statbucket"); -namespace storage { -namespace distributor { +namespace storage::distributor { StatBucketOperation::StatBucketOperation( [[maybe_unused]] DistributorComponent& manager, @@ -21,7 +20,7 @@ StatBucketOperation::StatBucketOperation( { } -StatBucketOperation::~StatBucketOperation() {} +StatBucketOperation::~StatBucketOperation() = default; void StatBucketOperation::onClose(DistributorMessageSender& sender) @@ -36,8 +35,7 @@ StatBucketOperation::onStart(DistributorMessageSender& sender) { std::vector nodes; - BucketDatabase::Entry entry( - _bucketSpace.getBucketDatabase().get(_command->getBucketId())); + BucketDatabase::Entry entry(_bucketSpace.getBucketDatabase().get(_command->getBucketId())); if (entry.valid()) { nodes = entry->getNodes(); @@ -83,7 +81,7 @@ StatBucketOperation::onReceive(DistributorMessageSender& sender, const std::shar if (myreply.getResult().getResult() == api::ReturnCode::OK) { ost << "\tBucket information from node " << found->second << ":\n" << myreply.getResults() << "\n\n"; } else { - ost << "\tBucket information retrieval failed on node " << found->second << ": " << myreply.getResult() << "\n\n"; + ost << "\tBucket information retrieval failed on node " << found->second << ": " << myreply.getResult().toString() << "\n\n"; } _results[found->second] = ost.str(); @@ -103,5 +101,4 @@ StatBucketOperation::onReceive(DistributorMessageSender& sender, const std::shar } } -} // distributor -} // storage +} diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.h b/storage/src/vespa/storage/persistence/filestorage/filestormanager.h index bfc89a70a85..65d4035a3dd 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.h +++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.h @@ -36,11 +36,8 @@ namespace api { class StorageReply; } -class BucketMergeTest; -class DiskInfo; struct FileStorManagerTest; class ReadBucketList; -class ModifiedBucketCheckerThread; class BucketOwnershipNotifier; class AbortBucketOperationsCommand; diff --git a/storage/src/vespa/storage/visiting/recoveryvisitor.cpp b/storage/src/vespa/storage/visiting/recoveryvisitor.cpp index f9e0fa17d66..8bac2bac10a 100644 --- a/storage/src/vespa/storage/visiting/recoveryvisitor.cpp +++ b/storage/src/vespa/storage/visiting/recoveryvisitor.cpp @@ -36,8 +36,7 @@ RecoveryVisitor::handleDocuments(const document::BucketId& bid, { vespalib::LockGuard guard(_mutex); - LOG(debug, "Visitor %s handling block of %zu documents.", - _id.c_str(), entries.size()); + LOG(debug, "Visitor %s handling block of %zu documents.", _id.c_str(), entries.size()); documentapi::DocumentListMessage* cmd = nullptr; diff --git a/storage/src/vespa/storage/visiting/recoveryvisitor.h b/storage/src/vespa/storage/visiting/recoveryvisitor.h index e68a8fdbc8c..1da2acfed9c 100644 --- a/storage/src/vespa/storage/visiting/recoveryvisitor.h +++ b/storage/src/vespa/storage/visiting/recoveryvisitor.h @@ -12,16 +12,13 @@ #include "visitor.h" #include -namespace documentapi { -class DocumentListMessage; -} +namespace documentapi { class DocumentListMessage; } namespace storage { class RecoveryVisitor : public Visitor { public: - RecoveryVisitor(StorageComponent&, - const vdslib::Parameters& params); + RecoveryVisitor(StorageComponent&, const vdslib::Parameters& params); private: void handleDocuments(const document::BucketId& bucketId, @@ -43,12 +40,11 @@ struct RecoveryVisitorFactory : public VisitorFactory { VisitorEnvironment::UP makeVisitorEnvironment(StorageComponent&) override { - return VisitorEnvironment::UP(new VisitorEnvironment); + return std::make_unique(); }; Visitor* - makeVisitor(StorageComponent& c, VisitorEnvironment&, - const vdslib::Parameters& params) override + makeVisitor(StorageComponent& c, VisitorEnvironment&, const vdslib::Parameters& params) override { return new RecoveryVisitor(c, params); } diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp index 4b213dff1d5..153d75fd50e 100644 --- a/storage/src/vespa/storage/visiting/visitor.cpp +++ b/storage/src/vespa/storage/visiting/visitor.cpp @@ -44,17 +44,12 @@ Visitor::HitCounter::addHit(const document::DocumentId& , uint32_t size) } void -Visitor::HitCounter::updateVisitorStatistics( - vdslib::VisitorStatistics& statistics) +Visitor::HitCounter::updateVisitorStatistics(vdslib::VisitorStatistics& statistics) { - statistics.setDocumentsReturned( - statistics.getDocumentsReturned() + _firstPassHits); - statistics.setBytesReturned( - statistics.getBytesReturned() + _firstPassBytes); - statistics.setSecondPassDocumentsReturned( - statistics.getSecondPassDocumentsReturned() + _secondPassHits); - statistics.setSecondPassBytesReturned( - statistics.getSecondPassBytesReturned() + _secondPassBytes); + statistics.setDocumentsReturned(statistics.getDocumentsReturned() + _firstPassHits); + statistics.setBytesReturned(statistics.getBytesReturned() + _firstPassBytes); + statistics.setSecondPassDocumentsReturned(statistics.getSecondPassDocumentsReturned() + _secondPassHits); + statistics.setSecondPassBytesReturned(statistics.getSecondPassBytesReturned() + _secondPassBytes); } Visitor::VisitorTarget::MessageMeta::MessageMeta( @@ -68,8 +63,7 @@ Visitor::VisitorTarget::MessageMeta::MessageMeta( { } -Visitor::VisitorTarget::MessageMeta::MessageMeta( - Visitor::VisitorTarget::MessageMeta&& rhs) noexcept +Visitor::VisitorTarget::MessageMeta::MessageMeta(Visitor::VisitorTarget::MessageMeta&& rhs) noexcept : messageId(rhs.messageId), retryCount(rhs.retryCount), memoryUsage(rhs.memoryUsage), @@ -78,9 +72,7 @@ Visitor::VisitorTarget::MessageMeta::MessageMeta( { } -Visitor::VisitorTarget::MessageMeta::~MessageMeta() -{ -} +Visitor::VisitorTarget::MessageMeta::~MessageMeta() = default; Visitor::VisitorTarget::MessageMeta& Visitor::VisitorTarget::MessageMeta::operator=( @@ -977,7 +969,7 @@ Visitor::getStatus(std::ostream& out, bool verbose) const << "\n"; out << "Current status" - << _result << "\n"; + << _result.toString() << "\n"; out << "Failed" << (failed() ? "true" : "false") << "\n"; diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.cpp b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.cpp index b90153c9517..0cfd2160497 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.cpp +++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.cpp @@ -24,8 +24,7 @@ ProtocolSerialization4_2::ProtocolSerialization4_2( { } -void ProtocolSerialization4_2::onEncode( - GBBuf& buf, const api::GetCommand& msg) const +void ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::GetCommand& msg) const { buf.putString(msg.getDocumentId().toString()); putBucket(msg.getBucket(), buf); @@ -41,14 +40,12 @@ ProtocolSerialization4_2::onDecodeGetCommand(BBuf& buf) const document::Bucket bucket = getBucket(buf); api::Timestamp beforeTimestamp(SH::getLong(buf)); bool headerOnly(SH::getBoolean(buf)); - api::GetCommand::UP msg( - new api::GetCommand(bucket, did, headerOnly ? "[header]" : "[all]", beforeTimestamp)); + auto msg = std::make_unique(bucket, did, headerOnly ? "[header]" : "[all]", beforeTimestamp); onDecodeCommand(buf, *msg); return msg; } -void ProtocolSerialization4_2::onEncode( - GBBuf& buf, const api::RemoveCommand& msg) const +void ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::RemoveCommand& msg) const { buf.putString(msg.getDocumentId().toString()); putBucket(msg.getBucket(), buf); @@ -62,13 +59,12 @@ ProtocolSerialization4_2::onDecodeRemoveCommand(BBuf& buf) const document::DocumentId did(SH::getString(buf)); document::Bucket bucket = getBucket(buf); api::Timestamp timestamp(SH::getLong(buf)); - api::RemoveCommand::UP msg(new api::RemoveCommand(bucket, did, timestamp)); + auto msg = std::make_unique(bucket, did, timestamp); onDecodeBucketInfoCommand(buf, *msg); return msg; } -void ProtocolSerialization4_2::onEncode( - GBBuf& buf, const api::RevertCommand& msg) const +void ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::RevertCommand& msg) const { putBucket(msg.getBucket(), buf); buf.putInt(msg.getRevertTokens().size()); @@ -86,13 +82,12 @@ ProtocolSerialization4_2::onDecodeRevertCommand(BBuf& buf) const for (uint32_t i=0, n=tokens.size(); i(bucket, tokens); onDecodeBucketInfoCommand(buf, *msg); return msg; } -void ProtocolSerialization4_2::onEncode( - GBBuf& buf, const api::CreateBucketCommand& msg) const +void ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::CreateBucketCommand& msg) const { putBucket(msg.getBucket(), buf); onEncodeBucketInfoCommand(buf, msg); @@ -102,13 +97,12 @@ api::StorageCommand::UP ProtocolSerialization4_2::onDecodeCreateBucketCommand(BBuf& buf) const { document::Bucket bucket = getBucket(buf); - api::CreateBucketCommand::UP msg(new api::CreateBucketCommand(bucket)); + auto msg = std::make_unique(bucket); onDecodeBucketInfoCommand(buf, *msg); return msg; } -void ProtocolSerialization4_2::onEncode( - GBBuf& buf, const api::MergeBucketCommand& msg) const +void ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::MergeBucketCommand& msg) const { putBucket(msg.getBucket(), buf); const std::vector& nodes(msg.getNodes()); @@ -135,14 +129,12 @@ ProtocolSerialization4_2::onDecodeMergeBucketCommand(BBuf& buf) const nodes.push_back(Node(index, sourceOnly)); } api::Timestamp timestamp(SH::getLong(buf)); - api::MergeBucketCommand::UP msg( - new api::MergeBucketCommand(bucket, nodes, timestamp)); + auto msg = std::make_unique(bucket, nodes, timestamp); onDecodeCommand(buf, *msg); return msg; } -void ProtocolSerialization4_2::onEncode( - GBBuf& buf, const api::GetBucketDiffCommand& msg) const +void ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::GetBucketDiffCommand& msg) const { putBucket(msg.getBucket(), buf); const std::vector& nodes(msg.getNodes()); @@ -174,8 +166,7 @@ ProtocolSerialization4_2::onDecodeGetBucketDiffCommand(BBuf& buf) const nodes.push_back(Node(index, sourceOnly)); } api::Timestamp timestamp = SH::getLong(buf); - api::GetBucketDiffCommand::UP msg( - new api::GetBucketDiffCommand(bucket, nodes, timestamp)); + auto msg = std::make_unique(bucket, nodes, timestamp); std::vector& entries(msg->getDiff()); uint32_t entryCount = SH::getInt(buf); if (entryCount > buf.getRemaining()) { @@ -190,8 +181,7 @@ ProtocolSerialization4_2::onDecodeGetBucketDiffCommand(BBuf& buf) const return msg; } -void ProtocolSerialization4_2::onEncode( - GBBuf& buf, const api::ApplyBucketDiffCommand& msg) const +void ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::ApplyBucketDiffCommand& msg) const { putBucket(msg.getBucket(), buf); const std::vector& nodes(msg.getNodes()); @@ -201,18 +191,15 @@ void ProtocolSerialization4_2::onEncode( buf.putBoolean(nodes[i].sourceOnly); } buf.putInt(msg.getMaxBufferSize()); - const std::vector& entries( - msg.getDiff()); + const std::vector& entries(msg.getDiff()); buf.putInt(entries.size()); for (uint32_t i=0; i(bucket, nodes, maxBufferSize); std::vector& entries(msg->getDiff()); uint32_t entryCount = SH::getInt(buf); if (entryCount > buf.getRemaining()) { @@ -248,15 +234,13 @@ ProtocolSerialization4_2::onDecodeApplyBucketDiffCommand(BBuf& buf) const buf.incPos(headerSize); } entries[i]._headerBlob.resize(headerSize); - buf.getBytes(&entries[i]._headerBlob[0], - entries[i]._headerBlob.size()); + buf.getBytes(&entries[i]._headerBlob[0], entries[i]._headerBlob.size()); uint32_t bodySize = SH::getInt(buf); if (bodySize > buf.getRemaining()) { buf.incPos(bodySize); } entries[i]._bodyBlob.resize(bodySize); - buf.getBytes(&entries[i]._bodyBlob[0], - entries[i]._bodyBlob.size()); + buf.getBytes(&entries[i]._bodyBlob[0], entries[i]._bodyBlob.size()); } onDecodeBucketInfoCommand(buf, *msg); return msg; @@ -274,11 +258,9 @@ ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::RequestBucketInfoReply } api::StorageReply::UP -ProtocolSerialization4_2::onDecodeRequestBucketInfoReply(const SCmd& cmd, - BBuf& buf) const +ProtocolSerialization4_2::onDecodeRequestBucketInfoReply(const SCmd& cmd, BBuf& buf) const { - api::RequestBucketInfoReply::UP msg(new api::RequestBucketInfoReply( - static_cast(cmd))); + auto msg = std::make_unique(static_cast(cmd)); api::RequestBucketInfoReply::EntryVector & entries(msg->getBucketInfo()); uint32_t entryCount = SH::getInt(buf); if (entryCount > buf.getRemaining()) { @@ -294,8 +276,7 @@ ProtocolSerialization4_2::onDecodeRequestBucketInfoReply(const SCmd& cmd, return msg; } -void ProtocolSerialization4_2::onEncode( - GBBuf& buf, const api::NotifyBucketChangeCommand& msg) const +void ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::NotifyBucketChangeCommand& msg) const { putBucket(msg.getBucket(), buf); putBucketInfo(msg.getBucketInfo(), buf); @@ -307,30 +288,25 @@ ProtocolSerialization4_2::onDecodeNotifyBucketChangeCommand(BBuf& buf) const { document::Bucket bucket = getBucket(buf); api::BucketInfo info(getBucketInfo(buf)); - api::NotifyBucketChangeCommand::UP msg( - new api::NotifyBucketChangeCommand(bucket, info)); + auto msg = std::make_unique(bucket, info); onDecodeCommand(buf, *msg); - return api::StorageCommand::UP(msg.release()); + return msg; } -void ProtocolSerialization4_2::onEncode( - GBBuf& buf, const api::NotifyBucketChangeReply& msg) const +void ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::NotifyBucketChangeReply& msg) const { onEncodeReply(buf, msg); } api::StorageReply::UP -ProtocolSerialization4_2::onDecodeNotifyBucketChangeReply(const SCmd& cmd, - BBuf& buf) const +ProtocolSerialization4_2::onDecodeNotifyBucketChangeReply(const SCmd& cmd,BBuf& buf) const { - api::NotifyBucketChangeReply::UP msg(new api::NotifyBucketChangeReply( - static_cast(cmd))); + auto msg = std::make_unique(static_cast(cmd)); onDecodeReply(buf, *msg); return msg; } -void ProtocolSerialization4_2::onEncode( - GBBuf& buf, const api::SplitBucketCommand& msg) const +void ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::SplitBucketCommand& msg) const { putBucket(msg.getBucket(), buf); buf.putByte(msg.getMinSplitBits()); @@ -344,7 +320,7 @@ api::StorageCommand::UP ProtocolSerialization4_2::onDecodeSplitBucketCommand(BBuf& buf) const { document::Bucket bucket = getBucket(buf); - api::SplitBucketCommand::UP msg(new api::SplitBucketCommand(bucket)); + auto msg = std::make_unique(bucket); msg->setMinSplitBits(SH::getByte(buf)); msg->setMaxSplitBits(SH::getByte(buf)); msg->setMinByteSize(SH::getInt(buf)); @@ -353,30 +329,24 @@ ProtocolSerialization4_2::onDecodeSplitBucketCommand(BBuf& buf) const return msg; } -void ProtocolSerialization4_2::onEncode( - GBBuf&, const api::SetBucketStateCommand&) const +void ProtocolSerialization4_2::onEncode(GBBuf&, const api::SetBucketStateCommand&) const { - throw vespalib::IllegalStateException("Unsupported serialization", - VESPA_STRLOC); + throw vespalib::IllegalStateException("Unsupported serialization", VESPA_STRLOC); } api::StorageCommand::UP ProtocolSerialization4_2::onDecodeSetBucketStateCommand(BBuf&) const { - throw vespalib::IllegalStateException("Unsupported deserialization", - VESPA_STRLOC); + throw vespalib::IllegalStateException("Unsupported deserialization", VESPA_STRLOC); } -void ProtocolSerialization4_2::onEncode( - GBBuf&, const api::SetBucketStateReply&) const +void ProtocolSerialization4_2::onEncode(GBBuf&, const api::SetBucketStateReply&) const { - throw vespalib::IllegalStateException("Unsupported serialization", - VESPA_STRLOC); + throw vespalib::IllegalStateException("Unsupported serialization", VESPA_STRLOC); } api::StorageReply::UP -ProtocolSerialization4_2::onDecodeSetBucketStateReply(const SCmd&, - BBuf&) const +ProtocolSerialization4_2::onDecodeSetBucketStateReply(const SCmd&, BBuf&) const { throw vespalib::IllegalStateException("Unsupported deserialization", VESPA_STRLOC); } @@ -404,11 +374,7 @@ ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::CreateVisitorCommand& buf.putBoolean(msg.getFieldSet() == "[header]"); buf.putBoolean(msg.visitInconsistentBuckets()); buf.putInt(vespalib::count_ms(msg.getQueueTimeout())); - - uint32_t size = msg.getParameters().getSerializedSize(); - char* docBuffer = buf.allocate(size); - document::ByteBuffer bbuf(docBuffer, size); - msg.getParameters().serialize(bbuf); + msg.getParameters().serialize(buf); onEncodeCommand(buf, msg); } @@ -420,8 +386,7 @@ ProtocolSerialization4_2::onDecodeCreateVisitorCommand(BBuf& buf) const vespalib::stringref libraryName = SH::getString(buf); vespalib::stringref instanceId = SH::getString(buf); vespalib::stringref selection = SH::getString(buf); - api::CreateVisitorCommand::UP msg( - new api::CreateVisitorCommand(bucketSpace, libraryName, instanceId, selection)); + auto msg = std::make_unique(bucketSpace, libraryName, instanceId, selection); msg->setVisitorCmdId(SH::getInt(buf)); msg->setControlDestination(SH::getString(buf)); msg->setDataDestination(SH::getString(buf)); @@ -450,7 +415,7 @@ ProtocolSerialization4_2::onDecodeCreateVisitorCommand(BBuf& buf) const msg->setVisitInconsistentBuckets(); } msg->setQueueTimeout(std::chrono::milliseconds(SH::getInt(buf))); - msg->getParameters().deserialize(getTypeRepo(), buf); + msg->getParameters().deserialize(buf); onDecodeCommand(buf, *msg); msg->setVisitorDispatcherVersion(42); @@ -458,8 +423,7 @@ ProtocolSerialization4_2::onDecodeCreateVisitorCommand(BBuf& buf) const } void -ProtocolSerialization4_2::onEncode( - GBBuf& buf, const api::DestroyVisitorCommand& msg) const +ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::DestroyVisitorCommand& msg) const { buf.putString(msg.getInstanceId()); onEncodeCommand(buf, msg); @@ -469,7 +433,7 @@ api::StorageCommand::UP ProtocolSerialization4_2::onDecodeDestroyVisitorCommand(BBuf& buf) const { vespalib::stringref instanceId = SH::getString(buf); - api::DestroyVisitorCommand::UP msg(new api::DestroyVisitorCommand(instanceId)); + auto msg = std::make_unique(instanceId); onDecodeCommand(buf, *msg); return msg; } @@ -483,7 +447,7 @@ ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::DestroyVisitorReply& m api::StorageReply::UP ProtocolSerialization4_2::onDecodeDestroyVisitorReply(const SCmd& cmd, BBuf& buf) const { - api::DestroyVisitorReply::UP msg(new api::DestroyVisitorReply(static_cast(cmd))); + auto msg = std::make_unique(static_cast(cmd)); onDecodeReply(buf, *msg); return msg; } @@ -502,8 +466,7 @@ ProtocolSerialization4_2::onDecodeRemoveLocationCommand(BBuf& buf) const vespalib::stringref documentSelection = SH::getString(buf); document::Bucket bucket = getBucket(buf); - api::RemoveLocationCommand::UP msg; - msg.reset(new api::RemoveLocationCommand(documentSelection, bucket)); + auto msg = std::make_unique(documentSelection, bucket); onDecodeCommand(buf, *msg); return msg; } @@ -517,7 +480,7 @@ ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::RemoveLocationReply& m api::StorageReply::UP ProtocolSerialization4_2::onDecodeRemoveLocationReply(const SCmd& cmd, BBuf& buf) const { - api::RemoveLocationReply::UP msg(new api::RemoveLocationReply(static_cast(cmd))); + auto msg = std::make_unique(static_cast(cmd)); onDecodeBucketInfoReply(buf, *msg); return msg; } @@ -525,15 +488,13 @@ ProtocolSerialization4_2::onDecodeRemoveLocationReply(const SCmd& cmd, BBuf& buf // Utility functions for serialization void -ProtocolSerialization4_2::onEncodeBucketInfoCommand( - GBBuf& buf, const api::BucketInfoCommand& msg) const +ProtocolSerialization4_2::onEncodeBucketInfoCommand(GBBuf& buf, const api::BucketInfoCommand& msg) const { onEncodeCommand(buf, msg); } void -ProtocolSerialization4_2::onDecodeBucketInfoCommand( - BBuf& buf, api::BucketInfoCommand& msg) const +ProtocolSerialization4_2::onDecodeBucketInfoCommand(BBuf& buf, api::BucketInfoCommand& msg) const { onDecodeCommand(buf, msg); } @@ -547,8 +508,7 @@ ProtocolSerialization4_2::onEncode(GBBuf& buf, const api::ReturnCode& rc) const } void -ProtocolSerialization4_2::onEncodeDiffEntry( - GBBuf& buf, const api::GetBucketDiffCommand::Entry& entry) const +ProtocolSerialization4_2::onEncodeDiffEntry(GBBuf& buf, const api::GetBucketDiffCommand::Entry& entry) const { buf.putLong(entry._timestamp); SH::putGlobalId(entry._gid, buf); @@ -559,8 +519,7 @@ ProtocolSerialization4_2::onEncodeDiffEntry( } void -ProtocolSerialization4_2::onDecodeDiffEntry( - BBuf& buf, api::GetBucketDiffCommand::Entry& entry) const +ProtocolSerialization4_2::onDecodeDiffEntry(BBuf& buf, api::GetBucketDiffCommand::Entry& entry) const { entry._timestamp = SH::getLong(buf); entry._gid = SH::getGlobalId(buf); diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_1.cpp b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_1.cpp index b0a1685ed8c..0b1f66127ba 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_1.cpp +++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_1.cpp @@ -62,32 +62,26 @@ ProtocolSerialization5_1::onDecodeSetBucketStateCommand(BBuf& buf) const { document::Bucket bucket = getBucket(buf); api::SetBucketStateCommand::BUCKET_STATE state( - static_cast( - SH::getByte(buf))); - api::SetBucketStateCommand::UP msg( - new api::SetBucketStateCommand(bucket, state)); + static_cast(SH::getByte(buf))); + auto msg = std::make_unique(bucket, state); onDecodeCommand(buf, *msg); - return api::StorageCommand::UP(msg.release()); + return msg; } -void ProtocolSerialization5_1::onEncode( - GBBuf& buf, const api::SetBucketStateReply& msg) const +void ProtocolSerialization5_1::onEncode(GBBuf& buf, const api::SetBucketStateReply& msg) const { onEncodeBucketReply(buf, msg); } api::StorageReply::UP -ProtocolSerialization5_1::onDecodeSetBucketStateReply(const SCmd& cmd, - BBuf& buf) const +ProtocolSerialization5_1::onDecodeSetBucketStateReply(const SCmd& cmd, BBuf& buf) const { - api::SetBucketStateReply::UP msg(new api::SetBucketStateReply( - static_cast(cmd))); + auto msg = std::make_unique(static_cast(cmd)); onDecodeBucketReply(buf, *msg); - return api::StorageReply::UP(msg.release()); + return msg; } -void ProtocolSerialization5_1::onEncode( - GBBuf& buf, const api::GetCommand& msg) const +void ProtocolSerialization5_1::onEncode(GBBuf& buf, const api::GetCommand& msg) const { buf.putString(msg.getDocumentId().toString()); putBucket(msg.getBucket(), buf); @@ -103,15 +97,13 @@ ProtocolSerialization5_1::onDecodeGetCommand(BBuf& buf) const document::Bucket bucket = getBucket(buf); api::Timestamp beforeTimestamp(SH::getLong(buf)); std::string fieldSet(SH::getString(buf)); - api::GetCommand::UP msg( - new api::GetCommand(bucket, did, fieldSet, beforeTimestamp)); + auto msg = std::make_unique(bucket, did, fieldSet, beforeTimestamp); onDecodeCommand(buf, *msg); - return api::StorageCommand::UP(msg.release()); + return msg; } void -ProtocolSerialization5_1::onEncode( - GBBuf& buf, const api::CreateVisitorCommand& msg) const +ProtocolSerialization5_1::onEncode(GBBuf& buf, const api::CreateVisitorCommand& msg) const { putBucketSpace(msg.getBucketSpace(), buf); buf.putString(msg.getLibraryName()); @@ -133,11 +125,7 @@ ProtocolSerialization5_1::onEncode( buf.putString(msg.getFieldSet()); buf.putBoolean(msg.visitInconsistentBuckets()); buf.putInt(vespalib::count_ms(msg.getQueueTimeout())); - - uint32_t size = msg.getParameters().getSerializedSize(); - char* docBuffer = buf.allocate(size); - document::ByteBuffer bbuf(docBuffer, size); - msg.getParameters().serialize(bbuf); + msg.getParameters().serialize(buf); onEncodeCommand(buf, msg); @@ -152,8 +140,7 @@ ProtocolSerialization5_1::onDecodeCreateVisitorCommand(BBuf& buf) const vespalib::stringref libraryName = SH::getString(buf); vespalib::stringref instanceId = SH::getString(buf); vespalib::stringref selection = SH::getString(buf); - api::CreateVisitorCommand::UP msg( - new api::CreateVisitorCommand(bucketSpace, libraryName, instanceId, selection)); + auto msg = std::make_unique(bucketSpace, libraryName, instanceId, selection); msg->setVisitorCmdId(SH::getInt(buf)); msg->setControlDestination(SH::getString(buf)); msg->setDataDestination(SH::getString(buf)); @@ -182,17 +169,16 @@ ProtocolSerialization5_1::onDecodeCreateVisitorCommand(BBuf& buf) const msg->setVisitInconsistentBuckets(); } msg->setQueueTimeout(std::chrono::milliseconds(SH::getInt(buf))); - msg->getParameters().deserialize(getTypeRepo(), buf); + msg->getParameters().deserialize(buf); onDecodeCommand(buf, *msg); SH::getInt(buf); // Unused msg->setMaxBucketsPerVisitor(SH::getInt(buf)); msg->setVisitorDispatcherVersion(50); - return api::StorageCommand::UP(msg.release()); + return msg; } -void ProtocolSerialization5_1::onEncode( - GBBuf& buf, const api::CreateBucketCommand& msg) const +void ProtocolSerialization5_1::onEncode(GBBuf& buf, const api::CreateBucketCommand& msg) const { putBucket(msg.getBucket(), buf); buf.putBoolean(msg.getActive()); @@ -204,10 +190,10 @@ ProtocolSerialization5_1::onDecodeCreateBucketCommand(BBuf& buf) const { document::Bucket bucket = getBucket(buf); bool setActive = SH::getBoolean(buf); - api::CreateBucketCommand::UP msg(new api::CreateBucketCommand(bucket)); + auto msg = std::make_unique(bucket); msg->setActive(setActive); onDecodeBucketInfoCommand(buf, *msg); - return api::StorageCommand::UP(msg.release()); + return msg; } } diff --git a/storageapi/src/vespa/storageapi/mbusprot/storagereply.cpp b/storageapi/src/vespa/storageapi/mbusprot/storagereply.cpp index 469c6a41bc7..596af6a79d4 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/storagereply.cpp +++ b/storageapi/src/vespa/storageapi/mbusprot/storagereply.cpp @@ -3,6 +3,8 @@ #include "storagereply.h" #include "storagecommand.h" #include +#include + using vespalib::alloc::Alloc; using vespalib::IllegalStateException; @@ -17,8 +19,8 @@ StorageReply::StorageReply(mbus::BlobRef data, const ProtocolSerialization& seri _reply() { memcpy(_buffer.get(), data.data(), _sz); - document::ByteBuffer buf(data.data(), _sz); - buf.getIntNetwork(reinterpret_cast(_mbusType)); + vespalib::nbostream nbo(data.data(), _sz); + nbo >> _mbusType; } StorageReply::StorageReply(api::StorageReply::SP reply) diff --git a/storageapi/src/vespa/storageapi/message/bucket.h b/storageapi/src/vespa/storageapi/message/bucket.h index f6185d9f8e7..ec1df33a285 100644 --- a/storageapi/src/vespa/storageapi/message/bucket.h +++ b/storageapi/src/vespa/storageapi/message/bucket.h @@ -14,12 +14,12 @@ #include #include #include +#include #include namespace document { class DocumentTypeRepo; } -namespace storage { -namespace api { +namespace storage::api { /** * @class CreateBucketCommand @@ -482,5 +482,4 @@ public: DECLARE_STORAGEREPLY(SetBucketStateReply, onSetBucketStateReply) }; -} // api -} // storage +} diff --git a/storageapi/src/vespa/storageapi/message/visitor.cpp b/storageapi/src/vespa/storageapi/message/visitor.cpp index aeb58f30fb4..f398b4c8146 100644 --- a/storageapi/src/vespa/storageapi/message/visitor.cpp +++ b/storageapi/src/vespa/storageapi/message/visitor.cpp @@ -65,7 +65,7 @@ CreateVisitorCommand::CreateVisitorCommand(const CreateVisitorCommand& o) { } -CreateVisitorCommand::~CreateVisitorCommand() {} +CreateVisitorCommand::~CreateVisitorCommand() = default; document::Bucket CreateVisitorCommand::getBucket() const @@ -141,8 +141,7 @@ DestroyVisitorCommand::DestroyVisitorCommand(vespalib::stringref instanceId) } void -DestroyVisitorCommand::print(std::ostream& out, bool verbose, - const std::string& indent) const +DestroyVisitorCommand::print(std::ostream& out, bool verbose, const std::string& indent) const { out << "DestroyVisitorCommand(" << _instanceId << ")"; if (verbose) { @@ -157,8 +156,7 @@ DestroyVisitorReply::DestroyVisitorReply(const DestroyVisitorCommand& cmd) } void -DestroyVisitorReply::print(std::ostream& out, bool verbose, - const std::string& indent) const +DestroyVisitorReply::print(std::ostream& out, bool verbose, const std::string& indent) const { out << "DestroyVisitorReply()"; if (verbose) { @@ -175,17 +173,15 @@ VisitorInfoCommand::VisitorInfoCommand() { } -VisitorInfoCommand::~VisitorInfoCommand() { -} +VisitorInfoCommand::~VisitorInfoCommand() = default; void -VisitorInfoCommand::print(std::ostream& out, bool verbose, - const std::string& indent) const +VisitorInfoCommand::print(std::ostream& out, bool verbose, const std::string& indent) const { out << "VisitorInfoCommand("; if (_completed) { out << "completed"; } if (_error.failed()) { - out << _error; + out << _error.toString(); } if (verbose) { out << ") : "; @@ -205,8 +201,7 @@ VisitorInfoReply::VisitorInfoReply(const VisitorInfoCommand& cmd) } void -VisitorInfoReply::print(std::ostream& out, bool verbose, - const std::string& indent) const +VisitorInfoReply::print(std::ostream& out, bool verbose, const std::string& indent) const { out << "VisitorInfoReply("; if (_completed) { out << "completed"; } diff --git a/storageapi/src/vespa/storageapi/messageapi/returncode.cpp b/storageapi/src/vespa/storageapi/messageapi/returncode.cpp index 68fbca75393..9b497a297a1 100644 --- a/storageapi/src/vespa/storageapi/messageapi/returncode.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/returncode.cpp @@ -3,8 +3,7 @@ #include "returncode.h" #include -namespace storage { -namespace api { +namespace storage::api { ReturnCode::ReturnCode() : _result(OK), @@ -14,60 +13,22 @@ ReturnCode::ReturnCode() ReturnCode::ReturnCode(const ReturnCode &) = default; ReturnCode & ReturnCode::operator = (const ReturnCode &) = default; ReturnCode & ReturnCode::operator = (ReturnCode &&) = default; -ReturnCode::~ReturnCode() {} +ReturnCode::~ReturnCode() = default; ReturnCode::ReturnCode(Result result, vespalib::stringref msg) : _result(result), _message(msg) {} -ReturnCode::ReturnCode(const document::DocumentTypeRepo &repo, - document::ByteBuffer& buffer) - : _result(OK), - _message() -{ - deserialize(repo, buffer); -} - -void ReturnCode:: -onDeserialize(const document::DocumentTypeRepo &, document::ByteBuffer& buffer) -{ - int32_t result; - buffer.getInt(result); - _result = static_cast(result); - int32_t size; - buffer.getInt(size); - const char * p = buffer.getBufferAtPos(); - buffer.incPos(size); - _message.assign(p, size); -} - -void ReturnCode::onSerialize(document::ByteBuffer& buffer) const -{ - buffer.putInt(_result); - buffer.putInt(_message.size()); - buffer.putBytes(_message.c_str(), _message.size()); -} - -size_t ReturnCode::getSerializedSize() const -{ - return 2 * sizeof(int32_t) + _message.size(); -} - -void -ReturnCode::print(std::ostream& out, bool verbose, - const std::string& indent) const -{ - (void) verbose; (void) indent; - out << "ReturnCode(" << ReturnCode::getResultString(getResult()); - if (getMessage().size() > 0) out << ", " << getMessage(); - out << ")"; -} - vespalib::string ReturnCode::getResultString(Result result) { return documentapi::DocumentProtocol::getErrorName(result); } +vespalib::string +ReturnCode::toString() const { + return getResultString(_result) + " : " + _message; +} + bool ReturnCode::isBusy() const { @@ -173,5 +134,4 @@ ReturnCode::isBucketDisappearance() const } } -} // api -} // storage +} diff --git a/storageapi/src/vespa/storageapi/messageapi/returncode.h b/storageapi/src/vespa/storageapi/messageapi/returncode.h index ccd95a81aa3..58392e545a9 100644 --- a/storageapi/src/vespa/storageapi/messageapi/returncode.h +++ b/storageapi/src/vespa/storageapi/messageapi/returncode.h @@ -10,20 +10,12 @@ #pragma once -#include -#include #include -#include -namespace document { - class ByteBuffer; -} -namespace storage { -namespace api { +namespace storage::api { -class ReturnCode : public document::Deserializable, - public vespalib::Printable { +class ReturnCode { public: typedef documentapi::DocumentProtocol Protocol; @@ -68,31 +60,20 @@ public: private: Result _result; vespalib::string _message; - void onDeserialize(const document::DocumentTypeRepo &repo, document::ByteBuffer& buffer) override; - void onSerialize(document::ByteBuffer& buffer) const override; - public: ReturnCode(); explicit ReturnCode(Result result, vespalib::stringref msg = ""); - ReturnCode(const document::DocumentTypeRepo &repo, - document::ByteBuffer& buffer); ReturnCode(const ReturnCode &); ReturnCode & operator = (const ReturnCode &); ReturnCode(ReturnCode &&) = default; ReturnCode & operator = (ReturnCode &&); ~ReturnCode(); - ReturnCode* clone() const override { return new ReturnCode(*this); } - - size_t getSerializedSize() const override; - const vespalib::string& getMessage() const { return _message; } void setMessage(vespalib::stringref message) { _message = message; } Result getResult() const { return _result; } - void print(std::ostream& out, bool verbose, const std::string& indent) const override; - /** * Translate from status code to human-readable string * @param result Status code returned from getResult() @@ -121,8 +102,7 @@ public: bool isShutdownRelated() const; bool isBucketDisappearance() const; bool isNonCriticalForIntegrityChecker() const; + vespalib::string toString() const; }; - -} // api -} // storage +} diff --git a/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp b/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp index 86f7cb6e16d..1033b016629 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp @@ -4,8 +4,7 @@ #include "storagecommand.h" #include -namespace storage { -namespace api { +namespace storage::api { StorageReply::StorageReply(const StorageCommand& cmd, ReturnCode code) : StorageMessage(cmd.getType().getReplyType(), cmd.getMsgId()), @@ -19,16 +18,14 @@ StorageReply::StorageReply(const StorageCommand& cmd, ReturnCode code) setTransportContext(cmd.getTransportContext()); } -StorageReply::~StorageReply() { } +StorageReply::~StorageReply() = default; void StorageReply::print(std::ostream& out, bool verbose, const std::string& indent) const { (void) verbose; (void) indent; - out << "StorageReply(" << _type.getName() << ", " - << _result.toString() << ")"; + out << "StorageReply(" << _type.getName() << ", " << _result.toString() << ")"; } -} // api -} // storage +} diff --git a/storageapi/src/vespa/storageapi/messageapi/storagereply.h b/storageapi/src/vespa/storageapi/messageapi/storagereply.h index 4219f3e28cd..1a3bbe35eb4 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagereply.h +++ b/storageapi/src/vespa/storageapi/messageapi/storagereply.h @@ -15,8 +15,7 @@ #include "returncode.h" #include "storagemessage.h" -namespace storage { -namespace api { +namespace storage::api { class StorageCommand; @@ -28,7 +27,7 @@ protected: ReturnCode code = ReturnCode(ReturnCode::OK)); public: - ~StorageReply(); + ~StorageReply() override; DECLARE_POINTER_TYPEDEFS(StorageReply); void setResult(const ReturnCode& r) { _result = r; } @@ -37,6 +36,4 @@ public: void print(std::ostream& out, bool verbose, const std::string& indent) const override; }; -} // api -} // storage - +} diff --git a/vdslib/src/tests/container/parameterstest.cpp b/vdslib/src/tests/container/parameterstest.cpp index c54d8ae66da..95a29fb97be 100644 --- a/vdslib/src/tests/container/parameterstest.cpp +++ b/vdslib/src/tests/container/parameterstest.cpp @@ -1,10 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include #include +#include +#include #include -using document::DocumentTypeRepo; +using vespalib::GrowableByteBuffer; +using document::ByteBuffer; using namespace vdslib; TEST(ParametersTest, test_parameters) @@ -15,11 +17,12 @@ TEST(ParametersTest, test_parameters) par.set("number", 6); par.set("int64_t", INT64_C(8589934590)); par.set("double", 0.25); - std::unique_ptr buffer(par.serialize()); - buffer->flip(); - DocumentTypeRepo repo; - Parameters par2(repo, *buffer); + GrowableByteBuffer buffer; + par.serialize(buffer); + + ByteBuffer bBuf(buffer.getBuffer(), buffer.position()); + Parameters par2(bBuf); EXPECT_EQ(vespalib::stringref("overture"), par2.get("fast")); EXPECT_EQ(vespalib::stringref("yahoo"), par2.get("overture")); @@ -35,4 +38,5 @@ TEST(ParametersTest, test_parameters) EXPECT_EQ(numberDefault, par2.get("nonexistingnumber", numberDefault)); EXPECT_EQ(int64Default, par2.get("nonexistingint64_t", int64Default)); EXPECT_EQ(doubleDefault, par2.get("nonexistingdouble", doubleDefault)); + } diff --git a/vdslib/src/vespa/vdslib/container/documentsummary.cpp b/vdslib/src/vespa/vdslib/container/documentsummary.cpp index bc8a0473ab3..f948bd64687 100644 --- a/vdslib/src/vespa/vdslib/container/documentsummary.cpp +++ b/vdslib/src/vespa/vdslib/container/documentsummary.cpp @@ -1,6 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "documentsummary.h" +#include +#include #include namespace vdslib { @@ -21,7 +23,7 @@ DocumentSummary::DocumentSummary(document::ByteBuffer& buf) : deserialize(buf); } -DocumentSummary::~DocumentSummary() {} +DocumentSummary::~DocumentSummary() = default; void DocumentSummary::deserialize(document::ByteBuffer& buf) { @@ -47,18 +49,18 @@ void DocumentSummary::deserialize(document::ByteBuffer& buf) } } -void DocumentSummary::serialize(document::ByteBuffer& buf) const +void DocumentSummary::serialize(vespalib::GrowableByteBuffer& buf) const { - buf.putIntNetwork(0); // Just serialize dummy 4 byte field, to avoid versioning. - buf.putIntNetwork(_summary.size()); + buf.putInt(0); // Just serialize dummy 4 byte field, to avoid versioning. + buf.putInt(_summary.size()); if ( ! _summary.empty() ) { - buf.putIntNetwork(getSummarySize()); + buf.putInt(getSummarySize()); for (size_t i(0), m(_summary.size()); i < m; i++) { Summary s(_summary[i]); buf.putBytes(s.getDocId(_summaryBuffer->c_str()), s.getTotalSize()); } for (size_t i(0), m(_summary.size()); i < m; i++) { - buf.putIntNetwork(_summary[i].getSummarySize()); + buf.putInt(_summary[i].getSummarySize()); } } } diff --git a/vdslib/src/vespa/vdslib/container/documentsummary.h b/vdslib/src/vespa/vdslib/container/documentsummary.h index f04c1fa06bf..375546920ec 100644 --- a/vdslib/src/vespa/vdslib/container/documentsummary.h +++ b/vdslib/src/vespa/vdslib/container/documentsummary.h @@ -2,9 +2,10 @@ #pragma once #include -#include #include +namespace document { class ByteBuffer; } +namespace vespalib { class GrowableByteBuffer; } namespace vdslib { class DocumentSummary { @@ -29,7 +30,7 @@ public: void sort(); void deserialize(document::ByteBuffer& buf); - void serialize(document::ByteBuffer& buf) const; + void serialize(vespalib::GrowableByteBuffer& buf) const; uint32_t getSerializedSize() const; private: class Summary { diff --git a/vdslib/src/vespa/vdslib/container/parameters.cpp b/vdslib/src/vespa/vdslib/container/parameters.cpp index 4358c42ff29..9c843df1caa 100644 --- a/vdslib/src/vespa/vdslib/container/parameters.cpp +++ b/vdslib/src/vespa/vdslib/container/parameters.cpp @@ -1,20 +1,22 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "parameters.hpp" +#include #include #include #include #include +#include #include using namespace vdslib; Parameters::Parameters() = default; -Parameters::Parameters(const document::DocumentTypeRepo &repo, document::ByteBuffer& buffer) +Parameters::Parameters(document::ByteBuffer& buffer) : _parameters() { - deserialize(repo, buffer); + deserialize(buffer); } Parameters::~Parameters() = default; @@ -28,20 +30,19 @@ size_t Parameters::getSerializedSize() const return mysize; } -void Parameters::onSerialize(document::ByteBuffer& buffer) const +void Parameters::serialize(vespalib::GrowableByteBuffer& buffer) const { - buffer.putIntNetwork(_parameters.size()); + buffer.putInt(_parameters.size()); for (const auto & entry : _parameters) { - buffer.putIntNetwork(entry.first.size()); + buffer.putInt(entry.first.size()); buffer.putBytes(entry.first.c_str(), entry.first.size()); - buffer.putIntNetwork(entry.second.size()); + buffer.putInt(entry.second.size()); buffer.putBytes(entry.second.c_str(), entry.second.size()); } } -void Parameters::onDeserialize(const document::DocumentTypeRepo &repo, document::ByteBuffer& buffer) +void Parameters::deserialize(document::ByteBuffer& buffer) { - (void) repo; _parameters.clear(); int32_t mysize; buffer.getIntNetwork(mysize); @@ -88,11 +89,6 @@ Parameters::operator==(const Parameters &other) const return true; } -Parameters* Parameters::clone() const -{ - return new Parameters(*this); -} - vespalib::stringref Parameters::get(vespalib::stringref id, vespalib::stringref def) const { ParametersMap::const_iterator it = _parameters.find(id); @@ -130,7 +126,7 @@ void Parameters::print(std::ostream& out, bool verbose, const std::string& inden out << ")"; } -std::string Parameters::toString() const +vespalib::string Parameters::toString() const { vespalib::string ret; for (const auto & entry : _parameters) { diff --git a/vdslib/src/vespa/vdslib/container/parameters.h b/vdslib/src/vespa/vdslib/container/parameters.h index f3ea0543546..61649b29bbe 100644 --- a/vdslib/src/vespa/vdslib/container/parameters.h +++ b/vdslib/src/vespa/vdslib/container/parameters.h @@ -14,18 +14,15 @@ #pragma once -#include #include #include -namespace vespalib { - class asciistream; -} +namespace vespalib { class GrowableByteBuffer; } +namespace document { class ByteBuffer; } namespace vdslib { -class Parameters : public document::Deserializable, - public document::XmlSerializable { +class Parameters : public document::XmlSerializable { public: typedef vespalib::stringref KeyT; class Value : public vespalib::string @@ -42,27 +39,25 @@ public: private: ParametersMap _parameters; - void onSerialize(document::ByteBuffer& buffer) const override; - void onDeserialize(const document::DocumentTypeRepo &repo, document::ByteBuffer& buffer) override; void printXml(document::XmlOutputStream& xos) const override; public: Parameters(); - Parameters(const document::DocumentTypeRepo &repo, document::ByteBuffer& buffer); - virtual ~Parameters(); + Parameters(document::ByteBuffer& buffer); + ~Parameters(); bool operator==(const Parameters &other) const; - Parameters* clone() const override; + size_t getSerializedSize() const; - size_t getSerializedSize() const override; - - bool hasValue(KeyT id) const { return (_parameters.find(id) != _parameters.end()); } - unsigned int size() const { return _parameters.size(); } + bool hasValue(KeyT id) const { return (_parameters.find(id) != _parameters.end()); } + unsigned int size() const { return _parameters.size(); } bool lookup(KeyT id, ValueRef & v) const; void set(KeyT id, const void * v, size_t sz) { _parameters[id] = Value(v, sz); } void print(std::ostream& out, bool verbose, const std::string& indent) const; + void serialize(vespalib::GrowableByteBuffer& buffer) const; + void deserialize(document::ByteBuffer& buffer); // Disallow ParametersMap::const_iterator begin() const { return _parameters.begin(); } @@ -92,7 +87,7 @@ public: template T get(KeyT id, T def) const; - std::string toString() const; + vespalib::string toString() const; }; } // vdslib diff --git a/vdslib/src/vespa/vdslib/container/searchresult.cpp b/vdslib/src/vespa/vdslib/container/searchresult.cpp index 73b19a2f8a2..20cc53e2de9 100644 --- a/vdslib/src/vespa/vdslib/container/searchresult.cpp +++ b/vdslib/src/vespa/vdslib/container/searchresult.cpp @@ -1,6 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "searchresult.h" +#include +#include #include namespace vdslib { @@ -25,21 +27,21 @@ void AggregatorList::deserialize(document::ByteBuffer & buf) } } -void AggregatorList::serialize(document::ByteBuffer & buf) const +void AggregatorList::serialize(vespalib::GrowableByteBuffer & buf) const { - buf.putIntNetwork(size()); - for (const_iterator it(begin()), mt(end()); it != mt; it++) { - buf.putIntNetwork(it->first); - buf.putIntNetwork(it->second.size()); - buf.putBytes(it->second, it->second.size()); + buf.putInt(size()); + for (const auto & entry : *this) { + buf.putInt(entry.first); + buf.putInt(entry.second.size()); + buf.putBytes(entry.second, entry.second.size()); } } uint32_t AggregatorList::getSerializedSize() const { size_t sz(sizeof(uint32_t) * (1 + 2*size())); - for (const_iterator it(begin()), mt(end()); it != mt; it++) { - sz += it->second.size(); + for (const auto & entry : *this) { + sz += entry.second.size(); } return sz; } @@ -51,7 +53,7 @@ BlobContainer::BlobContainer(size_t reserve) : _offsets.push_back(0); } -BlobContainer::~BlobContainer() {} +BlobContainer::~BlobContainer() = default; size_t BlobContainer::append(const void * v, size_t sz) { @@ -84,11 +86,11 @@ void BlobContainer::deserialize(document::ByteBuffer & buf) buf.getBytes(_blob, getSize()); } -void BlobContainer::serialize(document::ByteBuffer & buf) const +void BlobContainer::serialize(vespalib::GrowableByteBuffer & buf) const { - buf.putIntNetwork(getCount()); + buf.putInt(getCount()); for(size_t i(0), m(getCount()); i < m; i++) { - buf.putIntNetwork(getSize(i)); + buf.putInt(getSize(i)); } buf.putBytes(_blob, getSize()); } @@ -116,7 +118,7 @@ SearchResult::SearchResult(document::ByteBuffer & buf) : deserialize(buf); } -SearchResult::~SearchResult() {} +SearchResult::~SearchResult() = default; void SearchResult::deserialize(document::ByteBuffer & buf) { @@ -143,26 +145,26 @@ void SearchResult::deserialize(document::ByteBuffer & buf) _groupingList.deserialize(buf); } -void SearchResult::serialize(document::ByteBuffer & buf) const +void SearchResult::serialize(vespalib::GrowableByteBuffer & buf) const { - buf.putIntNetwork(_totalHits); + buf.putInt(_totalHits); uint32_t hitCount = std::min(_hits.size(), _wantedHits); - buf.putIntNetwork(hitCount); + buf.putInt(hitCount); if (hitCount > 0) { uint32_t sz = getBufCount(); - buf.putIntNetwork(sz); + buf.putInt(sz); for (size_t i(0), m(hitCount); i < m; i++) { const char * s(_hits[i].getDocId(_docIdBuffer->c_str())); buf.putBytes(s, strlen(s)+1); } for (size_t i(0), m(hitCount); i < m; i++) { - buf.putDoubleNetwork(_hits[i].getRank()); + buf.putDouble(_hits[i].getRank()); } } uint32_t sortCount = std::min(_sortBlob.getCount(), _wantedHits); - buf.putIntNetwork(sortCount); + buf.putInt(sortCount); for (size_t i(0); i < sortCount; i++) { - buf.putIntNetwork(_sortBlob.getSize(_hits[i].getIndex())); + buf.putInt(_sortBlob.getSize(_hits[i].getIndex())); } for (size_t i(0); i < sortCount; i++) { size_t sz; diff --git a/vdslib/src/vespa/vdslib/container/searchresult.h b/vdslib/src/vespa/vdslib/container/searchresult.h index 081873e2989..fc893f6b5be 100644 --- a/vdslib/src/vespa/vdslib/container/searchresult.h +++ b/vdslib/src/vespa/vdslib/container/searchresult.h @@ -2,10 +2,11 @@ #pragma once #include -#include #include #include +namespace document { class ByteBuffer; } +namespace vespalib { class GrowableByteBuffer; } namespace vdslib { typedef std::map IntBlobMapT; @@ -15,7 +16,7 @@ class AggregatorList : public IntBlobMapT public: void add(size_t id, const vespalib::MallocPtr & aggrBlob); void deserialize(document::ByteBuffer & buf); - void serialize(document::ByteBuffer & buf) const; + void serialize(vespalib::GrowableByteBuffer & buf) const; uint32_t getSerializedSize() const; }; @@ -31,7 +32,7 @@ public: size_t getSize(size_t index) const { return _offsets[index+1] - _offsets[index]; } const void * getBuf(size_t index) const { return _blob.c_str() + _offsets[index]; } void deserialize(document::ByteBuffer & buf); - void serialize(document::ByteBuffer & buf) const; + void serialize(vespalib::GrowableByteBuffer & buf) const; uint32_t getSerializedSize() const { return (1 + getCount()) * sizeof(uint32_t) + getSize(); } private: typedef vespalib::MallocPtr Blob; @@ -76,7 +77,7 @@ public: void sort(); void deserialize(document::ByteBuffer & buf); - void serialize(document::ByteBuffer & buf) const; + void serialize(vespalib::GrowableByteBuffer & buf) const; uint32_t getSerializedSize() const; private: class Hit { -- cgit v1.2.3 From 1f9cb926b91659840e687f9cab0f508522d58690 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 20 Jan 2020 15:50:01 +0000 Subject: Make it known that getting serialized size will always be expensive. --- document/src/tests/documenttestcase.cpp | 38 ++++++---------- document/src/tests/documentupdatetestcase.cpp | 53 ++++++++++------------ document/src/tests/fieldpathupdatetestcase.cpp | 28 ++++++------ .../src/vespa/document/fieldvalue/document.cpp | 7 --- document/src/vespa/document/fieldvalue/document.h | 2 - .../src/vespa/document/update/documentupdate.cpp | 11 ++--- .../src/vespa/document/update/documentupdate.h | 6 +-- document/src/vespa/document/util/bytebuffer.cpp | 35 ++++---------- document/src/vespa/document/util/bytebuffer.h | 34 ++------------ .../messagebus/messages/documentstate.cpp | 16 +++---- .../documentapi/messagebus/routablefactories60.cpp | 17 +++---- .../conformancetest/conformancetest.cpp | 2 +- persistence/src/vespa/persistence/spi/docentry.cpp | 8 ++-- .../proton/persistenceengine/document_iterator.cpp | 2 +- .../tests/transactionlogstress/translogstress.cpp | 27 ++++++----- storage/src/tests/visiting/visitormanagertest.cpp | 3 +- .../src/vespa/storage/visiting/recoveryvisitor.cpp | 4 +- .../src/vespa/storageapi/message/persistence.cpp | 2 +- .../src/vespa/storageapi/messageapi/returncode.cpp | 9 +++- 19 files changed, 118 insertions(+), 186 deletions(-) (limited to 'storage') diff --git a/document/src/tests/documenttestcase.cpp b/document/src/tests/documenttestcase.cpp index a8d4829d355..4f769841d72 100644 --- a/document/src/tests/documenttestcase.cpp +++ b/document/src/tests/documenttestcase.cpp @@ -567,14 +567,14 @@ TEST(DocumentTest, testReadSerializedFile) int fd = open(TEST_PATH("data/serializejava.dat").c_str(), O_RDONLY); size_t len = lseek(fd,0,SEEK_END); - ByteBuffer buf(len); + vespalib::alloc::Alloc buf = vespalib::alloc::Alloc::alloc(len); lseek(fd,0,SEEK_SET); - if (read(fd, buf.getBuffer(), len) != (ssize_t)len) { + if (read(fd, buf.get(), len) != (ssize_t)len) { throw vespalib::Exception("read failed"); } close(fd); - nbostream stream(buf.getBufferAtPos(), len); + nbostream stream(buf.get(), len); Document doc(repo, stream); verifyJavaDocument(doc); @@ -586,7 +586,7 @@ TEST(DocumentTest, testReadSerializedFile) EXPECT_TRUE(buf2.empty()); buf2.rp(0); EXPECT_EQ(len, buf2.size()); - EXPECT_TRUE(memcmp(buf2.peek(), buf.getBuffer(), buf2.size()) == 0); + EXPECT_TRUE(memcmp(buf2.peek(), buf.get(), buf2.size()) == 0); doc2.setValue("stringfield", StringFieldValue("hei")); @@ -603,14 +603,14 @@ TEST(DocumentTest, testReadSerializedFileCompressed) int fd = open(TEST_PATH("data/serializejava-compressed.dat").c_str(), O_RDONLY); int len = lseek(fd,0,SEEK_END); - ByteBuffer buf(len); + vespalib::alloc::Alloc buf = vespalib::alloc::Alloc::alloc(len); lseek(fd,0,SEEK_SET); - if (read(fd, buf.getBuffer(), len) != len) { + if (read(fd, buf.get(), len) != len) { throw vespalib::Exception("read failed"); } close(fd); - nbostream stream(buf.getBufferAtPos(), len); + nbostream stream(buf.get(), len); Document doc(repo, stream); verifyJavaDocument(doc); } @@ -753,14 +753,14 @@ TEST(DocumentTest,testReadSerializedAllVersions) } int fd = open(tests[i]._dataFile.c_str(), O_RDONLY); int len = lseek(fd,0,SEEK_END); - ByteBuffer buf(len); + vespalib::alloc::Alloc buf = vespalib::alloc::Alloc::alloc(len); lseek(fd,0,SEEK_SET); - if (read(fd, buf.getBuffer(), len) != len) { - throw vespalib::Exception("read failed"); - } + if (read(fd, buf.get(), len) != len) { + throw vespalib::Exception("read failed"); + } close(fd); - nbostream stream(buf.getBufferAtPos(), len); + nbostream stream(buf.get(), len); Document doc(repo, stream); IntFieldValue intVal; @@ -1181,14 +1181,14 @@ TEST(DocumentTest, testAnnotationDeserialization) int fd = open(TEST_PATH("data/serializejavawithannotations.dat").c_str(), O_RDONLY); int len = lseek(fd,0,SEEK_END); - ByteBuffer buf(len); + vespalib::alloc::Alloc buf = vespalib::alloc::Alloc::alloc(len); lseek(fd,0,SEEK_SET); - if (read(fd, buf.getBuffer(), len) != len) { + if (read(fd, buf.get(), len) != len) { throw vespalib::Exception("read failed"); } close(fd); - nbostream stream1(buf.getBufferAtPos(), len); + nbostream stream1(buf.get(), len); Document doc(repo, stream1); StringFieldValue strVal; EXPECT_TRUE(doc.getValue(doc.getField("story"), strVal)); @@ -1228,14 +1228,6 @@ TEST(DocumentTest, testAnnotationDeserialization) EXPECT_EQ((int64_t)2384LL, longVal.getAsLong()); } -TEST(DocumentTest, testGetSerializedSize) -{ - TestDocMan testDocMan; - Document::UP doc = testDocMan.createDocument(); - - EXPECT_EQ(getSerializedSize(*doc), doc->getSerializedSize()); -} - TEST(DocumentTest, testDeserializeMultiple) { TestDocRepo testDocRepo; diff --git a/document/src/tests/documentupdatetestcase.cpp b/document/src/tests/documentupdatetestcase.cpp index 9ffe30c1080..a1ce861855c 100644 --- a/document/src/tests/documentupdatetestcase.cpp +++ b/document/src/tests/documentupdatetestcase.cpp @@ -43,14 +43,13 @@ namespace document { namespace { -ByteBuffer::UP serializeHEAD(const DocumentUpdate & update) +nbostream +serializeHEAD(const DocumentUpdate & update) { nbostream stream; VespaDocumentSerializer serializer(stream); serializer.writeHEAD(update); - ByteBuffer::UP retVal(new ByteBuffer(stream.size())); - retVal->putBytes(stream.peek(), stream.size()); - return retVal; + return stream; } nbostream serialize(const ValueUpdate & update) @@ -83,25 +82,26 @@ void testRoundtripSerialize(const UpdateType& update, const DataType &type) { } void -writeBufferToFile(const ByteBuffer &buf, const vespalib::string &fileName) +writeBufferToFile(const nbostream &buf, const vespalib::string &fileName) { auto file = std::fstream(fileName, std::ios::out | std::ios::binary); - file.write(buf.getBuffer(), buf.getPos()); + file.write(buf.c_str(), buf.size()); assert(file.good()); file.close(); } -ByteBuffer::UP +nbostream readBufferFromFile(const vespalib::string &fileName) { auto file = std::fstream(fileName, std::ios::in | std::ios::binary | std::ios::ate); auto size = file.tellg(); auto result = std::make_unique(size); file.seekg(0); - file.read(result->getBuffer(), size); + vespalib::alloc::Alloc buf = vespalib::alloc::Alloc::alloc(size); + file.read(static_cast(buf.get()), size); assert(file.good()); file.close(); - return result; + return nbostream(std::move(buf), size); } } @@ -132,9 +132,8 @@ TEST(DocumentUpdateTest, testSimpleUsage) // Test that a document update can be serialized DocumentUpdate docUpdate(repo, *docType, DocumentId("id:ns:test::1")); docUpdate.addUpdate(fieldUpdateCopy); - ByteBuffer::UP docBuf = serializeHEAD(docUpdate); - docBuf->flip(); - auto docUpdateCopy(DocumentUpdate::createHEAD(repo, nbostream(docBuf->getBufferAtPos(), docBuf->getRemaining()))); + nbostream docBuf = serializeHEAD(docUpdate); + auto docUpdateCopy(DocumentUpdate::createHEAD(repo, docBuf)); // Create a test document Document doc(*docType, DocumentId("id:ns:test::1")); @@ -236,7 +235,7 @@ TEST(DocumentUpdateTest, testUpdateArray) // Create a document. TestDocMan docMan; Document::UP doc(docMan.createDocument()); - EXPECT_EQ((document::FieldValue*)NULL, doc->getValue(doc->getField("tags")).get()); + EXPECT_EQ((document::FieldValue*)nullptr, doc->getValue(doc->getField("tags")).get()); // Assign array field. ArrayFieldValue myarray(doc->getType().getField("tags").getDataType()); @@ -459,8 +458,7 @@ TEST(DocumentUpdateTest, testReadSerializedFile) const std::string file_name = "data/crossplatform-java-cpp-doctypes.cfg"; DocumentTypeRepo repo(readDocumenttypesConfig(file_name)); - auto buf = readBufferFromFile("data/serializeupdatejava.dat"); - nbostream is(buf->getBufferAtPos(), buf->getRemaining()); + auto is = readBufferFromFile("data/serializeupdatejava.dat"); DocumentUpdate::UP updp(DocumentUpdate::createHEAD(repo, is)); DocumentUpdate& upd(*updp); @@ -539,8 +537,8 @@ TEST(DocumentUpdateTest, testGenerateSerializedFile) ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 2))) .addUpdate(MapValueUpdate(StringFieldValue("foo"), ArithmeticValueUpdate(ArithmeticValueUpdate::Mul, 2)))); - ByteBuffer::UP buf(serializeHEAD(upd)); - writeBufferToFile(*buf, "data/serializeupdatecpp.dat"); + nbostream buf(serializeHEAD(upd)); + writeBufferToFile(buf, "data/serializeupdatecpp.dat"); } @@ -549,7 +547,7 @@ TEST(DocumentUpdateTest, testSetBadFieldTypes) // Create a test document TestDocMan docMan; Document::UP doc(docMan.createDocument()); - EXPECT_EQ((document::FieldValue*)NULL, doc->getValue(doc->getField("headerval")).get()); + EXPECT_EQ((document::FieldValue*)nullptr, doc->getValue(doc->getField("headerval")).get()); // Assign a float value to an int field. DocumentUpdate update(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()); @@ -561,7 +559,7 @@ TEST(DocumentUpdateTest, testSetBadFieldTypes) update.applyTo(*doc); // Verify that the field is NOT set in the document. - EXPECT_EQ((document::FieldValue*)NULL, + EXPECT_EQ((document::FieldValue*)nullptr, doc->getValue(doc->getField("headerval")).get()); } @@ -569,7 +567,7 @@ TEST(DocumentUpdateTest, testUpdateApplyNoParams) { TestDocMan docMan; Document::UP doc(docMan.createDocument()); - EXPECT_EQ((document::FieldValue*)NULL, doc->getValue(doc->getField("tags")).get()); + EXPECT_EQ((document::FieldValue*)nullptr, doc->getValue(doc->getField("tags")).get()); DocumentUpdate update(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()); update.addUpdate(FieldUpdate(doc->getField("tags")).addUpdate(AssignValueUpdate())); @@ -1095,13 +1093,12 @@ struct TensorUpdateSerializeFixture { } void serializeUpdateToFile(const DocumentUpdate &update, const vespalib::string &fileName) { - ByteBuffer::UP buf = serializeHEAD(update); - writeBufferToFile(*buf, fileName); + nbostream buf = serializeHEAD(update); + writeBufferToFile(buf, fileName); } DocumentUpdate::UP deserializeUpdateFromFile(const vespalib::string &fileName) { - auto buf = readBufferFromFile(fileName); - nbostream stream(buf->getBufferAtPos(), buf->getRemaining()); + auto stream = readBufferFromFile(fileName); return DocumentUpdate::createHEAD(*repo, stream); } @@ -1181,10 +1178,9 @@ TEST(DocumentUpdateTest, testThatCreateIfNonExistentFlagIsSerializedAndDeseriali { CreateIfNonExistentFixture f; - ByteBuffer::UP buf(serializeHEAD(*f.update)); - buf->flip(); + nbostream buf(serializeHEAD(*f.update)); - DocumentUpdate::UP deserialized = DocumentUpdate::createHEAD(f.docMan.getTypeRepo(), *buf); + DocumentUpdate::UP deserialized = DocumentUpdate::createHEAD(f.docMan.getTypeRepo(), buf); EXPECT_EQ(*f.update, *deserialized); EXPECT_TRUE(deserialized->getCreateIfNonExistent()); } @@ -1216,9 +1212,8 @@ TEST(DocumentUpdateTest, array_element_update_can_be_roundtrip_serialized) ArrayUpdateFixture f; auto buffer = serializeHEAD(*f.update); - buffer->flip(); - auto deserialized = DocumentUpdate::createHEAD(f.doc_man.getTypeRepo(), *buffer); + auto deserialized = DocumentUpdate::createHEAD(f.doc_man.getTypeRepo(), buffer); EXPECT_EQ(*f.update, *deserialized); } diff --git a/document/src/tests/fieldpathupdatetestcase.cpp b/document/src/tests/fieldpathupdatetestcase.cpp index 82443f13716..39360119766 100644 --- a/document/src/tests/fieldpathupdatetestcase.cpp +++ b/document/src/tests/fieldpathupdatetestcase.cpp @@ -19,6 +19,7 @@ #include using vespalib::Identifiable; +using vespalib::nbostream; using namespace document::config_builder; namespace document { @@ -133,23 +134,21 @@ createTestDocument(const DocumentTypeRepo &repo) return doc; } -ByteBuffer::UP serializeHEAD(const DocumentUpdate & update) +nbostream +serializeHEAD(const DocumentUpdate & update) { vespalib::nbostream stream; VespaDocumentSerializer serializer(stream); serializer.writeHEAD(update); - ByteBuffer::UP retVal(new ByteBuffer(stream.size())); - retVal->putBytes(stream.peek(), stream.size()); - return retVal; + return stream; } void testSerialize(const DocumentTypeRepo& repo, const DocumentUpdate& a) { try{ - ByteBuffer::UP bb(serializeHEAD(a)); - bb->flip(); - DocumentUpdate::UP b(DocumentUpdate::createHEAD(repo, *bb)); + auto bb(serializeHEAD(a)); + DocumentUpdate::UP b(DocumentUpdate::createHEAD(repo, bb)); - EXPECT_EQ(size_t(0), bb->getRemaining()); + EXPECT_EQ(size_t(0), bb.size()); EXPECT_EQ(a.getId().toString(), b->getId().toString()); EXPECT_EQ(a.getUpdates().size(), b->getUpdates().size()); for (size_t i(0); i < a.getUpdates().size(); i++) { @@ -157,8 +156,7 @@ void testSerialize(const DocumentTypeRepo& repo, const DocumentUpdate& a) { const FieldUpdate & ub = b->getUpdates()[i]; EXPECT_EQ(&ua.getField(), &ub.getField()); - EXPECT_EQ(ua.getUpdates().size(), - ub.getUpdates().size()); + EXPECT_EQ(ua.getUpdates().size(), ub.getUpdates().size()); for (size_t j(0); j < ua.getUpdates().size(); j++) { EXPECT_EQ(ua.getUpdates()[j]->getType(), ub.getUpdates()[j]->getType()); } @@ -1073,14 +1071,14 @@ TEST_F(FieldPathUpdateTestCase, testReadSerializedFile) int fd = open(TEST_PATH("data/serialize-fieldpathupdate-java.dat").c_str(), O_RDONLY); int len = lseek(fd,0,SEEK_END); - ByteBuffer buf(len); + vespalib::alloc::Alloc buf = vespalib::alloc::Alloc::alloc(len); lseek(fd,0,SEEK_SET); - if (read(fd, buf.getBuffer(), len) != len) { + if (read(fd, buf.get(), len) != len) { throw vespalib::Exception("read failed"); } close(fd); - DocumentUpdate::UP updp(DocumentUpdate::createHEAD(repo, buf)); + DocumentUpdate::UP updp(DocumentUpdate::createHEAD(repo, nbostream(std::move(buf), len))); DocumentUpdate& upd(*updp); DocumentUpdate::UP compare(createDocumentUpdateForSerialization(repo)); @@ -1094,11 +1092,11 @@ TEST_F(FieldPathUpdateTestCase, testGenerateSerializedFile) // Tests nothing, only generates a file for java test DocumentUpdate::UP upd(createDocumentUpdateForSerialization(repo)); - ByteBuffer::UP buf(serializeHEAD(*upd)); + nbostream buf(serializeHEAD(*upd)); int fd = open(TEST_PATH("data/serialize-fieldpathupdate-cpp.dat").c_str(), O_WRONLY | O_TRUNC | O_CREAT, 0644); - if (write(fd, buf->getBuffer(), buf->getPos()) != (ssize_t)buf->getPos()) { + if (write(fd, buf.c_str(), buf.size()) != (ssize_t)buf.size()) { throw vespalib::Exception("write failed"); } close(fd); diff --git a/document/src/vespa/document/fieldvalue/document.cpp b/document/src/vespa/document/fieldvalue/document.cpp index b137f4bac2e..48059c7038d 100644 --- a/document/src/vespa/document/fieldvalue/document.cpp +++ b/document/src/vespa/document/fieldvalue/document.cpp @@ -256,13 +256,6 @@ void Document::deserializeBody(const DocumentTypeRepo& repo, vespalib::nbostream deserializer.readStructNoReset(getFields()); } -size_t -Document::getSerializedSize() const -{ - // Temporary non-optimal (but guaranteed correct) implementation. - return serialize().size(); -} - StructuredFieldValue::StructuredIterator::UP Document::getIterator(const Field* first) const { diff --git a/document/src/vespa/document/fieldvalue/document.h b/document/src/vespa/document/fieldvalue/document.h index 23b97a2a56d..6a111fe6782 100644 --- a/document/src/vespa/document/fieldvalue/document.h +++ b/document/src/vespa/document/fieldvalue/document.h @@ -96,8 +96,6 @@ public: /** Deserialize document contained in given bytebuffers. */ void deserialize(const DocumentTypeRepo& repo, vespalib::nbostream & body, vespalib::nbostream & header); - size_t getSerializedSize() const; - /** Undo fieldvalue's toXml override for document. */ std::string toXml() const { return toXml(""); } std::string toXml(const std::string& indent) const override; diff --git a/document/src/vespa/document/update/documentupdate.cpp b/document/src/vespa/document/update/documentupdate.cpp index b43b7a59c5b..f289e6a8f27 100644 --- a/document/src/vespa/document/update/documentupdate.cpp +++ b/document/src/vespa/document/update/documentupdate.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include @@ -231,20 +230,18 @@ DocumentUpdate::serializeFlags(int size_) const } DocumentUpdate::UP -DocumentUpdate::createHEAD(const DocumentTypeRepo& repo, ByteBuffer& buffer) +DocumentUpdate::createHEAD(const DocumentTypeRepo& repo, vespalib::nbostream && stream) { - vespalib::nbostream is(buffer.getBufferAtPos(), buffer.getRemaining()); auto update = std::make_unique(); - update->initHEAD(repo, is); - buffer.setPos(buffer.getPos() + is.rp()); + update->initHEAD(repo, std::move(stream)); return update; } DocumentUpdate::UP -DocumentUpdate::createHEAD(const DocumentTypeRepo& repo, vespalib::nbostream stream) +DocumentUpdate::createHEAD(const DocumentTypeRepo& repo, vespalib::nbostream & stream) { auto update = std::make_unique(); - update->initHEAD(repo, std::move(stream)); + update->initHEAD(repo, stream); return update; } diff --git a/document/src/vespa/document/update/documentupdate.h b/document/src/vespa/document/update/documentupdate.h index cb9113f5039..fb3d6c0f7a3 100644 --- a/document/src/vespa/document/update/documentupdate.h +++ b/document/src/vespa/document/update/documentupdate.h @@ -51,8 +51,8 @@ public: /** * Create new style document update, possibly with field path updates. */ - static DocumentUpdate::UP createHEAD(const DocumentTypeRepo & repo, vespalib::nbostream stream); - static DocumentUpdate::UP createHEAD(const DocumentTypeRepo & repo, ByteBuffer & buffer); + static DocumentUpdate::UP createHEAD(const DocumentTypeRepo & repo, vespalib::nbostream & stream); + static DocumentUpdate::UP createHEAD(const DocumentTypeRepo & repo, vespalib::nbostream && stream); DocumentUpdate(); /** @@ -125,8 +125,8 @@ private: bool _needHardReserialize; int deserializeFlags(int sizeAndFlags); - void initHEAD(const DocumentTypeRepo & repo, vespalib::nbostream && stream); void initHEAD(const DocumentTypeRepo & repo, vespalib::nbostream & stream); + void initHEAD(const DocumentTypeRepo & repo, vespalib::nbostream && stream); void deserializeBody(const DocumentTypeRepo &repo, vespalib::nbostream &stream); void lazyDeserialize(const DocumentTypeRepo & repo, vespalib::nbostream & stream); void ensureDeserialized() const; diff --git a/document/src/vespa/document/util/bytebuffer.cpp b/document/src/vespa/document/util/bytebuffer.cpp index c909ca5fe61..468f8d653ab 100644 --- a/document/src/vespa/document/util/bytebuffer.cpp +++ b/document/src/vespa/document/util/bytebuffer.cpp @@ -89,10 +89,11 @@ ByteBuffer::ByteBuffer(const ByteBuffer& rhs) : _ownedBuffer() { if (rhs._len > 0 && rhs._buffer) { - Alloc::alloc(rhs._len + 1).swap(_ownedBuffer); - _buffer = static_cast(_ownedBuffer.get()); - memcpy(_buffer, rhs._buffer, rhs._len); - _buffer[rhs._len] = 0; + Alloc buf = Alloc::alloc(rhs._len + 1); + memcpy(buf.get(), rhs._buffer, rhs._len); + static_cast(buf.get())[rhs._len] = 0; + _ownedBuffer = std::move(buf); + _buffer = static_cast(_ownedBuffer.get()); } } @@ -110,16 +111,6 @@ ByteBuffer* ByteBuffer::copyBuffer(const char* buffer, size_t len) } } -void -ByteBuffer::setPos(size_t pos) // throw (BufferOutOfBoundsException) -{ - if (pos > _len) { - throwOutOfBounds(pos, _len); - } else { - _pos=pos; - } -} - void ByteBuffer::incPos(size_t pos) { if (_pos + pos > _len) { @@ -133,7 +124,7 @@ void ByteBuffer::getNumeric(uint8_t & v) { if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { - v = *(uint8_t *) getBufferAtPos(); + v = *reinterpret_cast(getBufferAtPos()); incPosNoCheck(sizeof(v)); } } @@ -142,7 +133,7 @@ void ByteBuffer::getNumericNetwork(int16_t & v) { if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { - uint16_t val = *(uint16_t *) (void *) getBufferAtPos(); + uint16_t val = *reinterpret_cast(getBufferAtPos()); v = ntohs(val); incPosNoCheck(sizeof(v)); } @@ -152,7 +143,7 @@ void ByteBuffer::getNumericNetwork(int32_t & v) { if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { - uint32_t val = *(uint32_t *) (void *) getBufferAtPos(); + uint32_t val = *reinterpret_cast(getBufferAtPos()); v = ntohl(val); incPosNoCheck(sizeof(v)); } @@ -162,7 +153,7 @@ void ByteBuffer::getNumeric(int64_t& v) { if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { - v = *(int64_t *) (void *) getBufferAtPos(); + v = *reinterpret_cast(getBufferAtPos()); incPosNoCheck(sizeof(v)); } } @@ -181,13 +172,5 @@ void ByteBuffer::getBytes(void *buffer, size_t count) incPos(count); memcpy(buffer, v, count); } -void ByteBuffer::putBytes(const void *buf, size_t count) { - if (__builtin_expect(getRemaining() < count, 0)) { - throwOutOfBounds(getRemaining(), sizeof(count)); - } else { - memcpy(getBufferAtPos(), buf, count); - incPosNoCheck(count); - } -} } // document diff --git a/document/src/vespa/document/util/bytebuffer.h b/document/src/vespa/document/util/bytebuffer.h index ce01231602d..bb11f009cc6 100644 --- a/document/src/vespa/document/util/bytebuffer.h +++ b/document/src/vespa/document/util/bytebuffer.h @@ -62,13 +62,13 @@ public: static ByteBuffer* copyBuffer(const char* buffer, size_t len); /** @return Returns the buffer pointed to by this object (at position 0) */ - char* getBuffer() const { return _buffer; } + const char* getBuffer() const { return _buffer; } /** @return Returns the length of the buffer pointed to by this object. */ size_t getLength() const { return _len; } /** @return Returns a pointer to the current position in the buffer. */ - char* getBufferAtPos() const { return _buffer + _pos; } + const char* getBufferAtPos() const { return _buffer + _pos; } /** @return Returns the index of the current position in the buffer. */ size_t getPos() const { return _pos; } @@ -79,13 +79,6 @@ public: */ size_t getRemaining() const { return _len -_pos; } - /** - * Changes the position in the buffer. - * - * @throws BufferOutOfBoundsException; - */ - void setPos(size_t pos); - /** * Moves the position in the buffer. * @@ -98,14 +91,6 @@ public: */ void incPos(size_t pos); - /** - * Resets pos to 0, and sets limit to old pos. Use this before reading - * from a buffer you have written to - */ - void flip() { - _pos = 0; - } - void getNumeric(uint8_t & v); void getNumericNetwork(int16_t & v); void getNumericNetwork(int32_t & v); @@ -133,24 +118,15 @@ public: */ void getBytes(void *buffer, size_t count); - /** - * Writes the given number of bytes into the ByteBuffer at the current - * position, and updates the positition accordingly - * - * @param buf the bytes to store - * @param count number of bytes to store - */ - void putBytes(const void *buf, size_t count); - private: template void getDoubleLongNetwork(T &val); void incPosNoCheck(size_t pos) { _pos += pos; } - char * _buffer; - size_t _len; - size_t _pos; + const char * _buffer; + size_t _len; + size_t _pos; vespalib::alloc::Alloc _ownedBuffer; }; diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/documentstate.cpp b/documentapi/src/vespa/documentapi/messagebus/messages/documentstate.cpp index 718611dfc04..6c8394b1b4c 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/documentstate.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/messages/documentstate.cpp @@ -15,12 +15,11 @@ DocumentState::DocumentState(const DocumentState& o) : _gid(o._gid), _timestamp(o._timestamp), _removeEntry(o._removeEntry) { if (o._docId.get() != 0) { - _docId.reset(new document::DocumentId(*o._docId)); + _docId = std::make_unique(*o._docId); } } -DocumentState::DocumentState(const document::DocumentId& id, - uint64_t timestamp, bool removeEntry) +DocumentState::DocumentState(const document::DocumentId& id, uint64_t timestamp, bool removeEntry) : _docId(new document::DocumentId(id)), _gid(_docId->getGlobalId()), _timestamp(timestamp), @@ -28,8 +27,7 @@ DocumentState::DocumentState(const document::DocumentId& id, { } -DocumentState::DocumentState(const document::GlobalId& gid, - uint64_t timestamp, bool removeEntry) +DocumentState::DocumentState(const document::GlobalId& gid, uint64_t timestamp, bool removeEntry) : _gid(gid), _timestamp(timestamp), _removeEntry(removeEntry) {} DocumentState::DocumentState(document::ByteBuffer& buf) @@ -39,10 +37,10 @@ DocumentState::DocumentState(document::ByteBuffer& buf) buf.getByte(hasDocId); if (hasDocId) { vespalib::nbostream stream(buf.getBufferAtPos(), buf.getRemaining()); - _docId.reset(new document::DocumentId(stream)); + _docId = std::make_unique(stream); buf.incPos(stream.rp()); } - char* gid = buf.getBufferAtPos(); + const char* gid = buf.getBufferAtPos(); buf.incPos(document::GlobalId::LENGTH); _gid.set(gid); buf.getLongNetwork((int64_t&) _timestamp); @@ -55,8 +53,8 @@ DocumentState& DocumentState::operator=(const DocumentState& other) { _docId.reset(); - if (other._docId.get() != 0) { - _docId.reset(new document::DocumentId(*other._docId)); + if (other._docId) { + _docId = std::make_unique(*other._docId); } _gid = other._gid; _timestamp = other._timestamp; diff --git a/documentapi/src/vespa/documentapi/messagebus/routablefactories60.cpp b/documentapi/src/vespa/documentapi/messagebus/routablefactories60.cpp index 2fe9ffa3da9..58367410ddc 100644 --- a/documentapi/src/vespa/documentapi/messagebus/routablefactories60.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/routablefactories60.cpp @@ -438,14 +438,12 @@ RoutableFactories60::GetBucketStateReplyFactory::doEncode(const DocumentReply &o DocumentMessage::UP RoutableFactories60::GetDocumentMessageFactory::doDecode(document::ByteBuffer &buf) const { - return DocumentMessage::UP( - new GetDocumentMessage(decodeDocumentId(buf), - decodeString(buf))); + document::DocumentId docId = decodeDocumentId(buf); + return std::make_unique(docId, decodeString(buf)); } bool -RoutableFactories60::GetDocumentMessageFactory::doEncode(const DocumentMessage &obj, - vespalib::GrowableByteBuffer &buf) const +RoutableFactories60::GetDocumentMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const { const GetDocumentMessage &msg = static_cast(obj); @@ -526,7 +524,7 @@ void RoutableFactories60::PutDocumentMessageFactory::decodeInto(PutDocumentMessage & msg, document::ByteBuffer & buf) const { vespalib::nbostream stream(buf.getBufferAtPos(), buf.getRemaining()); msg.setDocument(make_shared(_repo, stream)); - buf.incPos(buf.getRemaining() - stream.size()); + buf.incPos(stream.rp()); msg.setTimestamp(static_cast(decodeLong(buf))); decodeTasCondition(msg, buf); } @@ -550,9 +548,6 @@ RoutableFactories60::PutDocumentReplyFactory::doDecode(document::ByteBuffer &buf { auto reply = make_unique(DocumentProtocol::REPLY_PUTDOCUMENT); reply->setHighestModificationTimestamp(decodeLong(buf)); - - // Doing an explicit move here to force converting result to an rvalue. - // This is done automatically in GCC >= 5. return reply; } @@ -768,7 +763,9 @@ RoutableFactories60::StatDocumentReplyFactory::doEncode(const DocumentReply &, v void RoutableFactories60::UpdateDocumentMessageFactory::decodeInto(UpdateDocumentMessage & msg, document::ByteBuffer & buf) const { - msg.setDocumentUpdate(document::DocumentUpdate::createHEAD(_repo, buf)); + vespalib::nbostream stream(buf.getBufferAtPos(), buf.getRemaining()); + msg.setDocumentUpdate(document::DocumentUpdate::createHEAD(_repo, stream)); + buf.incPos(stream.rp()); msg.setOldTimestamp(static_cast(decodeLong(buf))); msg.setNewTimestamp(static_cast(decodeLong(buf))); decodeTasCondition(msg, buf); diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp index 10f03b8f8e4..5c8b5b029d2 100644 --- a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp +++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp @@ -260,7 +260,7 @@ verifyDocs(const std::vector& wanted, << entry.getDocument()->toString(true); } EXPECT_EQ(wanted[wantedIdx].timestamp, entry.getTimestamp()); - size_t serSize = wanted[wantedIdx].doc->getSerializedSize(); + size_t serSize = wanted[wantedIdx].doc->serialize().size(); EXPECT_EQ(serSize + sizeof(DocEntry), size_t(entry.getSize())); EXPECT_EQ(serSize, size_t(entry.getDocumentSize())); ++wantedIdx; diff --git a/persistence/src/vespa/persistence/spi/docentry.cpp b/persistence/src/vespa/persistence/spi/docentry.cpp index d482d144d73..f46be6f3a25 100644 --- a/persistence/src/vespa/persistence/spi/docentry.cpp +++ b/persistence/src/vespa/persistence/spi/docentry.cpp @@ -2,6 +2,7 @@ #include "docentry.h" #include +#include #include #include @@ -10,16 +11,13 @@ namespace storage::spi { DocEntry::DocEntry(Timestamp t, int metaFlags, DocumentUP doc) : _timestamp(t), _metaFlags(metaFlags), - _persistedDocumentSize(doc->getSerializedSize()), + _persistedDocumentSize(doc->serialize().size()), _size(_persistedDocumentSize + sizeof(DocEntry)), _documentId(), _document(std::move(doc)) { } -DocEntry::DocEntry(Timestamp t, - int metaFlags, - DocumentUP doc, - size_t serializedDocumentSize) +DocEntry::DocEntry(Timestamp t, int metaFlags, DocumentUP doc, size_t serializedDocumentSize) : _timestamp(t), _metaFlags(metaFlags), _persistedDocumentSize(serializedDocumentSize), diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp index e658bc0cfa0..1bcbe4e9683 100644 --- a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp +++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp @@ -31,7 +31,7 @@ DocEntry *createDocEntry(Timestamp timestamp, bool removed, Document::UP doc, ss if (removed) { return new DocEntry(timestamp, storage::spi::REMOVE_ENTRY, doc->getId()); } else { - ssize_t serializedSize = defaultSerializedSize >= 0 ? defaultSerializedSize : doc->getSerializedSize(); + ssize_t serializedSize = defaultSerializedSize >= 0 ? defaultSerializedSize : doc->serialize().size(); return new DocEntry(timestamp, storage::spi::NONE, std::move(doc), serializedSize); } } else { diff --git a/searchlib/src/tests/transactionlogstress/translogstress.cpp b/searchlib/src/tests/transactionlogstress/translogstress.cpp index 36ce43cc52a..76587d78b69 100644 --- a/searchlib/src/tests/transactionlogstress/translogstress.cpp +++ b/searchlib/src/tests/transactionlogstress/translogstress.cpp @@ -17,7 +17,7 @@ LOG_SETUP("translogstress"); -using document::ByteBuffer; +using vespalib::nbostream; using search::Runnable; using vespalib::Monitor; using vespalib::MonitorGuard; @@ -47,10 +47,10 @@ public: BufferGenerator(uint32_t minStrLen, uint32_t maxStrLen) : _rnd(), _minStrLen(minStrLen), _maxStrLen(maxStrLen) {} void setSeed(long seed) { _rnd.srand48(seed); } - ByteBuffer getRandomBuffer(); + nbostream getRandomBuffer(); }; -ByteBuffer +nbostream BufferGenerator::getRandomBuffer() { size_t len = _minStrLen + _rnd.lrand48() % (_maxStrLen - _minStrLen); @@ -59,9 +59,8 @@ BufferGenerator::getRandomBuffer() char c = 'a' + _rnd.lrand48() % ('z' - 'a' + 1); str.push_back(c); } - ByteBuffer buf(str.size() + 1); - buf.putBytes(str.c_str(), str.size() + 1); - buf.flip(); + nbostream buf(str.size() + 1); + buf.write(str.c_str(), str.size() + 1); return buf; } @@ -75,8 +74,8 @@ private: Rand48 _rnd; long _baseSeed; BufferGenerator _bufferGenerator; - const std::vector * _buffers; - ByteBuffer _lastGeneratedBuffer; + const std::vector * _buffers; + nbostream _lastGeneratedBuffer; public: EntryGenerator(long baseSeed, const BufferGenerator & bufferGenerator) : @@ -95,7 +94,7 @@ public: SerialNum getRandomSerialNum(SerialNum begin, SerialNum end); Packet::Entry getRandomEntry(SerialNum num); Rand48 & getRnd() { return _rnd; } - void setBuffers(const std::vector & buffers) { + void setBuffers(const std::vector & buffers) { _buffers = &buffers; } }; @@ -118,12 +117,12 @@ EntryGenerator::getRandomEntry(SerialNum num) _rnd.srand48(_baseSeed + num); if (_buffers != NULL) { size_t i = _rnd.lrand48() % _buffers->size(); - const ByteBuffer& buffer = (*_buffers)[i]; - return Packet::Entry(num, 1024, ConstBufferRef(buffer.getBuffer(), buffer.getLength())); + const nbostream& buffer = (*_buffers)[i]; + return Packet::Entry(num, 1024, ConstBufferRef(buffer.c_str(), buffer.size())); } else { _bufferGenerator.setSeed(_baseSeed + num); - _lastGeneratedBuffer = std::move(_bufferGenerator.getRandomBuffer()); - return Packet::Entry(num, 1024, ConstBufferRef(_lastGeneratedBuffer.getBuffer(), _lastGeneratedBuffer.getLength())); + _lastGeneratedBuffer = _bufferGenerator.getRandomBuffer(); + return Packet::Entry(num, 1024, ConstBufferRef(_lastGeneratedBuffer.c_str(), _lastGeneratedBuffer.size())); } } @@ -708,7 +707,7 @@ TransLogStress::Main() BufferGenerator bufferGenerator(_cfg.minStrLen, _cfg.maxStrLen); bufferGenerator.setSeed(_cfg.baseSeed); - std::vector buffers; + std::vector buffers; for (uint32_t i = 0; i < _cfg.numPreGeneratedBuffers; ++i) { buffers.push_back(bufferGenerator.getRandomBuffer()); } diff --git a/storage/src/tests/visiting/visitormanagertest.cpp b/storage/src/tests/visiting/visitormanagertest.cpp index 2544fcc9a4b..62dbce890cc 100644 --- a/storage/src/tests/visiting/visitormanagertest.cpp +++ b/storage/src/tests/visiting/visitormanagertest.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -337,7 +338,7 @@ int getTotalSerializedSize(const std::vector& docs) { int total = 0; for (size_t i = 0; i < docs.size(); ++i) { - total += int(docs[i]->getSerializedSize()); + total += int(docs[i]->serialize().size()); } return total; } diff --git a/storage/src/vespa/storage/visiting/recoveryvisitor.cpp b/storage/src/vespa/storage/visiting/recoveryvisitor.cpp index 8bac2bac10a..80e74e890a1 100644 --- a/storage/src/vespa/storage/visiting/recoveryvisitor.cpp +++ b/storage/src/vespa/storage/visiting/recoveryvisitor.cpp @@ -2,7 +2,7 @@ #include "recoveryvisitor.h" - +#include #include #include #include @@ -70,7 +70,7 @@ RecoveryVisitor::handleDocuments(const document::BucketId& bid, } } - hitCounter.addHit(doc->getId(), doc->getSerializedSize()); + hitCounter.addHit(doc->getId(), doc->serialize().size()); int64_t timestamp = doc->getLastModified(); cmd->getDocuments().push_back(documentapi::DocumentListMessage::Entry( diff --git a/storageapi/src/vespa/storageapi/message/persistence.cpp b/storageapi/src/vespa/storageapi/message/persistence.cpp index 8b8dedda231..bd8195c2feb 100644 --- a/storageapi/src/vespa/storageapi/message/persistence.cpp +++ b/storageapi/src/vespa/storageapi/message/persistence.cpp @@ -60,7 +60,7 @@ PutCommand::print(std::ostream& out, bool verbose, const std::string& indent) co { out << "Put(" << getBucketId() << ", " << _doc->getId() << ", timestamp " << _timestamp << ", size " - << _doc->getSerializedSize() << ")"; + << _doc->serialize().size() << ")"; if (verbose) { out << " {\n" << indent << " "; _doc->print(out, verbose, indent + " "); diff --git a/storageapi/src/vespa/storageapi/messageapi/returncode.cpp b/storageapi/src/vespa/storageapi/messageapi/returncode.cpp index 9b497a297a1..634a7166d74 100644 --- a/storageapi/src/vespa/storageapi/messageapi/returncode.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/returncode.cpp @@ -26,7 +26,14 @@ vespalib::string ReturnCode::getResultString(Result result) { vespalib::string ReturnCode::toString() const { - return getResultString(_result) + " : " + _message; + vespalib::string ret = "ReturnCode("; + ret += getResultString(_result); + if ( ! _message.empty()) { + ret += ", "; + ret += _message; + } + ret += ")"; + return ret; } bool -- cgit v1.2.3 From d6a77cbf0cf342f2091c9c92c58ace6ed4951b89 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 20 Jan 2020 17:29:16 +0000 Subject: Add TODO for next commit. --- storage/src/vespa/storage/persistence/mergehandler.cpp | 1 + storageapi/src/vespa/storageapi/message/bucket.h | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) (limited to 'storage') diff --git a/storage/src/vespa/storage/persistence/mergehandler.cpp b/storage/src/vespa/storage/persistence/mergehandler.cpp index 1ef26a969ed..e5e358cbb60 100644 --- a/storage/src/vespa/storage/persistence/mergehandler.cpp +++ b/storage/src/vespa/storage/persistence/mergehandler.cpp @@ -552,6 +552,7 @@ MergeHandler::deserializeDiffDocument( auto doc = std::make_unique(); vespalib::nbostream hbuf(&e._headerBlob[0], e._headerBlob.size()); if (e._bodyBlob.size() > 0) { + // TODO Remove this branch and add warning on error. vespalib::nbostream bbuf(&e._bodyBlob[0], e._bodyBlob.size()); doc->deserialize(repo, hbuf, bbuf); } else { diff --git a/storageapi/src/vespa/storageapi/message/bucket.h b/storageapi/src/vespa/storageapi/message/bucket.h index ec1df33a285..45af2296e8a 100644 --- a/storageapi/src/vespa/storageapi/message/bucket.h +++ b/storageapi/src/vespa/storageapi/message/bucket.h @@ -254,6 +254,8 @@ public: GetBucketDiffCommand::Entry _entry; vespalib::string _docName; std::vector _headerBlob; + // TODO: In theory the body blob could be removed now as all is in one blob + // That will enable simplification of code in document. std::vector _bodyBlob; const document::DocumentTypeRepo *_repo; @@ -282,7 +284,7 @@ public: ApplyBucketDiffCommand(const document::Bucket &bucket, const std::vector& nodes, uint32_t maxBufferSize); - ~ApplyBucketDiffCommand(); + ~ApplyBucketDiffCommand() override; const std::vector& getNodes() const { return _nodes; } const std::vector& getDiff() const { return _diff; } -- cgit v1.2.3 From e7c1a4fc8c55795f707e83ce6d5607e0b41f5099 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Tue, 21 Jan 2020 19:41:33 +0000 Subject: Add stream method and use memcpy over casting. --- document/src/vespa/document/util/bytebuffer.cpp | 8 +++++--- storage/src/tests/common/message_sender_stub.cpp | 2 +- storage/src/tests/persistence/filestorage/filestormanagertest.cpp | 2 +- storage/src/tests/visiting/visitormanagertest.cpp | 2 +- storage/src/vespa/storage/common/bucketmessages.cpp | 3 +-- storage/src/vespa/storage/common/storagelink.cpp | 4 ++-- .../distributor/operations/external/statbucketoperation.cpp | 2 +- storage/src/vespa/storage/storageserver/opslogger.cpp | 8 ++++---- storage/src/vespa/storage/visiting/visitor.cpp | 2 +- storageapi/src/vespa/storageapi/message/visitor.cpp | 2 +- storageapi/src/vespa/storageapi/messageapi/returncode.cpp | 5 +++++ storageapi/src/vespa/storageapi/messageapi/returncode.h | 2 ++ storageapi/src/vespa/storageapi/messageapi/storagereply.cpp | 2 +- 13 files changed, 26 insertions(+), 18 deletions(-) (limited to 'storage') diff --git a/document/src/vespa/document/util/bytebuffer.cpp b/document/src/vespa/document/util/bytebuffer.cpp index c6d760d597c..45d86cdc41e 100644 --- a/document/src/vespa/document/util/bytebuffer.cpp +++ b/document/src/vespa/document/util/bytebuffer.cpp @@ -128,7 +128,8 @@ void ByteBuffer::getNumericNetwork(int16_t & v) { if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { - uint16_t val = *reinterpret_cast(getBufferAtPos()); + uint16_t val; + memcpy(&val, getBufferAtPos(), sizeof(val)); v = ntohs(val); incPosNoCheck(sizeof(v)); } @@ -138,7 +139,8 @@ void ByteBuffer::getNumericNetwork(int32_t & v) { if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { - uint32_t val = *reinterpret_cast(getBufferAtPos()); + uint32_t val; + memcpy(&val, getBufferAtPos(), sizeof(val)); v = ntohl(val); incPosNoCheck(sizeof(v)); } @@ -148,7 +150,7 @@ void ByteBuffer::getNumeric(int64_t& v) { if (__builtin_expect(getRemaining() < sizeof(v), 0)) { throwOutOfBounds(getRemaining(), sizeof(v)); } else { - v = *reinterpret_cast(getBufferAtPos()); + memcpy(&v, getBufferAtPos(), sizeof(v)); incPosNoCheck(sizeof(v)); } } diff --git a/storage/src/tests/common/message_sender_stub.cpp b/storage/src/tests/common/message_sender_stub.cpp index f4e64e07955..a82d45b0b99 100644 --- a/storage/src/tests/common/message_sender_stub.cpp +++ b/storage/src/tests/common/message_sender_stub.cpp @@ -36,7 +36,7 @@ MessageSenderStub::dumpMessage(const api::StorageMessage& msg, bool includeAddre ost << " => " << msg.getAddress()->getIndex(); } if (verbose && msg.getType().isReply()) { - ost << " " << dynamic_cast(msg).getResult().toString(); + ost << " " << dynamic_cast(msg).getResult(); } return ost.str(); diff --git a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp index 64306fa7c24..4576f8a08f8 100644 --- a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp +++ b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp @@ -1501,7 +1501,7 @@ TEST_F(FileStorManagerTest, visiting) { for (uint32_t i=3; i(top.getReply(i)); ASSERT_TRUE(reply.get()); - ASSERT_TRUE(reply->getResult().success()) << reply->getResult().toString(); + ASSERT_TRUE(reply->getResult().success()) << reply->getResult(); info = reply->getBucketInfo(); } diff --git a/storage/src/tests/visiting/visitormanagertest.cpp b/storage/src/tests/visiting/visitormanagertest.cpp index 62dbce890cc..20934d04eaa 100644 --- a/storage/src/tests/visiting/visitormanagertest.cpp +++ b/storage/src/tests/visiting/visitormanagertest.cpp @@ -654,7 +654,7 @@ TEST_F(VisitorManagerTest, visitor_cleanup) { if (api::ReturnCode::ILLEGAL_PARAMETERS == reply->getResult().getResult()) { failures++; } else { - std::cerr << reply->getResult().toString() << "\n"; + std::cerr << reply->getResult() << "\n"; } } else { if (api::ReturnCode::BUSY == reply->getResult().getResult()) { diff --git a/storage/src/vespa/storage/common/bucketmessages.cpp b/storage/src/vespa/storage/common/bucketmessages.cpp index c80b7bbb17a..1a4dc61a3ce 100644 --- a/storage/src/vespa/storage/common/bucketmessages.cpp +++ b/storage/src/vespa/storage/common/bucketmessages.cpp @@ -214,8 +214,7 @@ void BucketDiskMoveReply::print(std::ostream& out, bool, const std::string&) const { out << "BucketDiskMoveReply(" << _bucket.getBucketId() << ", source " << _srcDisk - << ", target " << _dstDisk << ", " << _bucketInfo << ", " - << getResult().toString() << ")"; + << ", target " << _dstDisk << ", " << _bucketInfo << ", " << getResult() << ")"; } std::unique_ptr diff --git a/storage/src/vespa/storage/common/storagelink.cpp b/storage/src/vespa/storage/common/storagelink.cpp index d9612eb48a0..431c90b27f2 100644 --- a/storage/src/vespa/storage/common/storagelink.cpp +++ b/storage/src/vespa/storage/common/storagelink.cpp @@ -142,7 +142,7 @@ void StorageLink::sendDown(const StorageMessage::SP& msg) sendUp(reply); } } else { - ost << " Return code: " << static_cast(*msg).getResult().toString(); + ost << " Return code: " << static_cast(*msg).getResult(); LOGBP(warning, "%s", ost.str().c_str()); } } else if (!_down->onDown(msg)) { @@ -182,7 +182,7 @@ void StorageLink::sendUp(const shared_ptr & msg) sendDown(reply); } } else { - ost << " Return code: " << static_cast(*msg).getResult().toString(); + ost << " Return code: " << static_cast(*msg).getResult(); LOGBP(warning, "%s", ost.str().c_str()); } } else if (!_up->onUp(msg)) { diff --git a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp index c19168ca6e1..60c1137bd6d 100644 --- a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp @@ -81,7 +81,7 @@ StatBucketOperation::onReceive(DistributorMessageSender& sender, const std::shar if (myreply.getResult().getResult() == api::ReturnCode::OK) { ost << "\tBucket information from node " << found->second << ":\n" << myreply.getResults() << "\n\n"; } else { - ost << "\tBucket information retrieval failed on node " << found->second << ": " << myreply.getResult().toString() << "\n\n"; + ost << "\tBucket information retrieval failed on node " << found->second << ": " << myreply.getResult() << "\n\n"; } _results[found->second] = ost.str(); diff --git a/storage/src/vespa/storage/storageserver/opslogger.cpp b/storage/src/vespa/storage/storageserver/opslogger.cpp index 6fc9795993e..b6bceabf7a1 100644 --- a/storage/src/vespa/storage/storageserver/opslogger.cpp +++ b/storage/src/vespa/storage/storageserver/opslogger.cpp @@ -77,7 +77,7 @@ OpsLogger::onPutReply(const std::shared_ptr& msg) std::ostringstream ost; ost << _component.getClock().getTimeInSeconds().getTime() << "\tPUT\t" << msg->getDocumentId() << "\t" - << msg->getResult().toString() << "\n"; + << msg->getResult() << "\n"; { vespalib::LockGuard lock(_lock); if (_targetFile == nullptr) return false; @@ -94,7 +94,7 @@ OpsLogger::onUpdateReply(const std::shared_ptr& msg) std::ostringstream ost; ost << _component.getClock().getTimeInSeconds().getTime() << "\tUPDATE\t" << msg->getDocumentId() << "\t" - << msg->getResult().toString() << "\n"; + << msg->getResult() << "\n"; { vespalib::LockGuard lock(_lock); if (_targetFile == nullptr) return false; @@ -111,7 +111,7 @@ OpsLogger::onRemoveReply(const std::shared_ptr& msg) std::ostringstream ost; ost << _component.getClock().getTimeInSeconds().getTime() << "\tREMOVE\t" << msg->getDocumentId() << "\t" - << msg->getResult().toString() << "\n"; + << msg->getResult() << "\n"; { vespalib::LockGuard lock(_lock); if (_targetFile == nullptr) return false; @@ -128,7 +128,7 @@ OpsLogger::onGetReply(const std::shared_ptr& msg) std::ostringstream ost; ost << _component.getClock().getTimeInSeconds().getTime() << "\tGET\t" << msg->getDocumentId() << "\t" - << msg->getResult().toString() << "\n"; + << msg->getResult() << "\n"; { vespalib::LockGuard lock(_lock); if (_targetFile == nullptr) return false; diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp index 153d75fd50e..bdd066e8a4a 100644 --- a/storage/src/vespa/storage/visiting/visitor.cpp +++ b/storage/src/vespa/storage/visiting/visitor.cpp @@ -969,7 +969,7 @@ Visitor::getStatus(std::ostream& out, bool verbose) const << "\n"; out << "Current status" - << _result.toString() << "\n"; + << _result << "\n"; out << "Failed" << (failed() ? "true" : "false") << "\n"; diff --git a/storageapi/src/vespa/storageapi/message/visitor.cpp b/storageapi/src/vespa/storageapi/message/visitor.cpp index f398b4c8146..e531e73fea5 100644 --- a/storageapi/src/vespa/storageapi/message/visitor.cpp +++ b/storageapi/src/vespa/storageapi/message/visitor.cpp @@ -181,7 +181,7 @@ VisitorInfoCommand::print(std::ostream& out, bool verbose, const std::string& in out << "VisitorInfoCommand("; if (_completed) { out << "completed"; } if (_error.failed()) { - out << _error.toString(); + out << _error; } if (verbose) { out << ") : "; diff --git a/storageapi/src/vespa/storageapi/messageapi/returncode.cpp b/storageapi/src/vespa/storageapi/messageapi/returncode.cpp index 634a7166d74..1868e53e4a5 100644 --- a/storageapi/src/vespa/storageapi/messageapi/returncode.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/returncode.cpp @@ -36,6 +36,11 @@ ReturnCode::toString() const { return ret; } +std::ostream & +operator << (std::ostream & os, const ReturnCode & returnCode) { + return os << returnCode.toString(); +} + bool ReturnCode::isBusy() const { diff --git a/storageapi/src/vespa/storageapi/messageapi/returncode.h b/storageapi/src/vespa/storageapi/messageapi/returncode.h index 58392e545a9..305a998918c 100644 --- a/storageapi/src/vespa/storageapi/messageapi/returncode.h +++ b/storageapi/src/vespa/storageapi/messageapi/returncode.h @@ -105,4 +105,6 @@ public: vespalib::string toString() const; }; +std::ostream & operator << (std::ostream & os, const ReturnCode & returnCode); + } diff --git a/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp b/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp index 1033b016629..81cdadb3623 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp @@ -25,7 +25,7 @@ StorageReply::print(std::ostream& out, bool verbose, const std::string& indent) const { (void) verbose; (void) indent; - out << "StorageReply(" << _type.getName() << ", " << _result.toString() << ")"; + out << "StorageReply(" << _type.getName() << ", " << _result << ")"; } } -- cgit v1.2.3