Comments (4)
It looks good, have you timed resize, with a non trivial copy constructor, in the two versions?
from tpie.
I discussed this issue with Jakob, and we conclude that m_tss_used is an acceptable complexity in the code. (Using Boost type traits and SFINAE is an added complexity in itself.)
We experimented with "placement new[]" in place of the default_initialize() for-loop, and indeed it is as fast as the current code that uses m_tss_used, but it turns out that placement new[] is flawed in its design, see http://stackoverflow.com/a/15948/1570972
Since the naΓ―ve for-loop makes array::resize(memory_size_type) around 10% slower than the code using the m_tss_used flag, we choose to trade complexity for a speed-up and do not merge the patch in this issue.
from tpie.
For future reference (since I'm deleting the branch), the suggested patch (566e5a5) was the following:
commit 566e5a5ab8beed0d9961ceaea8ec7399fb32badb
Parent: 9496caa4cbd4cf8ad40e3ed21a5a1cdbc4388c4c
Author: Mathias Rav <[email protected]>
Date: Mon Aug 27 11:13:32 2012 +0200
Always allocate array::m_elements using tpie::trivially_same_size
diff --git 9496caa/tpie/array.h 566e5a5/tpie/array.h
index 5604bbb..7c35c11 100644
--- 9496caa/tpie/array.h
+++ 566e5a5/tpie/array.h
@@ -29,6 +29,8 @@
#include <boost/utility/enable_if.hpp>
#include <tpie/memory.h>
#include <tpie/array_view_base.h>
+#include <boost/type_traits.hpp>
+#include <tpie/tpie_log.h>
namespace tpie {
@@ -349,14 +351,14 @@ public:
/// \param s The number of elements in the array.
/// \param value Each entry of the array is initialized with this value.
///////////////////////////////////////////////////////////////////////////
- array_base(size_type s, const T & value): m_elements(0), m_size(0), m_tss_used(false) {resize(s, value);}
+ array_base(size_type s, const T & value): m_elements(0), m_size(0) {resize(s, value);}
///////////////////////////////////////////////////////////////////////////
/// \brief Construct array of given size.
///
/// \param s The number of elements in the array.
///////////////////////////////////////////////////////////////////////////
- array_base(size_type s=0): m_elements(0), m_size(0), m_tss_used(false) {resize(s);}
+ array_base(size_type s=0): m_elements(0), m_size(0) {resize(s);}
/////////////////////////////////////////////////////////
/// \brief Construct a copy of another array.
@@ -365,7 +367,6 @@ public:
array_base(const array_base & other): m_elements(0), m_size(other.m_size) {
if (other.size() == 0) return;
m_elements = m_size ? reinterpret_cast<T*>(tpie_new_array<trivial_same_size<T> >(m_size)) : 0;
- m_tss_used = true;
std::uninitialized_copy(other.m_elements+0, other.m_elements+m_size, m_elements+0);
}
@@ -390,7 +391,6 @@ public:
m_size = size;
m_elements = size ? reinterpret_cast<T*>(tpie_new_array<trivial_same_size<T> >(m_size)) : 0;
- m_tss_used = true;
// call copy constructors manually
std::uninitialized_fill(m_elements+0, m_elements+m_size, elm);
@@ -405,7 +405,6 @@ public:
void swap(array_base & other) {
std::swap(m_elements, other.m_elements);
std::swap(m_size, other.m_size);
- std::swap(m_tss_used, other.m_tss_used);
}
///////////////////////////////////////////////////////////////////////////
@@ -420,8 +419,8 @@ public:
void resize(size_t s) {
destruct_and_dealloc();
m_size = s;
- m_elements = m_size ? tpie_new_array<T>(m_size) : 0;
- m_tss_used = false;
+ m_elements = m_size ? reinterpret_cast<T*>(tpie_new_array<trivial_same_size<T> >(m_size)) : 0;
+ default_initialize(m_elements);
}
///////////////////////////////////////////////////////////////////////////
@@ -447,12 +446,6 @@ protected:
private:
inline void destruct_and_dealloc() {
- if (!m_tss_used) {
- // calls destructors
- tpie_delete_array(m_elements, m_size);
- return;
- }
-
// call destructors manually
for (size_t i = 0; i < m_size; ++i) {
m_elements[i].~T();
@@ -460,8 +453,22 @@ private:
tpie_delete_array(reinterpret_cast<trivial_same_size<T>*>(m_elements), m_size);
}
- // did we allocate m_elements as a trivial_same_size<T> *?
- bool m_tss_used;
+ template <typename U>
+ inline void
+ default_initialize(U *,
+ typename boost::enable_if<boost::has_trivial_constructor<U> >::type * dummy = 0) {
+ unused(dummy);
+ }
+
+ template <typename U>
+ inline void
+ default_initialize(U * m_elements,
+ typename boost::disable_if<boost::has_trivial_constructor<U> >::type * dummy = 0) {
+ unused(dummy);
+ for (memory_size_type i = 0; i < m_size; ++i) {
+ new (m_elements+i) T();
+ }
+ }
};
///////////////////////////////////////////////////////////////////////////////
from tpie.
If there is a decent speed increase I guess it makes sense to keep it.
However, maybe you could put a bit about the rationale behind using it in
the comments? The file is already pretty well documented on the
interface-level but the actual implementation can be a bit hard to parse -
especially with this added complication.
On Tue, Aug 28, 2012 at 4:43 AM, Mathias Rav [email protected]:
For future reference (since I'm deleting the branch), the suggested patch (
566e5a5 566e5a5) was the
following:commit 566e5a5ab8beed0d9961ceaea8ec7399fb32badb
Parent: 9496caa
Author: Mathias Rav [email protected]
Date: Mon Aug 27 11:13:32 2012 +0200Always allocate array::m_elements using tpie::trivially_same_size
diff --git 9496caa/tpie/array.h 566e5a5/tpie/array.hindex 5604bbb..7c35c11 100644--- 9496caa/tpie/array.h+++ 566e5a5/tpie/array.h@@ -29,6 +29,8 @@
#include <boost/utility/enable_if.hpp>
#include <tpie/memory.h>
#include <tpie/array_view_base.h>+#include <boost/type_traits.hpp>+#include <tpie/tpie_log.h>namespace tpie {
@@ -349,14 +351,14 @@ public:
/// \param s The number of elements in the array.
/// \param value Each entry of the array is initialized with this value.
///////////////////////////////////////////////////////////////////////////- array_base(size_type s, const T & value): m_elements(0), m_size(0), m_tss_used(false) {resize(s, value);}+ array_base(size_type s, const T & value): m_elements(0), m_size(0) {resize(s, value);}/////////////////////////////////////////////////////////////////////////// /// \brief Construct array of given size. /// /// \param s The number of elements in the array. ///////////////////////////////////////////////////////////////////////////- array_base(size_type s=0): m_elements(0), m_size(0), m_tss_used(false) {resize(s);}+ array_base(size_type s=0): m_elements(0), m_size(0) {resize(s);} ///////////////////////////////////////////////////////// /// \brief Construct a copy of another array.@@ -365,7 +367,6 @@ public: array_base(const array_base & other): m_elements(0), m_size(other.m_size) { if (other.size() == 0) return; m_elements = m_size ? reinterpret_cast<T*>(tpie_new_array<trivial_same_size<T> >(m_size)) : 0;- m_tss_used = true; std::uninitialized_copy(other.m_elements+0, other.m_elements+m_size, m_elements+0); }
@@ -390,7 +391,6 @@ public:
m_size = size;m_elements = size ? reinterpret_cast<T*>(tpie_new_array<trivial_same_size<T> >(m_size)) : 0;- m_tss_used = true; // call copy constructors manually std::uninitialized_fill(m_elements+0, m_elements+m_size, elm);@@ -405,7 +405,6 @@ public: void swap(array_base & other) { std::swap(m_elements, other.m_elements); std::swap(m_size, other.m_size);- std::swap(m_tss_used, other.m_tss_used); } ///////////////////////////////////////////////////////////////////////////@@ -420,8 +419,8 @@ public: void resize(size_t s) { destruct_and_dealloc(); m_size = s;- m_elements = m_size ? tpie_new_array<T>(m_size) : 0;- m_tss_used = false;+ m_elements = m_size ? reinterpret_cast<T*>(tpie_new_array<trivial_same_size<T> >(m_size)) : 0;+ default_initialize(m_elements); } ///////////////////////////////////////////////////////////////////////////@@ -447,12 +446,6 @@ protected:
private:
inline void destruct_and_dealloc() {- if (!m_tss_used) {- // calls destructors- tpie_delete_array(m_elements, m_size);- return;- }-
// call destructors manually
for (size_t i = 0; i < m_size; ++i) {
m_elements[i].~T();@@ -460,8 +453,22 @@ private:
tpie_delete_array(reinterpret_cast<trivial_same_size*>(m_elements), m_size);
}
- // did we allocate m_elements as a trivial_same_size *?- bool m_tss_used;+ template + inline void+ default_initialize(U *,+ typename boost::enable_ifboost::has_trivial_constructor::type * dummy = 0) {+ unused(dummy);+ }++ template + inline void+ default_initialize(U * m_elements,+ typename boost::disable_ifboost::has_trivial_constructor::type * dummy = 0) {+ unused(dummy);+ for (memory_size_type i = 0; i < m_size; ++i) {+ new (m_elements+i) T();+ }+ }
};///////////////////////////////////////////////////////////////////////////////
β
Reply to this email directly or view it on GitHubhttps://github.com//issues/28#issuecomment-8085041.
from tpie.
Related Issues (20)
- tuple_utils error HOT 4
- get rid of default_pred_sort_factory HOT 1
- turn factor constructed<dest_t>::type into constructed_type<dest_t>
- tpie reference not found HOT 4
- I/O efficient deque HOT 5
- Tpie rendering examples HOT 4
- internal data structures memory limit HOT 1
- I/O stack performance HOT 2
- peek_back for file_stream and peek for merge_sorter HOT 4
- Sorting of tpie::file<T>::stream HOT 2
- License exception for static linking HOT 4
- What's the status of this project? HOT 3
- tpie::priority_queue(mm_avail) not in documentation HOT 3
- Get CI back up to speed HOT 3
- merge_sorter::minimum_memory_phase_X() is lower than actual memory requirements HOT 1
- Resource memory limit exceeded for `tpie::priority_queue` HOT 2
- `file_stream::close()` does not terminate if TPIE has been Reinitialized
- `array_iter_base::advance` Should Take a `ssize_t` HOT 1
- Cannot compile tests with MSC 19.28+ HOT 2
- Cannot find the file "#include <tpie/tpie_export.h>" HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
π Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. πππ
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google β€οΈ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from tpie.