[poppler] 4 commits - poppler/Object.cc poppler/Object.h poppler/XRef.cc qt5/tests

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Thu Oct 4 07:50:26 UTC 2018


 poppler/Object.cc          |   26 ++----------------
 poppler/Object.h           |   63 ++++++++++++++++++++++-----------------------
 poppler/XRef.cc            |   12 ++++----
 qt5/tests/CMakeLists.txt   |    1 
 qt5/tests/check_object.cpp |   36 +++++++++++++++++++++++++
 5 files changed, 77 insertions(+), 61 deletions(-)

New commits:
commit 6007b32e10c6283f3d9f4eeb0cb862ecfeba239a
Author: Adam Reichold <adam.reichold at t-online.de>
Date:   Wed Oct 3 20:24:24 2018 +0200

    Inline move construction and assignment of Object to avoid function call overhead.

diff --git a/poppler/Object.cc b/poppler/Object.cc
index 36e35abe..d25cebeb 100644
--- a/poppler/Object.cc
+++ b/poppler/Object.cc
@@ -59,22 +59,6 @@ static const char *objTypeNames[numObjTypes] = {
   "dead"
 };
 
-Object::Object(Object&& other)
-{
-  std::memcpy(reinterpret_cast<void*>(this), &other, sizeof(Object));
-  other.type = objDead;
-}
-
-Object& Object::operator=(Object&& other)
-{
-  free();
-
-  std::memcpy(reinterpret_cast<void*>(this), &other, sizeof(Object));
-  other.type = objDead;
-
-  return *this;
-}
-
 Object Object::copy() const {
   CHECK_NOT_DEAD;
 
diff --git a/poppler/Object.h b/poppler/Object.h
index 231c1453..0b856d3d 100644
--- a/poppler/Object.h
+++ b/poppler/Object.h
@@ -182,8 +182,21 @@ public:
 
   template<typename T> Object(T) = delete;
 
-  Object(Object&& other);
-  Object& operator=(Object&& other);
+  Object(Object&& other)
+  {
+    std::memcpy(reinterpret_cast<void*>(this), &other, sizeof(Object));
+    other.type = objDead;
+  }
+
+  Object& operator=(Object&& other)
+  {
+    free();
+
+    std::memcpy(reinterpret_cast<void*>(this), &other, sizeof(Object));
+    other.type = objDead;
+
+    return *this;
+  }
 
   Object &operator=(const Object &other) = delete;
   Object(const Object &other) = delete;
commit 5e5e7234c06bc2aa327a1461419f15de4a21b563
Author: Adam Reichold <adam.reichold at t-online.de>
Date:   Wed Oct 3 16:53:59 2018 +0200

    Add microbenchmark to check effect of using memcpy instead of assignment.

diff --git a/qt5/tests/CMakeLists.txt b/qt5/tests/CMakeLists.txt
index 7dbad8b4..af8de6ef 100644
--- a/qt5/tests/CMakeLists.txt
+++ b/qt5/tests/CMakeLists.txt
@@ -70,6 +70,7 @@ qt5_add_qtest(check_qt5_search check_search.cpp)
 qt5_add_qtest(check_qt5_actualtext check_actualtext.cpp)
 qt5_add_qtest(check_qt5_lexer check_lexer.cpp)
 qt5_add_qtest(check_qt5_goostring check_goostring.cpp)
+qt5_add_qtest(check_qt5_object check_object.cpp)
 qt5_add_qtest(check_qt5_utf_conversion check_utf_conversion.cpp)
 if (NOT WIN32)
   qt5_add_qtest(check_qt5_pagelabelinfo check_pagelabelinfo.cpp)
diff --git a/qt5/tests/check_object.cpp b/qt5/tests/check_object.cpp
new file mode 100644
index 00000000..9ebce92a
--- /dev/null
+++ b/qt5/tests/check_object.cpp
@@ -0,0 +1,36 @@
+#include <QtCore/QScopedPointer>
+#include <QtTest/QtTest>
+
+#include "poppler/Object.h"
+
+class TestObject : public QObject
+{
+    Q_OBJECT
+private slots:
+    void benchDefaultConstructor();
+    void benchMoveConstructor();
+    void benchSetToNull();
+};
+
+void TestObject::benchDefaultConstructor() {
+  QBENCHMARK {
+    Object obj;
+  }
+}
+
+void TestObject::benchMoveConstructor() {
+  Object src;
+  QBENCHMARK {
+    Object dst{std::move(src)};
+  }
+}
+
+void TestObject::benchSetToNull() {
+  Object obj;
+  QBENCHMARK {
+    obj.setToNull();
+  }
+}
+
+QTEST_GUILESS_MAIN(TestObject)
+#include "check_object.moc"
commit 64fec200816005b6c324f455446ea2ca290ffb87
Author: Adam Reichold <adam.reichold at t-online.de>
Date:   Sat Sep 22 11:13:39 2018 +0200

    Avoid Object being too friendly with Array, Dict and XRef as these can just use placement-new and the usual destructor which is public anyways.

diff --git a/poppler/Object.cc b/poppler/Object.cc
index 5f0bfd15..36e35abe 100644
--- a/poppler/Object.cc
+++ b/poppler/Object.cc
@@ -75,11 +75,6 @@ Object& Object::operator=(Object&& other)
   return *this;
 }
 
-Object::~Object()
-{
-  free();
-}
-
 Object Object::copy() const {
   CHECK_NOT_DEAD;
 
diff --git a/poppler/Object.h b/poppler/Object.h
index 390c21e2..231c1453 100644
--- a/poppler/Object.h
+++ b/poppler/Object.h
@@ -154,10 +154,8 @@ constexpr int numObjTypes = 16;		// total number of object types
 
 class Object {
 public:
-  // Default constructor.
-  Object():
-    type(objNone) {}
-  ~Object();
+  Object() : type(objNone) {}
+  ~Object() { free(); }
 
   explicit Object(GBool boolnA)
     { type = objBool; booln = boolnA; }
@@ -296,16 +294,9 @@ public:
   void print(FILE *f = stdout) const;
 
 private:
-  friend class Array; // Needs free and initNullAfterMalloc
-  friend class Dict; // Needs free and initNullAfterMalloc
-  friend class XRef; // Needs free and initNullAfterMalloc
-
   // Free object contents.
   void free();
 
-  // Only use if are mallocing Objects
-  void initNullAfterMalloc() { type = objNull; }
-
   ObjType type;			// object type
   union {			// value for each type:
     GBool booln;		//   boolean
diff --git a/poppler/XRef.cc b/poppler/XRef.cc
index 7b492491..56fa2b6a 100644
--- a/poppler/XRef.cc
+++ b/poppler/XRef.cc
@@ -366,7 +366,7 @@ XRef::XRef(BaseStream *strA, Goffset pos, Goffset mainXRefEntriesOffsetA, GBool
 
 XRef::~XRef() {
   for(int i=0; i<size; i++) {
-      entries[i].obj.free ();
+      entries[i].obj.~Object();
   }
   gfree(entries);
 
@@ -415,7 +415,7 @@ XRef *XRef::copy() const {
   for (int i = 0; i < size; ++i) {
     xref->entries[i].offset = entries[i].offset;
     xref->entries[i].type = entries[i].type;
-    xref->entries[i].obj.initNullAfterMalloc();
+    new (&xref->entries[i].obj) Object(objNull);
     xref->entries[i].flags = entries[i].flags;
     xref->entries[i].gen = entries[i].gen;
   }
@@ -464,13 +464,13 @@ int XRef::resize(int newSize)
     for (int i = size; i < newSize; ++i) {
       entries[i].offset = -1;
       entries[i].type = xrefEntryNone;
-      entries[i].obj.initNullAfterMalloc ();
+      new (&entries[i].obj) Object(objNull);
       entries[i].flags = 0;
       entries[i].gen = 0;
     }
   } else {
     for (int i = newSize; i < size; i++) {
-      entries[i].obj.free ();
+      entries[i].obj.~Object();
     }
   }
 
@@ -1327,7 +1327,7 @@ void XRef::add(int num, int gen, Goffset offs, GBool used) {
     for (int i = size; i < num + 1; ++i) {
       entries[i].offset = -1;
       entries[i].type = xrefEntryFree;
-      entries[i].obj.initNullAfterMalloc();
+      new (&entries[i].obj) Object(objNull);
       entries[i].flags = 0;
       entries[i].gen = 0;
     }
@@ -1399,7 +1399,7 @@ void XRef::removeIndirectObject(Ref r) {
   if (e->type == xrefEntryFree) {
     return;
   }
-  e->obj.free();
+  e->obj.~Object();
   e->type = xrefEntryFree;
   e->gen++;
   e->setFlag(XRefEntry::Updated, gTrue);
commit cb29b28da31e3f563c46fd596812a7860d52ab8f
Author: Adam Reichold <adam.reichold at t-online.de>
Date:   Sat Sep 22 11:07:05 2018 +0200

    Remove unneccessary macros from Object, avoid zeroing uninitialized objects and avoid copying inactive union members.

diff --git a/poppler/Object.cc b/poppler/Object.cc
index 9112d316..5f0bfd15 100644
--- a/poppler/Object.cc
+++ b/poppler/Object.cc
@@ -61,17 +61,17 @@ static const char *objTypeNames[numObjTypes] = {
 
 Object::Object(Object&& other)
 {
-  type = other.type;
-  real = other.real; // this is the biggest of the union so it's enough
+  std::memcpy(reinterpret_cast<void*>(this), &other, sizeof(Object));
   other.type = objDead;
 }
 
 Object& Object::operator=(Object&& other)
 {
   free();
-  type = other.type;
-  real = other.real; // this is the biggest of the union so it's enough
+
+  std::memcpy(reinterpret_cast<void*>(this), &other, sizeof(Object));
   other.type = objDead;
+
   return *this;
 }
 
@@ -84,8 +84,8 @@ Object Object::copy() const {
   CHECK_NOT_DEAD;
 
   Object obj;
-  obj.type = type;
-  obj.real = real; // this is the biggest of the union so it's enough
+  std::memcpy(reinterpret_cast<void*>(&obj), this, sizeof(Object));
+
   switch (type) {
   case objString:
     obj.string = string->copy();
@@ -106,6 +106,7 @@ Object Object::copy() const {
   default:
     break;
   }
+
   return obj;
 }
 
diff --git a/poppler/Object.h b/poppler/Object.h
index 392b80d6..390c21e2 100644
--- a/poppler/Object.h
+++ b/poppler/Object.h
@@ -146,47 +146,42 @@ enum ObjType {
   objDead			// and object after shallowCopy
 };
 
-#define numObjTypes 16		// total number of object types
+constexpr int numObjTypes = 16;		// total number of object types
 
 //------------------------------------------------------------------------
 // Object
 //------------------------------------------------------------------------
 
-#define initObj(t) free(); zeroUnion(); type = t
-#define constructObj(t) type = t
-
 class Object {
 public:
-  // clear the anonymous union as best we can -- clear at least a pointer
-  void zeroUnion() { this->cString = nullptr; }
-
   // Default constructor.
   Object():
-    type(objNone) { zeroUnion(); }
+    type(objNone) {}
   ~Object();
 
   explicit Object(GBool boolnA)
-    { constructObj(objBool); booln = boolnA; }
+    { type = objBool; booln = boolnA; }
   explicit Object(int intgA)
-    { constructObj(objInt); intg = intgA; }
+    { type = objInt; intg = intgA; }
   explicit Object(ObjType typeA)
-    { constructObj(typeA); }
+    { type = typeA; }
   explicit Object(double realA)
-    { constructObj(objReal); real = realA; }
+    { type = objReal; real = realA; }
   explicit Object(GooString *stringA)
-    { assert(stringA); constructObj(objString); string = stringA; }
+    { assert(stringA); type = objString; string = stringA; }
   Object(ObjType typeA, const char *stringA)
-    { assert(typeA == objName || typeA == objCmd); assert(stringA); constructObj(typeA); cString = copyString(stringA); }
+    { assert(typeA == objName || typeA == objCmd); assert(stringA); type = typeA; cString = copyString(stringA); }
   explicit Object(long long int64gA)
-    { constructObj(objInt64); int64g = int64gA; }
+    { type = objInt64; int64g = int64gA; }
   explicit Object(Array *arrayA)
-    { assert(arrayA); constructObj(objArray); array = arrayA; }
+    { assert(arrayA); type = objArray; array = arrayA; }
   explicit Object(Dict *dictA)
-    { assert(dictA); constructObj(objDict); dict = dictA; }
+    { assert(dictA); type = objDict; dict = dictA; }
   explicit Object(Stream *streamA)
-    { assert(streamA); constructObj(objStream); stream = streamA; }
+    { assert(streamA); type = objStream; stream = streamA; }
   Object(int numA, int genA)
-    { constructObj(objRef); ref.num = numA; ref.gen = genA; }
+    { type = objRef; ref.num = numA; ref.gen = genA; }
+
   template<typename T> Object(T) = delete;
 
   Object(Object&& other);
@@ -196,7 +191,7 @@ public:
   Object(const Object &other) = delete;
 
   // Set object to null.
-  void setToNull() { initObj(objNull); }
+  void setToNull() { free(); type = objNull; }
 
   // Copy this to obj
   Object copy() const;
@@ -309,7 +304,7 @@ private:
   void free();
 
   // Only use if are mallocing Objects
-  void initNullAfterMalloc() { constructObj(objNull); }
+  void initNullAfterMalloc() { type = objNull; }
 
   ObjType type;			// object type
   union {			// value for each type:


More information about the poppler mailing list