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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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: _______________________________________________ poppler mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/poppler
