Remove and indirection for document id, for less memory footprint, and better generated code.

17 files changed, 177 insertions, 293 deletions

diff --git a/document/src/tests/base/documentid_benchmark.cpp b/document/src/tests/base/documentid_benchmark.cpp
index 7cfc880ba22..20c91d4a21e 100644
--- a/document/src/tests/base/documentid_benchmark.cpp
+++ b/document/src/tests/base/documentid_benchmark.cpp
@@ -10,12 +10,12 @@ int main(int argc, char *argv[])
         fprintf(stderr, "Usage %s <docid> <count>\n", argv[0]);
     }
     vespalib::string s(argv[1]);
-    uint64_t n = strtoul(argv[2], NULL, 0);
+    uint64_t n = strtoul(argv[2], nullptr, 0);
     printf("Creating documentid '%s' %" PRIu64 " times\n", s.c_str(), n);
-    printf("sizeof(IdString)=%ld,  sizeof(IdIdString)=%ld\n", sizeof(IdString), sizeof(IdIdString));
+    printf("sizeof(IdString)=%ld,  sizeof(IdIdString)=%ld\n", sizeof(IdString), sizeof(IdString));
     for (uint64_t i=0; i < n; i++) {
-        IdString::UP id = IdString::createIdString(s);
-        id.reset();
+        IdString id(s);
+        (void) id;
     }
     return 0;
 }
diff --git a/document/src/tests/base/documentid_test.cpp b/document/src/tests/base/documentid_test.cpp
index 741a490210f..e2bb9dbc4e9 100644
--- a/document/src/tests/base/documentid_test.cpp
+++ b/document/src/tests/base/documentid_test.cpp
@@ -14,10 +14,8 @@ const string ns = "namespace";
 const string ns_id = "namespaceid";
 const string type = "my_type";
 
-void checkId(const string &id, IdString::Type t,
-             const string &ns_str, const string local_id) {
+void checkId(const string &id, const string &ns_str, const string local_id) {
     DocumentId doc_id(id);
-    EXPECT_EQUAL(t, doc_id.getScheme().getType());
     EXPECT_EQUAL(ns_str, doc_id.getScheme().getNamespace());
     EXPECT_EQUAL(local_id, doc_id.getScheme().getNamespaceSpecific());
     EXPECT_EQUAL(id, doc_id.getScheme().toString());
@@ -44,21 +42,21 @@ void checkType(const string &id, const string &doc_type) {
 
 TEST("require that id id can be parsed") {
     const string id = "id:" + ns + ":" + type + "::" + ns_id;
-    checkId(id, IdString::ID, ns, ns_id);
+    checkId(id, ns, ns_id);
     checkType(id, type);
 }
 
 TEST("require that we allow ':' in namespace specific part") {
     const string nss=":a:b:c:";
     string id="id:" + ns + ":" + type + "::" + nss;
-    checkId(id, IdString::ID, ns, nss);
+    checkId(id, ns, nss);
     checkType(id, type);
 }
 
 TEST("require that id id can specify location") {
     DocumentId id("id:ns:type:n=12345:foo");
     EXPECT_EQUAL(12345u, id.getScheme().getLocation());
-    EXPECT_EQUAL(12345u, getAs<IdIdString>(id).getNumber());
+    EXPECT_EQUAL(12345u, getAs<IdString>(id).getNumber());
 }
 
 TEST("require that id id's n key must be a 64-bit number") {
@@ -76,7 +74,7 @@ TEST("require that id id can specify group") {
     EXPECT_EQUAL(id1.getScheme().getLocation(), id2.getScheme().getLocation());
     EXPECT_NOT_EQUAL(id1.getScheme().getLocation(),
                      id3.getScheme().getLocation());
-    EXPECT_EQUAL("mygroup", getAs<IdIdString>(id1).getGroup());
+    EXPECT_EQUAL("mygroup", getAs<IdString>(id1).getGroup());
 }
 
 TEST("require that id id location is specified by local id only by default") {
@@ -89,15 +87,15 @@ TEST("require that id id location is specified by local id only by default") {
 
 TEST("require that local id can be empty") {
     const string id = "id:" + ns + ":type:n=1234:";
-    checkId(id, IdString::ID, ns, "");
-    checkUser<IdIdString>(id, 1234);
+    checkId(id, ns, "");
+    checkUser<IdString>(id, 1234);
 }
 
 TEST("require that document ids can be assigned") {
     DocumentId id1("id:" + ns + ":type:n=1234:");
     DocumentId id2 = id1;
-    checkId(id2.toString(), IdString::ID, ns, "");
-    checkUser<IdIdString>(id2.toString(), 1234);
+    checkId(id2.toString(), ns, "");
+    checkUser<IdString>(id2.toString(), 1234);
 }
 
 TEST("require that illegal ids fail") {
diff --git a/document/src/tests/documenttestcase.cpp b/document/src/tests/documenttestcase.cpp
index b0128607ac5..d75a16ea1a6 100644
--- a/document/src/tests/documenttestcase.cpp
+++ b/document/src/tests/documenttestcase.cpp
@@ -28,8 +28,9 @@ using namespace fieldvalue;
 
 TEST(DocumentTest, testSizeOf)
 {
-    EXPECT_EQ(24ul, sizeof(DocumentId));
-    EXPECT_EQ(128ul, sizeof(Document));
+    EXPECT_EQ(88ul, sizeof(IdString));
+    EXPECT_EQ(104ul, sizeof(DocumentId));
+    EXPECT_EQ(208ul, sizeof(Document));
     EXPECT_EQ(72ul, sizeof(StructFieldValue));
     EXPECT_EQ(24ul, sizeof(StructuredFieldValue));
     EXPECT_EQ(64ul, sizeof(SerializableArray));
diff --git a/document/src/vespa/document/base/documentid.cpp b/document/src/vespa/document/base/documentid.cpp
index 5c240922e6d..58d9a83fc5b 100644
--- a/document/src/vespa/document/base/documentid.cpp
+++ b/document/src/vespa/document/base/documentid.cpp
@@ -10,19 +10,19 @@ namespace document {
 
 DocumentId::DocumentId()
     : _globalId(),
-      _id(new NullIdString())
+      _id()
 {
 }
 
 DocumentId::DocumentId(vespalib::stringref id)
     : _globalId(),
-      _id(IdString::createIdString(id.data(), id.size()).release())
+      _id(id)
 {
 }
 
 DocumentId::DocumentId(vespalib::nbostream & is)
     : _globalId(),
-      _id(IdString::createIdString(is.peek(), strlen(is.peek())).release())
+      _id({is.peek(), strlen(is.peek())})
 {
     is.adjustReadPos(strlen(is.peek()) + 1);
 }
@@ -33,30 +33,30 @@ DocumentId::~DocumentId() = default;
 
 vespalib::string
 DocumentId::toString() const {
-    return _id->toString();
+    return _id.toString();
 }
 
 void
 DocumentId::set(vespalib::stringref id) {
-    _id.reset(IdString::createIdString(id).release());
+    _id = IdString(id);
     _globalId.first = false;
 }
 
 size_t
 DocumentId::getSerializedSize() const
 {
-    return _id->toString().size() + 1;
+    return _id.toString().size() + 1;
 }
 
 void
 DocumentId::calculateGlobalId() const
 {
-    vespalib::string id(_id->toString());
+    vespalib::string id(_id.toString());
 
     unsigned char key[16];
     fastc_md5sum(reinterpret_cast<const unsigned char*>(id.c_str()), id.size(), key);
 
-    IdString::LocationType location(_id->getLocation());
+    IdString::LocationType location(_id.getLocation());
     memcpy(key, &location, 4);
 
     _globalId.first = true;
diff --git a/document/src/vespa/document/base/documentid.h b/document/src/vespa/document/base/documentid.h
index 99ab63bd296..2e4f54b43d3 100644
--- a/document/src/vespa/document/base/documentid.h
+++ b/document/src/vespa/document/base/documentid.h
@@ -60,12 +60,12 @@ public:
     */
     vespalib::string toString() const;
 
-    bool operator==(const DocumentId& other) const { return *_id == *other._id; }
-    bool operator!=(const DocumentId& other) const { return ! (*_id == *other._id); }
+    bool operator==(const DocumentId& other) const { return _id == other._id; }
+    bool operator!=(const DocumentId& other) const { return ! (_id == other._id); }
 
-    const IdString& getScheme() const { return *_id; }
-    bool hasDocType() const { return _id->hasDocType(); }
-    vespalib::string getDocType() const { return _id->getDocType(); }
+    const IdString& getScheme() const { return _id; }
+    bool hasDocType() const { return _id.hasDocType(); }
+    vespalib::string getDocType() const { return _id.getDocType(); }
 
     const GlobalId& getGlobalId() const {
         if (!_globalId.first) { calculateGlobalId(); }
@@ -75,7 +75,7 @@ public:
     size_t getSerializedSize() const;
 private:
     mutable std::pair<bool, GlobalId> _globalId;
-    vespalib::CloneablePtr<IdString> _id;
+    IdString _id;
 
     void calculateGlobalId() const;
 };
diff --git a/document/src/vespa/document/base/idstring.cpp b/document/src/vespa/document/base/idstring.cpp
index 1dcc11aa891..288c1d38133 100644
--- a/document/src/vespa/document/base/idstring.cpp
+++ b/document/src/vespa/document/base/idstring.cpp
@@ -17,43 +17,28 @@ VESPA_IMPLEMENT_EXCEPTION(IdParseException, vespalib::Exception);
 
 namespace {
 
-string _G_typeName[2] = {
-    "id",
-    "null"
-};
-
-}
-
-const string &
-IdString::getTypeName(Type t)
-{
-    return _G_typeName[t];
-}
-
-const string &
-IdString::toString() const
-{
-    return _rawId;
-}
-
-namespace {
-
 void reportError(const char* part) __attribute__((noinline));
 void reportTooShortDocId(const char * id, size_t sz) __attribute__((noinline));
 void reportNoSchemeSeparator(const char * id) __attribute__((noinline));
+void reportNoId(const char * id) __attribute__((noinline));
 
-void reportError(const char* part)
-{
+void
+reportError(const char* part) {
     throw IdParseException(make_string("Unparseable id: No %s separator ':' found", part), VESPA_STRLOC);
 }
 
-void reportNoSchemeSeparator(const char * id)
-{
+void
+reportNoSchemeSeparator(const char * id) {
     throw IdParseException(make_string("Unparseable id '%s': No scheme separator ':' found", id), VESPA_STRLOC);
 }
 
-void reportTooShortDocId(const char * id, size_t sz)
-{
+void
+reportNoId(const char * id){
+    throw IdParseException(make_string("Unparseable id '%s': No 'id:' found", id), VESPA_STRLOC);
+}
+
+void
+reportTooShortDocId(const char * id, size_t sz) {
     throw IdParseException( make_string( "Unparseable id '%s': It is too short(%li) " "to make any sense", id, sz), VESPA_STRLOC);
 }
 
@@ -74,8 +59,6 @@ typedef char v16qi __attribute__ ((__vector_size__(16)));
 
 v16qi _G_zero  = { ':', ':', ':', ':', ':', ':', ':', ':', ':', ':', ':', ':', ':', ':', ':', ':' };
 
-//const char * fmemchr_stdc(const char * s, const char * e) __attribute__((noinline));
-
 inline const char *
 fmemchr(const char * s, const char * e)
 {
@@ -119,64 +102,14 @@ fmemchr(const char * s, const char * e)
     return nullptr;
 }
 
-}  // namespace
-
-
-IdString::Offsets::Offsets(uint32_t maxComponents, uint32_t namespaceOffset, stringref id)
-{
-    _offsets[0] = namespaceOffset;
-    size_t index(1);
-    const char * s(id.data() + namespaceOffset);
-    const char * e(id.data() + id.size());
-    for(s=fmemchr(s, e);
-        (s != nullptr) && (index < maxComponents);
-        s = fmemchr(s+1, e))
-    {
-        _offsets[index++] = s - id.data() + 1;
-    }
-    _numComponents = index;
-    for (;index < VESPA_NELEMS(_offsets); index++) {
-        _offsets[index] = id.size() + 1; // 1 is added due to the implicitt accounting for ':'
-    }
-    _offsets[maxComponents] = id.size() + 1; // 1 is added due to the implicitt accounting for ':'
-}
-
-IdString::IdString(uint32_t maxComponents, uint32_t namespaceOffset, stringref rawId) :
-    _offsets(maxComponents, namespaceOffset, rawId),
-    _rawId(rawId)
-{
-}
-
-IdString::~IdString() = default;
-
 void
-IdString::validate() const
-{
-    if (_offsets.numComponents() < 2) {
-        reportError("namespace");
-    }
-}
-
-void
-IdIdString::validate() const
-{
-    IdString::validate();
-    if (getNumComponents() < 3) {
-        reportError("document type");
-    }
-    if (getNumComponents() < 4) {
-        reportError("key/value-pairs");
-    }
-}
-
-IdString::UP
-IdString::createIdString(const char * id, size_t sz_)
+verifyIdString(const char * id, size_t sz_)
 {
     if (sz_ > 4) {
         if (_G_id.as16 == *reinterpret_cast<const uint16_t *>(id) && id[2] == ':') {
-            return std::make_unique<IdIdString>(stringref(id, sz_));
+            return;
         } else if ((sz_ == 6) && (_G_null.as32 == *reinterpret_cast<const uint32_t *>(id)) && (id[4] == ':') && (id[5] == ':')) {
-            return std::make_unique<NullIdString>();
+            reportNoId(id);
         } else if (sz_ > 8) {
             reportNoSchemeSeparator(id);
         } else {
@@ -185,10 +118,28 @@ IdString::createIdString(const char * id, size_t sz_)
     } else {
         reportTooShortDocId(id, 5);
     }
-    return IdString::UP();
 }
 
-namespace {
+void
+validate(uint16_t numComponents)
+{
+    if (numComponents < 2) {
+        reportError("namespace");
+    }
+    if (numComponents < 3) {
+        reportError("document type");
+    }
+    if (numComponents < 4) {
+        reportError("key/value-pairs");
+    }
+}
+
+
+constexpr uint32_t NAMESPACE_OFFSET = 3;
+constexpr uint32_t MAX_COMPONENTS = 4;
+
+const vespalib::stringref DEFAULT_ID("id::::");
+
 union LocationUnion {
     uint8_t _key[16];
     IdString::LocationType _location[2];
@@ -219,6 +170,35 @@ void setLocation(IdString::LocationType &loc, IdString::LocationType val,
 
 }  // namespace
 
+const IdString::Offsets IdString::Offsets::DefaultID(DEFAULT_ID);
+
+IdString::Offsets::Offsets(stringref id)
+    : _offsets()
+{
+    compute(id);
+}
+
+uint16_t
+IdString::Offsets::compute(stringref id)
+{
+    _offsets[0] = NAMESPACE_OFFSET;
+    size_t index(1);
+    const char * s(id.data() + NAMESPACE_OFFSET);
+    const char * e(id.data() + id.size());
+    for(s=fmemchr(s, e);
+        (s != nullptr) && (index < MAX_COMPONENTS);
+        s = fmemchr(s+1, e))
+    {
+        _offsets[index++] = s - id.data() + 1;
+    }
+    uint16_t numComponents = index;
+    for (;index < VESPA_NELEMS(_offsets); index++) {
+        _offsets[index] = id.size() + 1; // 1 is added due to the implicitt accounting for ':'
+    }
+    _offsets[MAX_COMPONENTS] = id.size() + 1; // 1 is added due to the implicitt accounting for ':'
+    return numComponents;
+}
+
 IdString::LocationType
 IdString::makeLocation(stringref s) {
     LocationUnion location;
@@ -226,14 +206,26 @@ IdString::makeLocation(stringref s) {
     return location._location[0];
 }
 
-IdIdString::IdIdString(stringref id)
-    : IdString(4, 3, id),
+IdString::IdString()
+    : _rawId(DEFAULT_ID),
       _location(0),
+      _offsets(Offsets::DefaultID),
+      _groupOffset(0),
+      _has_number(false)
+{
+}
+
+IdString::IdString(stringref id)
+    : _rawId(id),
+      _location(0),
+      _offsets(),
       _groupOffset(0),
       _has_number(false)
 {
     // TODO(magnarn): Require that keys are lexicographically ordered.
-    validate();
+    verifyIdString(id.data(), id.size());
+    validate(_offsets.compute(id));
+
 
     stringref key_values(getComponent(2));
     char key(0);
diff --git a/document/src/vespa/document/base/idstring.h b/document/src/vespa/document/base/idstring.h
index 8e8cf0057dc..6e21aebc478 100644
--- a/document/src/vespa/document/base/idstring.h
+++ b/document/src/vespa/document/base/idstring.h
@@ -3,7 +3,6 @@
 #pragma once
 
 #include <vespa/vespalib/util/memory.h>
-#include <vespa/vespalib/objects/cloneable.h>
 #include <vespa/vespalib/stllike/string.h>
 #include <cstdint>
 
@@ -13,108 +12,60 @@ namespace document {
  * \class document::IdString
  * \ingroup base
  *
- * \brief Superclass for all document identifier schemes.
+ * \brief New scheme for documents with no forced distribution.
+ *
+ * By using this scheme, documents will be evenly distributed within VDS,
+ * as the location of a doc identifier is a hash of the entire URI.
+ * This scheme also contains the DocumentType.
  */
-class IdString : public vespalib::Cloneable {
+class IdString {
 public:
-    typedef std::unique_ptr<IdString> UP;
-    typedef vespalib::CloneablePtr<IdString> CP;
     typedef uint64_t LocationType;
-    enum Type { ID=0, NULLID };
-    static const vespalib::string & getTypeName(Type t);
-
-    /** @throws document::IdParseException If parsing of id scheme failed. */
-    static IdString::UP createIdString(vespalib::stringref id) { return createIdString(id.data(), id.size()); }
-    static IdString::UP createIdString(const char *id, size_t sz);
     static LocationType makeLocation(vespalib::stringref s);
 
-    ~IdString();
-    IdString* clone() const override = 0;
+    explicit IdString(vespalib::stringref ns);
+    IdString();
 
-    virtual Type        getType() const = 0;
     vespalib::stringref getNamespace() const { return getComponent(0); }
-    virtual vespalib::stringref getNamespaceSpecific() const = 0;
-    virtual LocationType getLocation() const = 0;
-    virtual std::pair<int16_t, int64_t> getGidBitsOverride() const { return std::pair<int16_t, int64_t>(0, 0); }
-    virtual bool hasDocType() const { return false; }
-    virtual vespalib::stringref getDocType() const { return ""; }
-    virtual bool hasNumber() const { return false; }
-    virtual uint64_t getNumber() const { return 0; }
-    virtual bool hasGroup() const { return false; }
-    virtual vespalib::stringref getGroup() const { return ""; }
+    bool hasDocType() const { return size(1 != 0); }
+    vespalib::stringref getDocType() const  { return getComponent(1); }
+    LocationType getLocation() const  { return _location; }
+    bool hasNumber() const  { return _has_number; }
+    uint64_t getNumber() const  { return _location; }
+    bool hasGroup() const  { return _groupOffset != 0; }
+    vespalib::stringref getGroup() const  {
+        return vespalib::stringref(getRawId().c_str() + _groupOffset, offset(3) - _groupOffset - 1);
+    }
+    vespalib::stringref getNamespaceSpecific() const { return getComponent(3); }
 
     bool operator==(const IdString& other) const
         { return toString() == other.toString(); }
 
-    const vespalib::string & toString() const;
+    const vespalib::string & toString() const { return _rawId; }
 
-protected:
-    IdString(uint32_t maxComponents, uint32_t namespaceOffset, vespalib::stringref rawId);
+private:
     size_t offset(size_t index) const { return _offsets[index]; }
-    size_t size(size_t index) const { return _offsets[index+1] - _offsets[index] - 1; }
+    size_t size(size_t index) const { return std::max(0, _offsets[index+1] - _offsets[index] - 1); }
     vespalib::stringref getComponent(size_t index) const { return vespalib::stringref(_rawId.c_str() + offset(index), size(index)); }
     const vespalib::string & getRawId() const { return _rawId; }
-    virtual void validate() const;
-    size_t getNumComponents() const { return _offsets.numComponents(); }
 
-private:
     class Offsets {
     public:
-        Offsets(uint32_t maxComponents, uint32_t first, vespalib::stringref id);
-        uint16_t first() const { return _offsets[0]; }
+        Offsets() = default;
+        uint16_t compute(vespalib::stringref id);
         uint16_t operator [] (size_t i) const { return _offsets[i]; }
-        size_t numComponents() const { return _numComponents; }
+        static const Offsets DefaultID;
     private:
+        Offsets(vespalib::stringref id);
         uint16_t _offsets[5];
-        uint32_t _numComponents;
     };
-    Offsets _offsets;
-    vespalib::string _rawId;
-};
-
-class NullIdString final : public IdString
-{
-public:
-    NullIdString() : IdString(2, 5, "null::") { }
-private:
-    IdString* clone() const override { return new NullIdString(); }
-    LocationType getLocation() const override { return 0; }
-    Type getType() const override { return NULLID; }
-    vespalib::stringref getNamespaceSpecific() const override { return getComponent(1); }
-};
 
-/**
- * \class document::IdIdString
- * \ingroup base
- *
- * \brief New scheme for documents with no forced distribution.
- *
- * By using this scheme, documents will be evenly distributed within VDS,
- * as the location of a doc identifier is a hash of the entire URI.
- * This scheme also contains the DocumentType.
- */
-class IdIdString final : public IdString {
-    LocationType _location;
-    uint16_t     _groupOffset;
-    bool         _has_number;
-
-public:
-    IdIdString(vespalib::stringref ns);
+    vespalib::string _rawId;
+    LocationType     _location;
+    Offsets          _offsets;
+    uint16_t         _groupOffset;
+    bool             _has_number;
 
-    bool hasDocType() const override { return true; }
-    vespalib::stringref getDocType() const override { return getComponent(1); }
-    IdIdString* clone() const override { return new IdIdString(*this); }
-    LocationType getLocation() const override { return _location; }
-    bool hasNumber() const override { return _has_number; }
-    uint64_t getNumber() const override { return _location; }
-    bool hasGroup() const override { return _groupOffset != 0; }
-    vespalib::stringref getGroup() const override {
-        return vespalib::stringref(getRawId().c_str() + _groupOffset, offset(3) - _groupOffset - 1);
-    }
-private:
-    virtual void validate() const override;
-    Type getType() const override { return ID; }
-    vespalib::stringref getNamespaceSpecific() const override { return getComponent(3); }
 };
 
 } // document
diff --git a/document/src/vespa/document/bucket/bucketidfactory.cpp b/document/src/vespa/document/bucket/bucketidfactory.cpp
index 0d7ad9da71e..c5e75093181 100644
--- a/document/src/vespa/document/bucket/bucketidfactory.cpp
+++ b/document/src/vespa/document/bucket/bucketidfactory.cpp
@@ -42,29 +42,16 @@ BucketIdFactory::getBucketId(const DocumentId& id) const
 {
     uint64_t location = id.getScheme().getLocation();
     assert(GlobalId::LENGTH >= sizeof(uint64_t) + 4u);
-    uint64_t gid = reinterpret_cast<const uint64_t&>(
-            *(id.getGlobalId().get() + 4));
+    uint64_t gid = reinterpret_cast<const uint64_t&>(*(id.getGlobalId().get() + 4));
 
-    std::pair<int16_t, int64_t> gidOverride(
-            id.getScheme().getGidBitsOverride());
 
-    if (gidOverride.first > 0) {
-        return BucketId(_locationBits + _gidBits,
-            _initialCount
-            | (gidOverride.second << _locationBits)
-            | ((_gidMask << gidOverride.first) & gid)
-            | (_locationMask & location));
-    } else {
-        return BucketId(_locationBits + _gidBits,
-            _initialCount
-            | (_gidMask & gid)
-            | (_locationMask & location));
-    }
+
+    return BucketId(_locationBits + _gidBits,
+        _initialCount | (_gidMask & gid) | (_locationMask & location));
 }
 
 void
-BucketIdFactory::print(std::ostream& out, bool verbose,
-                       const std::string& indent) const
+BucketIdFactory::print(std::ostream& out, bool verbose, const std::string& indent) const
 {
     out << "BucketIdFactory("
         << _locationBits << " location bits, "
diff --git a/document/src/vespa/document/bucket/bucketselector.cpp b/document/src/vespa/document/bucket/bucketselector.cpp
index 62352718444..26afc1f2e66 100644
--- a/document/src/vespa/document/bucket/bucketselector.cpp
+++ b/document/src/vespa/document/bucket/bucketselector.cpp
@@ -50,7 +50,6 @@ using namespace document::select;
                                   _buckets.begin(), _buckets.end(),
                                   back_inserter(result));
             _buckets.swap(result);
-            return;
         }
 
         void visitOrBranch(const document::select::Or& node) override {
@@ -85,22 +84,21 @@ using namespace document::select;
                 if (!val) return;
                 vespalib::string docId(val->getValue());
                 if (op == FunctionOperator::EQ || !GlobOperator::containsVariables(docId)) {
-                    IdString::UP id(IdString::createIdString(docId));
-                    _buckets.push_back(BucketId(58, id->getLocation()));
+                    _buckets.emplace_back(58, IdString(docId).getLocation());
                     _unknown = false;
                 }
             } else if (node.getType() == IdValueNode::USER) {
                 auto val = dynamic_cast<const IntegerValueNode*>(&valnode);
                 if (!val) return;
-                IdIdString id(vespalib::make_string("id::test:n=%" PRIu64 ":", val->getValue()));
-                _buckets.push_back(BucketId(32, id.getNumber()));
+                IdString id(vespalib::make_string("id::test:n=%" PRIu64 ":", val->getValue()));
+                _buckets.emplace_back(32, id.getNumber());
                 _unknown = false;
             } else if (node.getType() == IdValueNode::GROUP) {
                 auto val = dynamic_cast<const StringValueNode*>(&valnode);
                 if (!val) return;
                 vespalib::string group(val->getValue());
                 if (op == FunctionOperator::EQ || !GlobOperator::containsVariables(group)) {
-                    _buckets.push_back(BucketId(32, IdString::makeLocation(group)));
+                    _buckets.emplace_back(32, IdString::makeLocation(group));
                     _unknown = false;
                 }
             } else if (node.getType() == IdValueNode::GID) {
@@ -109,7 +107,7 @@ using namespace document::select;
                 vespalib::string gid(val->getValue());
                 if (op == FunctionOperator::EQ || !GlobOperator::containsVariables(gid)) {
                     BucketId bid = document::GlobalId::parse(gid).convertToBucketId();
-                    _buckets.push_back(BucketId(32, bid.getRawId()));
+                    _buckets.emplace_back(32, bid.getRawId());
                     _unknown = false;
                 }
             } else if (node.getType() == IdValueNode::BUCKET) {
diff --git a/document/src/vespa/document/fieldvalue/referencefieldvalue.h b/document/src/vespa/document/fieldvalue/referencefieldvalue.h
index 578ab22aa97..0d25e5cb392 100644
--- a/document/src/vespa/document/fieldvalue/referencefieldvalue.h
+++ b/document/src/vespa/document/fieldvalue/referencefieldvalue.h
@@ -36,7 +36,7 @@ public:
     ReferenceFieldValue(const ReferenceDataType& dataType,
                         const DocumentId& documentId);
 
-    ~ReferenceFieldValue();
+    ~ReferenceFieldValue() override;
 
     ReferenceFieldValue(const ReferenceFieldValue&) = default;
     ReferenceFieldValue& operator=(const ReferenceFieldValue&) = default;
diff --git a/document/src/vespa/document/select/valuenodes.cpp b/document/src/vespa/document/select/valuenodes.cpp
index 9cfdae14de1..5c7820b76d7 100644
--- a/document/src/vespa/document/select/valuenodes.cpp
+++ b/document/src/vespa/document/select/valuenodes.cpp
@@ -494,7 +494,7 @@ IdValueNode::getValue(const DocumentId& id) const
     case NS:
         value = id.getScheme().getNamespace(); break;
     case SCHEME:
-        value = id.getScheme().getTypeName(id.getScheme().getType());
+        value = "id";
         break;
     case TYPE:
         if (id.getScheme().hasDocType()) {
@@ -563,7 +563,7 @@ IdValueNode::traceValue(const DocumentId& id, std::ostream& out) const
         out << "Resolved id.namespace to value\"" << value << "\".\n";
         break;
     case SCHEME:
-        value = id.getScheme().getTypeName(id.getScheme().getType());
+        value = "id";
         out << "Resolved id.scheme to value\"" << value << "\".\n";
         break;
     case TYPE:
@@ -586,8 +586,7 @@ IdValueNode::traceValue(const DocumentId& id, std::ostream& out) const
     case GROUP:
         if (id.getScheme().hasGroup()) {
             value = id.getScheme().getGroup();
-            out << "Resolved group of doc (type " << id.getScheme().getType()
-                << ") to \"" << value << "\".\n";
+            out << "Resolved group of doc (type id) to \"" << value << "\".\n";
         } else {
             out << "Can't resolve group of doc \"" << id << "\".\n";
             return std::make_unique<InvalidValue>();
@@ -600,8 +599,7 @@ IdValueNode::traceValue(const DocumentId& id, std::ostream& out) const
     case USER:
         if (id.getScheme().hasNumber()) {
             auto result = std::make_unique<IntegerValue>(id.getScheme().getNumber(), false);
-            out << "Resolved user of doc type " << id.getScheme().getType()
-                << " to " << *result << ".\n";
+            out << "Resolved user of doc type 'id' to " << *result << ".\n";
             return result;
         } else {
             out << "Could not resolve user of doc " << id << ".\n";
diff --git a/persistence/src/vespa/persistence/spi/docentry.cpp b/persistence/src/vespa/persistence/spi/docentry.cpp
index dac3761c6d2..d482d144d73 100644
--- a/persistence/src/vespa/persistence/spi/docentry.cpp
+++ b/persistence/src/vespa/persistence/spi/docentry.cpp
@@ -5,8 +5,7 @@
 #include <sstream>
 #include <cassert>
 
-namespace storage {
-namespace spi {
+namespace storage::spi {
 
 DocEntry::DocEntry(Timestamp t, int metaFlags, DocumentUP doc)
     : _timestamp(t),
@@ -47,17 +46,17 @@ DocEntry::DocEntry(Timestamp t, int metaFlags)
       _document()
 { }
 
-DocEntry::~DocEntry() { }
+DocEntry::~DocEntry() = default;
 
 DocEntry* 
 DocEntry::clone() const {
     DocEntry* ret;
-    if (_documentId.get() != 0) {
+    if (_documentId) {
         ret = new DocEntry(_timestamp, _metaFlags, *_documentId);
         ret->setPersistedDocumentSize(_persistedDocumentSize);
-    } else if (_document.get()) {
+    } else if (_document) {
         ret = new DocEntry(_timestamp, _metaFlags,
-                           DocumentUP(new Document(*_document)),
+                           std::make_unique<Document>(*_document),
                            _persistedDocumentSize);
     } else {
         ret = new DocEntry(_timestamp, _metaFlags);
@@ -68,8 +67,7 @@ DocEntry::clone() const {
 
 const DocumentId*
 DocEntry::getDocumentId() const {
-    return (_document.get() != 0 ? &_document->getId()
-                                 : _documentId.get());
+    return (_document ? &_document->getId() : _documentId.get());
 }
 
 DocumentUP
@@ -89,7 +87,7 @@ DocEntry::toString() const
 {
     std::ostringstream out;
     out << "DocEntry(" << _timestamp << ", " << _metaFlags << ", ";
-    if (_documentId.get() != 0) {
+    if (_documentId) {
         out << *_documentId;
     } else if (_document.get()) {
         out << "Doc(" << _document->getId() << ")";
@@ -100,26 +98,6 @@ DocEntry::toString() const
     return out.str();
 }
 
-void
-DocEntry::prettyPrint(std::ostream& out) const
-{
-    std::string flags;
-    if (_metaFlags == REMOVE_ENTRY) {
-        flags = " (remove)";
-    }
-
-    out << "DocEntry(Timestamp: " << _timestamp
-        << ", size " << getPersistedDocumentSize() << ", ";
-    if (_documentId) {
-        out << *_documentId;
-    } else if (_document.get()) {
-        out << "Doc(" << _document->getId() << ")";
-    } else {
-        out << "metadata only";
-    }
-    out << flags << ")";
-}
-
 std::ostream &
 operator << (std::ostream & os, const DocEntry & r) {
     return os << r.toString();
@@ -169,7 +147,4 @@ DocEntry::operator==(const DocEntry& entry) const {
     return true;
 }
 
-} // spi
-} // storage
-
-
+}
diff --git a/persistence/src/vespa/persistence/spi/docentry.h b/persistence/src/vespa/persistence/spi/docentry.h
index 5ad2856cdef..aa9bf20a7f9 100644
--- a/persistence/src/vespa/persistence/spi/docentry.h
+++ b/persistence/src/vespa/persistence/spi/docentry.h
@@ -15,8 +15,7 @@
 
 #include <persistence/spi/types.h>
 
-namespace storage {
-namespace spi {
+namespace storage::spi {
 
 enum DocumentMetaFlags {
     NONE             = 0x0,
@@ -88,13 +87,9 @@ public:
     }
 
     vespalib::string toString() const;
-    void prettyPrint(std::ostream& out) const;
     bool operator==(const DocEntry& entry) const;
 };
 
 std::ostream & operator << (std::ostream & os, const DocEntry & r);
 
-} // spi
-} // storage
-
-
+}
diff --git a/persistence/src/vespa/persistence/spi/documentselection.h b/persistence/src/vespa/persistence/spi/documentselection.h
index 0957289446f..af734d2e16e 100644
--- a/persistence/src/vespa/persistence/spi/documentselection.h
+++ b/persistence/src/vespa/persistence/spi/documentselection.h
@@ -10,8 +10,7 @@
 
 #include <persistence/spi/types.h>
 
-namespace storage {
-namespace spi {
+namespace storage::spi {
 
 class DocumentSelection
 {
@@ -28,5 +27,3 @@ class DocumentSelection
 };
 
 }
-}
-
diff --git a/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp b/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp
index c59e95a79eb..de2cd3c624f 100644
--- a/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp
+++ b/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp
@@ -145,7 +145,7 @@ TEST("require that toString() on derived classes are meaningful")
     MyStreamHandler stream_handler;
     DocumentIdT doc_id_limit = 15;
     DocumentId doc_id("id:ns:foo:::bar");
-    DocumentUpdate::SP update(new DocumentUpdate(repo, *DataType::DOCUMENT, doc_id));
+    auto update = std::make_shared<DocumentUpdate>(repo, *DataType::DOCUMENT, doc_id);
 
     EXPECT_EQUAL("DeleteBucket(BucketId(0x0000000000000000), serialNum=0)",
                  DeleteBucketOperation().toString());
@@ -167,7 +167,7 @@ TEST("require that toString() on derived classes are meaningful")
     EXPECT_EQUAL("Move(NULL, BucketId(0x0000000000000000), timestamp=0, dbdId=(subDbId=0, lid=0), "
                  "prevDbdId=(subDbId=0, lid=0), prevMarkedAsRemoved=false, prevTimestamp=0, serialNum=0)",
                  MoveOperation().toString());
-    EXPECT_EQUAL("Move(null::, BucketId(0x000000000000002a), timestamp=10, dbdId=(subDbId=1, lid=0), "
+    EXPECT_EQUAL("Move(id::::, BucketId(0x000000000000002a), timestamp=10, dbdId=(subDbId=1, lid=0), "
                  "prevDbdId=(subDbId=0, lid=0), prevMarkedAsRemoved=false, prevTimestamp=0, serialNum=0)",
                  MoveOperation(bucket_id1, timestamp, doc,
                                db_doc_id, sub_db_id).toString());
@@ -188,11 +188,11 @@ TEST("require that toString() on derived classes are meaningful")
     EXPECT_EQUAL("Put(NULL, BucketId(0x0000000000000000), timestamp=0, dbdId=(subDbId=0, lid=0), "
                  "prevDbdId=(subDbId=0, lid=0), prevMarkedAsRemoved=false, prevTimestamp=0, serialNum=0)",
                  PutOperation().toString());
-    EXPECT_EQUAL("Put(null::, BucketId(0x000000000000002a), timestamp=10, dbdId=(subDbId=0, lid=0), "
+    EXPECT_EQUAL("Put(id::::, BucketId(0x000000000000002a), timestamp=10, dbdId=(subDbId=0, lid=0), "
                  "prevDbdId=(subDbId=0, lid=0), prevMarkedAsRemoved=false, prevTimestamp=0, serialNum=0)",
                  PutOperation(bucket_id1, timestamp, doc).toString());
 
-    EXPECT_EQUAL("Remove(null::, BucketId(0x0000000000000000), timestamp=0, dbdId=(subDbId=0, lid=0), "
+    EXPECT_EQUAL("Remove(id::::, BucketId(0x0000000000000000), timestamp=0, dbdId=(subDbId=0, lid=0), "
                  "prevDbdId=(subDbId=0, lid=0), prevMarkedAsRemoved=false, prevTimestamp=0, serialNum=0)",
                  RemoveOperation().toString());
     EXPECT_EQUAL("Remove(id:ns:foo:::bar, BucketId(0x000000000000002a), timestamp=10, dbdId=(subDbId=0, lid=0), "
diff --git a/storage/src/vespa/storage/visiting/countvisitor.cpp b/storage/src/vespa/storage/visiting/countvisitor.cpp
index a9ebe7e452a..f00b333581e 100644
--- a/storage/src/vespa/storage/visiting/countvisitor.cpp
+++ b/storage/src/vespa/storage/visiting/countvisitor.cpp
@@ -46,18 +46,10 @@ CountVisitor::handleDocuments(const document::BucketId& /*bucketId*/,
                     _groupCount[idString.getGroup()]++;
                 }
 
-                switch (idString.getType()) {
-                case document::IdString::ID:
-                    if (_doScheme) {
-                        _schemeCount["id"]++;
-                    }
-                    break;
-                case document::IdString::NULLID:
-                    if (_doScheme) {
-                        _schemeCount["null"]++;
-                    }
-                    break;
+                if (_doScheme) {
+                    _schemeCount["id"]++;
                 }
+
             }
         }
     }
diff --git a/vsm/src/tests/document/document.cpp b/vsm/src/tests/document/document.cpp
index d6e3f507034..b8175a01547 100644
--- a/vsm/src/tests/document/document.cpp
+++ b/vsm/src/tests/document/document.cpp
@@ -48,21 +48,21 @@ DocumentTest::testStorageDocument()
 
     EXPECT_EQUAL(std::string("foo"), sdoc.getField(0)->getAsString());
     EXPECT_EQUAL(std::string("bar"), sdoc.getField(1)->getAsString());
-    EXPECT_TRUE(sdoc.getField(2) == NULL);
+    EXPECT_TRUE(sdoc.getField(2) == nullptr);
     // test caching
     EXPECT_EQUAL(std::string("foo"), sdoc.getField(0)->getAsString());
     EXPECT_EQUAL(std::string("bar"), sdoc.getField(1)->getAsString());
-    EXPECT_TRUE(sdoc.getField(2) == NULL);
+    EXPECT_TRUE(sdoc.getField(2) == nullptr);
 
     // set new values
     EXPECT_TRUE(sdoc.setField(0, FieldValue::UP(new StringFieldValue("baz"))));
     EXPECT_EQUAL(std::string("baz"), sdoc.getField(0)->getAsString());
     EXPECT_EQUAL(std::string("bar"), sdoc.getField(1)->getAsString());
-    EXPECT_TRUE(sdoc.getField(2) == NULL);
+    EXPECT_TRUE(sdoc.getField(2) == nullptr);
     EXPECT_TRUE(sdoc.setField(1, FieldValue::UP(new StringFieldValue("qux"))));
     EXPECT_EQUAL(std::string("baz"), sdoc.getField(0)->getAsString());
     EXPECT_EQUAL(std::string("qux"), sdoc.getField(1)->getAsString());
-    EXPECT_TRUE(sdoc.getField(2) == NULL);
+    EXPECT_TRUE(sdoc.getField(2) == nullptr);
     EXPECT_TRUE(sdoc.setField(2, FieldValue::UP(new StringFieldValue("quux"))));
     EXPECT_EQUAL(std::string("baz"), sdoc.getField(0)->getAsString());
     EXPECT_EQUAL(std::string("qux"), sdoc.getField(1)->getAsString());
@@ -72,7 +72,7 @@ DocumentTest::testStorageDocument()
 
     SharedFieldPathMap fim;
     StorageDocument s2(std::make_unique<document::Document>(), fim, 0);
-    EXPECT_EQUAL(vespalib::string("null::"), s2.docDoc().getId().toString());
+    EXPECT_EQUAL(IdString().toString(), s2.docDoc().getId().toString());
 }
 
 void DocumentTest::testStringFieldIdTMap()