Change in kio[master]: Store the UDS field number in the Field struct. On x86 this ...

Mark Gaiser (Code Review) noreply at kde.org
Sun Jul 19 20:35:05 UTC 2015


Mark Gaiser has uploaded a new change for review.

  https://gerrit.vesnicky.cesnet.cz/r/473

Change subject: Store the UDS field number in the Field struct. On x86 this is free because memory wasn't optimally used. Before: sizeof(Field) = 16 After: sizeof(Field) = 16
......................................................................

Store the UDS field number in the Field struct. On x86 this is free because memory wasn't optimally used.
Before: sizeof(Field) = 16
After: sizeof(Field) = 16

On x64 this makes the Field sizeof slightly larger.
Before: sizeof(Field) = 16
After: sizeof(Field) = 24

The difference here is QString taking 4 bytes on x86 and 8 on x64.

STL Algorithms are now used to determine if a UDS is already inserted.
Performance wise this is marginally faster. Memory wise there isn't any difference.

This change also removes the bookkeeping where fields and udsIndexes had to be kept in sync.

CHANGELOG: Store UDS value in Field struct.

Change-Id: Ifecd09a2a788353ca85eae83c59f9de8e24b278f
---
M src/core/udsentry.cpp
1 file changed, 59 insertions(+), 35 deletions(-)


  git pull ssh://gerrit.vesnicky.cesnet.cz:29418/kio refs/changes/73/473/1

diff --git a/src/core/udsentry.cpp b/src/core/udsentry.cpp
index 96d00d3..3efb45e 100644
--- a/src/core/udsentry.cpp
+++ b/src/core/udsentry.cpp
@@ -29,6 +29,8 @@
 
 #include <KUser>
 
+#include <algorithm>
+
 using namespace KIO;
 
 /* ---------- UDSEntry ------------ */
@@ -37,26 +39,48 @@
 {
 public:
     struct Field {
-        inline Field(const QString &value) : m_str(value), m_long(0) {}
-        inline Field(long long value = 0) : m_long(value) { }
+        inline Field(uint uds, const QString &value) : m_str(value), m_udsIndex(uds), m_long(0) {}
+        inline Field(uint uds = 0, long long value = 0) : m_udsIndex(uds), m_long(value) { }
         QString m_str;
+        uint m_udsIndex;
         long long m_long;
     };
 
     QVector<Field> fields;
 
-    // If udsIndexes[i] == uds, then fields[i] contains the value for 'uds'. Example:
-    // udsIndexes = {UDS_NAME, UDS_FILE_SIZE, ...}
-    // fields = {Field("filename"), Field(1234), ...}
-    QVector<uint> udsIndexes;
-
-    void insert(uint uds, const Field& field)
+    // Gets the UDS fields and puts them in a vector.
+    const QVector<uint> udsFields() const
     {
-        const int index = udsIndexes.indexOf(uds);
-        if (index >= 0) {
-            fields[index] = field;
-        } else {
-            udsIndexes.append(uds);
+        QVector<uint> udsVector;
+        foreach (const Field &field, fields) {
+            udsVector.push_back(field.m_udsIndex);
+        }
+
+        return udsVector;
+    }
+
+    // Returns true if the UDS exists, false otherwise.
+    bool contains(uint uds) const
+    {
+        auto result = std::find_if(fields.constBegin(), fields.constEnd(), [&uds](const Field &f){
+            return f.m_udsIndex == uds;
+        });
+
+        return result != fields.constEnd();
+    }
+
+    void insert(const Field& field)
+    {
+        bool replacedField = false;
+        const uint uds = field.m_udsIndex;
+
+        std::replace_if(fields.begin(), fields.end(), [&uds, &replacedField](const Field &f){
+            replacedField = f.m_udsIndex == uds;
+            return replacedField;
+        }, field);
+
+        // If the field isn't replaced yet, it wasn't found so add it.
+        if (!replacedField) {
             fields.append(field);
         }
     }
@@ -105,9 +129,12 @@
 
 QString UDSEntry::stringValue(uint field) const
 {
-    const int index = d->udsIndexes.indexOf(field);
-    if (index >= 0) {
-        return d->fields.at(index).m_str;
+    auto result = std::find_if(d->fields.constBegin(), d->fields.constEnd(), [&field](const UDSEntryPrivate::Field &f){
+        return f.m_udsIndex == field;
+    });
+
+    if (result != d->fields.constEnd()) {
+        return result->m_str;
     } else {
         return QString();
     }
@@ -115,9 +142,12 @@
 
 long long UDSEntry::numberValue(uint field, long long defaultValue) const
 {
-    const int index = d->udsIndexes.indexOf(field);
-    if (index >= 0) {
-        return d->fields.at(index).m_long;
+    auto result = std::find_if(d->fields.constBegin(), d->fields.constEnd(), [&field](const UDSEntryPrivate::Field &f){
+        return f.m_udsIndex == field;
+    });
+
+    if (result != d->fields.constEnd()) {
+        return result->m_long;
     } else {
         return defaultValue;
     }
@@ -136,45 +166,43 @@
 void UDSEntry::reserve(int size)
 {
     d->fields.reserve(size);
-    d->udsIndexes.reserve(size);
 }
 
 void UDSEntry::insert(uint field, const QString &value)
 {
-    d->insert(field, UDSEntryPrivate::Field(value));
+    d->insert(UDSEntryPrivate::Field(field, value));
 }
 
 void UDSEntry::insert(uint field, long long value)
 {
-    d->insert(field, UDSEntryPrivate::Field(value));
+    d->insert(UDSEntryPrivate::Field(field, value));
 }
 
 #ifndef KIOCORE_NO_DEPRECATED
 QList<uint> UDSEntry::listFields() const
 {
-    return d->udsIndexes.toList();
+    return fields().toList();
 }
 #endif
 
 QVector<uint> UDSEntry::fields() const
 {
-    return d->udsIndexes;
+    return d->udsFields();
 }
 
 int UDSEntry::count() const
 {
-    return d->udsIndexes.count();
+    return d->fields.count();
 }
 
 bool UDSEntry::contains(uint field) const
 {
-    return d->udsIndexes.contains(field);
+    return d->contains(field);
 }
 
 void UDSEntry::clear()
 {
     d->fields.clear();
-    d->udsIndexes.clear();
 }
 
 QDataStream &operator<<(QDataStream &s, const UDSEntry &a)
@@ -191,14 +219,13 @@
 
 void UDSEntryPrivate::save(QDataStream &s, const UDSEntry &a)
 {
-    const QVector<uint> &udsIndexes = a.d->udsIndexes;
     const QVector<Field> &fields = a.d->fields;
-    const int size = udsIndexes.size();
+    const int size = fields.size();
 
     s << size;
 
     for (int index = 0; index < size; ++index) {
-        uint uds = udsIndexes.at(index);
+        uint uds = fields.at(index).m_udsIndex;
         s << uds;
 
         if (uds & KIO::UDSEntry::UDS_STRING) {
@@ -216,12 +243,10 @@
     a.clear();
 
     QVector<Field> &fields = a.d->fields;
-    QVector<uint> &udsIndexes = a.d->udsIndexes;
 
     quint32 size;
     s >> size;
     fields.reserve(size);
-    udsIndexes.reserve(size);
 
     // We cache the loaded strings. Some of them, like, e.g., the user,
     // will often be the same for many entries in a row. Caching them
@@ -234,7 +259,6 @@
     for (quint32 i = 0; i < size; ++i) {
         quint32 uds;
         s >> uds;
-        udsIndexes.append(uds);
 
         if (uds & KIO::UDSEntry::UDS_STRING) {
             // If the QString is the same like the one we read for the
@@ -247,11 +271,11 @@
                 cachedStrings[i] = buffer;
             }
 
-            fields.append(Field(cachedStrings.at(i)));
+            fields.append(Field(uds, cachedStrings.at(i)));
         } else if (uds & KIO::UDSEntry::UDS_NUMBER) {
             long long value;
             s >> value;
-            fields.append(Field(value));
+            fields.append(Field(uds, value));
         } else {
             Q_ASSERT_X(false, "KIO::UDSEntry", "Found a field with an invalid type");
         }

-- 
To view, visit https://gerrit.vesnicky.cesnet.cz/r/473
To unsubscribe, visit https://gerrit.vesnicky.cesnet.cz/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifecd09a2a788353ca85eae83c59f9de8e24b278f
Gerrit-PatchSet: 1
Gerrit-Project: kio
Gerrit-Branch: master
Gerrit-Owner: Mark Gaiser <markg85 at gmail.com>


More information about the Kde-frameworks-devel mailing list