tag:blogger.com,1999:blog-31974345721087896282024-03-13T11:33:53.562-07:00c++ today"Almost" day by day thoughts on code inspectedGaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.comBlogger22125tag:blogger.com,1999:blog-3197434572108789628.post-84961468042637797592020-01-24T15:16:00.000-08:002020-01-29T07:31:45.256-08:00The Unbearable Lightness of a LambdaI believe that one of the game change features in c++ were the introduction of lambdas I also believe that performance-wise lambda are underestimated, the reason for not considering the cost of a lambda is the fact that are considered just as functions.<br />
Given the following code <br />
<br />
<script src="https://gist.github.com/kalman5/ec5dc4f1452389439cbf2d8bb9c5d5eb.js"></script>
And playing the compiler game this is what happens:<br />
<br />
<script src="https://gist.github.com/kalman5/50227b349d01ca73ba61b39bb794006f.js"></script>
Now it's clear that a lambda has a state.
With the help of godbolt looks like that if the captured amount of memory exceeds 16 bytes both compilers I have tried (gcc and clang) perform an allocation on the heap.
Let see how we can avoid this madness. <br />
Let's imagine we have some pieces of code that we need to execute after a preamble and before a postamble, something like the following: <br />
<br />
<script src="https://gist.github.com/kalman5/6f0385e53250e7e01a65b990e9749339.js"></script>
This is the generated code and you can observe the heap allocation made to store the lambda "internal state"<br />
<br />
<div class="separator" style="clear: both; text-align: center;">
<a href="https://3.bp.blogspot.com/-Cww5AqciuOk/Xitkttbr-HI/AAAAAAAAW1s/OqysDANNUjQRsvBbQkvM0ZM-oTZKetpDACLcBGAsYHQ/s1600/HeapAllocation.png" imageanchor="1" style="margin-left: 1em; margin-right: 1em;"><img border="0" data-original-height="321" data-original-width="864" height="237" src="https://3.bp.blogspot.com/-Cww5AqciuOk/Xitkttbr-HI/AAAAAAAAW1s/OqysDANNUjQRsvBbQkvM0ZM-oTZKetpDACLcBGAsYHQ/s640/HeapAllocation.png" width="640" /></a></div>
<br />
In this case, we can completely dodge the capture and save us some headaches avoiding the heap allocation when the lambda is converted to that std::function, we can make the function template on the lambda and avoid the std::function conversion.
And this is what we get<br /><br />
<script src="https://gist.github.com/kalman5/d32d2fbcec54825cd6299ecd95f65611.js"></script><br />
We have another issue now, PrePost function is not generic (even if it seems).<br />
Let's make it generic, as it is indeed doesn't work for example with mutable lambdas, the following code, for instance, does not compile:<br />
<br />
<script src="https://gist.github.com/kalman5/ff5d67128fc08d16afe0766c18a3b304.js"></script>
We need to get the lambda as a Universal Reference, that is:<br />
<br />
<script src="https://gist.github.com/kalman5/8cf9f0dd61c968aefa0c13525a559b04.js"></script>
We are not done yet with PrePost indeed as it is can also get callable objects but it doesn't behave well with callable objects with <a href="https://www.blogger.com/%3Ca%20href=%22http://cpp-today.blogspot.com/2014/03/ref-qualifiers.html%22%3E%3C/a%3E" target="_blank">"ref-qualified methods"</a> such as:<br /><br />
<script src="https://gist.github.com/kalman5/30b8d00f3f56a6eb7cc2be187dd0ef5d.js"></script>
As it is our PrePost function is bugged, it compiles but doesn't do what we expect, in the following code the r-value operator is expected to be used but is not<br /><br />
<script src="https://gist.github.com/kalman5/9ec61b77cbb79982ab86f3e602d1818c.js"></script>
In order to fix the issue we need to "perfect forward" the function/lambda, this is the last version of PrePost that covers all cases<br /><br />
<script src="https://gist.github.com/kalman5/fad4d27ac7e1e81064dde31caad928dc.js"></script>
Similar links<br /><br />
<a href="https://blog.demofox.org/2015/02/25/avoiding-the-performance-hazzards-of-stdfunction/">Avoid the performance hazzard of std::function</a><br />
<a href="https://www.drdobbs.com/cpp/efficient-use-of-lambda-expressions-and/232500059">Efficient use of lambda expressions</a>
Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com2tag:blogger.com,1999:blog-3197434572108789628.post-41667175804237117892017-05-08T14:39:00.000-07:002018-08-02T05:31:24.832-07:00Thread Collisions detector - Fake Mutex<script async="" src="//pagead2.googlesyndication.com/pagead/js/adsbygoogle.js"></script>
<script>
(adsbygoogle = window.adsbygoogle || []).push({
google_ad_client: "ca-pub-3937540536619226",
enable_page_level_ads: true
});
</script>
Some years ago I found myself wondering if a not supposed thread safe class was being used by multiple threads without being synchronized. In that occasion I wrote about it here <a href="http://cpp-today.blogspot.ch/2008/06/threading-mess.html">Threading mess!</a> and here <a href="http://cpp-today.blogspot.ch/2008/06/threading-mess-2.html">Threading mess (2)!</a>.
<br />
At that time (9 years ago) we had no threads neither atomic in the c++ standard and the solution proposed was based on pthreads and on gcc atomic builtins.
I think it's time to refresh the implementation using some C++11 features.
<br />
The idea is very simple, upon entering a critical section (part of code that should not be executed concurrently) we should save the current thread id resetting the stored value as soon the thread leaves the critical section. If a thread tries to enter a critical section but we already have a thread id saved then we have detected the collision.
<br />
The technique is very effective and at that time I wrote for the Chromium project the class ThreadCollisionWarner <a href="https://chromium.googlesource.com/chromium/chromium/+/master/base/threading/thread_collision_warner.h">thread_collision_warner.h</a> and <a href="https://chromium.googlesource.com/chromium/chromium/+/master/base/threading/thread_collision_warner.cc">thread_collision_warner.cc</a> using the described technique.
<br />
Basically what you need to do is to add to your classes a "FakeMutex" and then "locking" it where it's needed as you would do with a real mutex. It's called Fake Mutex because it will not suspend a thread if another one is active but it will assert(false) instead.
If you want to use this technique in your project I suggest to use the implementation done in Chromium.
<br />
<br />
Examples of uses: <br />
<br />
<script src="https://gist.github.com/kalman5/d14b99a1318e2eb088ade54ea9c2e80c.js"></script>
the macros DFAKE_MUTEX, DFAKE_SCOPED_LOCK, DFAKE_SCOPED_RECURSIVE_LOCK and DFAKE_SCOPED_LOCK_THREAD_LOCKED are defined only if compiled in DEBUG mode removing from your production code the atomic overhead.
<br />
<br />
The modern simplified version of Chromium <b>ThreadCollisionWarner</b> proposed in <a href="http://cpp-today.blogspot.ch/2008/06/threading-mess-2.html">Threading mess (2)!</a> is reported here.
<br />
<br />
<script src="https://gist.github.com/kalman5/cf619a82efaae22db9efb2a1e02220d6.js"></script>Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com2tag:blogger.com,1999:blog-3197434572108789628.post-82333346732689366652017-03-31T09:00:00.000-07:002018-08-02T05:31:54.380-07:00Structured Binding (C++17 inside)<script async src="//pagead2.googlesyndication.com/pagead/js/adsbygoogle.js"></script>
<script>
(adsbygoogle = window.adsbygoogle || []).push({
google_ad_client: "ca-pub-3937540536619226",
enable_page_level_ads: true
});
</script>
Let's see how structured binding introduced in C++17 will change the way to interact with std::pair, std::tuple, std::array and such:
<br />
<pre style="background: #f0f0f0; border: 1px dashed #cccccc; color: black; font-family: "arial"; font-size: 12px; height: auto; line-height: 20px; overflow: auto; padding: 0px; text-align: left; width: 99%;"><code style="color: black; word-wrap: normal;"> std::pair<int,float> foo();
auto [a,b] = foo();
</code></pre>
it will replaces:
<br />
<pre style="background: #f0f0f0; border: 1px dashed #cccccc; color: black; font-family: "arial"; font-size: 12px; height: auto; line-height: 20px; overflow: auto; padding: 0px; text-align: left; width: 99%;"><code style="color: black; word-wrap: normal;"> std::pair<int,float> foo();
int a;
float b;
std::tie(a,b) = foo();
</code></pre>
in case you are running an obfuscated code contest this new swap can scrub up well (please don't try this at home):
<br />
<pre style="background: #f0f0f0; border: 1px dashed #cccccc; color: black; font-family: "arial"; font-size: 12px; height: auto; line-height: 20px; overflow: auto; padding: 0px; text-align: left; width: 99%;"><code style="color: black; word-wrap: normal;"> std::tie(a,b) = std::make_pair(b,a);
</code></pre>
The decomposition works with c array and std::array as well:
<br />
<pre style="background: #f0f0f0; border: 1px dashed #cccccc; color: black; font-family: "arial"; font-size: 12px; height: auto; line-height: 20px; overflow: auto; padding: 0px; text-align: left; width: 99%;"><code style="color: black; word-wrap: normal;"> int a[4] = { 1, 2, 3, 4};
auto [b,c,d,e] = a; </code></pre>
<pre style="background: #f0f0f0; border: 1px dashed #cccccc; color: black; font-family: "arial"; font-size: 12px; height: auto; line-height: 20px; overflow: auto; padding: 0px; text-align: left; width: 99%;"><code style="color: black; word-wrap: normal;"> std::array<int, 4> a;
auto [b,c,d,e] = a;
</code></pre>
and this is what you can do using a ranged for loop:
<br />
<pre style="background: #f0f0f0; border: 1px dashed #cccccc; color: black; font-family: "arial"; font-size: 12px; height: auto; line-height: 20px; overflow: auto; padding: 0px; text-align: left; width: 99%;"><code style="color: black; word-wrap: normal;"> std::map myMap;
...
for (const auto & [k,v] : myMap) {
}
</code></pre>
I bet someone in c++ committee has become recently a python enthusiast.
Now if you wonder what your structures like this:
<br />
<pre style="background: #f0f0f0; border: 1px dashed #cccccc; color: black; font-family: "arial"; font-size: 12px; height: auto; line-height: 20px; overflow: auto; padding: 0px; text-align: left; width: 99%;"><code style="color: black; word-wrap: normal;"> stuct X {
int theInt = 3;
float thePi = 3.14;
};
auto [a,b] = x;
</code></pre>
shall provide to make the decomposition working the response is: a plain nothing. That will work indeed off the shelf.<br />
<br />
Unfortunately if you need to do something more fancy with your class it has to support the get<>() functions, and you need to reopen the std namespace to specialize std::tuple_size and std::tuple_element.<br />
<br />
Given the following user defined type (note a and b here are private members):
<br />
<pre style="background: #f0f0f0; border: 1px dashed #cccccc; color: black; font-family: "arial"; font-size: 12px; height: auto; line-height: 20px; overflow: auto; padding: 0px; text-align: left; width: 99%;"><code style="color: black; word-wrap: normal;"> class Y {
public:
int foo() const {
return a;
}
float bar() const {
return b;
}
private:
int a = 3;
float b = 3.14;
};
</code></pre>
you need to provide the gets<>() functions:
<br />
<pre style="background: #f0f0f0; border: 1px dashed #cccccc; color: black; font-family: "arial"; font-size: 12px; height: auto; line-height: 20px; overflow: auto; padding: 0px; text-align: left; width: 99%;"><code style="color: black; word-wrap: normal;"> template <int N> auto get(Y const &);
template <> auto get<0>(Y const & aY) {
return aY.foo();
}
template <> auto get<1>(Y const & aY) {
return aY.bar();
}
</code></pre>
and then you need to reopen the std namespace (one of those few allowed cases):
<br />
<pre style="background: #f0f0f0; border: 1px dashed #cccccc; color: black; font-family: "arial"; font-size: 12px; height: auto; line-height: 20px; overflow: auto; padding: 0px; text-align: left; width: 99%;"><code style="color: black; word-wrap: normal;"> namespace std {
template<>
struct std::tuple_size<Y> {
static const size_t value = 2;
};
template<size_t I>
struct std::tuple_element<I, Y> {
using type = decltype(get<I>(declval<Y>()));
};
}
</code></pre>
Note the partial specialization for std::tuple_element, you don't need to hard code the type of each index, it's enough to "deduce" it using the get function. You did a lot of work in order to have your class supporting the decomposition, in this case c++17 can save you some work taking advantage of a new c++17 feature, the "constexpr if", just writing a single version of get<>():
<br />
<div>
<br /></div>
<pre style="background: #f0f0f0; border: 1px dashed #cccccc; color: black; font-family: "arial"; font-size: 12px; height: auto; line-height: 20px; overflow: auto; padding: 0px; text-align: left; width: 99%;"><code style="color: black; word-wrap: normal;"> template<int N>
auto get(Y const & aY) {
static_assert(N==0 || N==1);
if constexpr (N == 0) {
return aY.foo();
} else if constexpr (N == 1) {
return aY.bar();
}
}
</code></pre>
If you want use/experiment with those new language features go for clang++ (I tried only version 5.0 but it should work with the 4.0 as well) and you need to specify -std=gnu++1z
Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com3tag:blogger.com,1999:blog-3197434572108789628.post-47838542295799276082017-03-18T01:00:00.000-07:002017-03-18T01:00:01.782-07:00Concepts!Let's start from something very easy, you have the simple function:<br />
<br />
<script src="https://gist.github.com/kalman5/1e1b39fea9b8248809d4ed710846a39a.js"></script>
soon you realize you need another one but for float and before you even need the third one you write it like this:<br />
<br />
<script src="https://gist.github.com/kalman5/28a717a415dcc41231a097ee4dbc0160.js"></script>
looks like you are done, everything goes well until, while compiling your huge project, the compiler gives the following error:<br />
<br />
<script src="https://gist.github.com/kalman5/ed62a3b5276add3fd848b373c4dd5457.js"></script>
right, you think, the type Point needs to have defined the operator+, after defining it and after some precious minutes you get now
another error:<br />
<br />
<script src="https://gist.github.com/kalman5/3e153aabb32d188dfead5fa62a278c6d.js"></script>
and finally this is the last error and fixing it fixes your whole compilation. <br />
<br />
Sometime that Point type is defined in an header that makes your entire project to recompile every time you add a missing feature.<br />
<br />
Let see how can <b>concepts</b> can save our time and some headache.<br />
<br />
Basically that sumScale function has a strong requirement on the type T. It should be a summable type (it has to support the operator+) and it should be scalable (it has to support operator* with an int), we can express these two concepts in the following way:<br />
<br />
<script src="https://gist.github.com/kalman5/7687e220e4e705654427c60bdd4c051b.js"></script>
and then use this defined concept rewriting the sumScale function:<br />
<br />
<script src="https://gist.github.com/kalman5/656fbf6dae8148f97c0a0c314a0d9849.js"></script>
doing so the error would have been a more useful one:<br />
<br />
<script src="https://gist.github.com/kalman5/3971204a123b976b053b978d7fd94112.js"></script>
wow, within a single iteration the compiler was able to gives us all the information we needed in order to fix the issue.
In case you missed it I'll report for convenience the old and the new version of sumScale function.<br />
<br />
<script src="https://gist.github.com/kalman5/29280ef464723e249e8d88af3fc5d575.js"></script>
and this is, in my humble opinion, one of the main advantages of concepts: simplify the generic programming taking rid of the cumbersome template syntax.<br />
<br />
Let's go back now to our concept:<br />
<br />
<script src="https://gist.github.com/kalman5/7687e220e4e705654427c60bdd4c051b.js"></script>
this concept is the refining of a Scalable concept indeed we can define the SummableScalable concept writing first a Summable concept then refining it in the following way:<br />
<br />
<script src="https://gist.github.com/kalman5/244ba0e34543cb174744a4faae15b932.js"></script>
even better we can combine two concepts Summale + Scalable obtaining a third one:<br />
<br />
<script src="https://gist.github.com/kalman5/c4e70d7ae11b2eefb26d6985592e4d1b.js"></script>
I believe when we will get the concepts available on our preferred compiler, for instance the concepts <a href="http://http//honermann.net/blog/2016/03/06/why-concepts-didnt-make-cxx17/">didn't make C++17</a> and today (at my knowledge) the concepts are implemented only in GCC 6 (using the flag -fconcepts), it will change the face of c++ code even more the c++11 specification did.<br />
<br />
<br />Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com2Lugano, Switzerland46.0036778 8.951052000000004245.8272878 8.6283285000000038 46.180067799999996 9.2737755000000046tag:blogger.com,1999:blog-3197434572108789628.post-8643335440653593902014-06-20T13:31:00.003-07:002014-06-20T13:32:06.029-07:00Deal with OOM conditionsImagine the following sci-fi scenario: your code is in the middle of aborting a nuclear missiles launch fired by mistake, it needs to allocate some memory and unfortunately the BOFH is using all the physical and virtual memory because, he just can.<br />
<br />
What shall we do?<br />
<br />
The life of thousand people depends on that function you need to call, passing to it some fresh allocated memory. The operator new (unless the placement one is called) deals with OOM condition throwing a bad_alloc or returning a null-pointer in case the nothrow version of it is used.<br />
<br />
But as programmer what can you do when a bad_alloc is thrown or a null-pointer is returned?<br />
<br />
There are several options, but the most "nifty" one is the following.<br />
<br />
When the operator new is not able to allocate the required memory it calls a function, at this point the function can try to free some memory, throwing an exception or exit the program. Exiting the program is not a good option I have to say, indeed the caller of the operator new (or operator new [] for the matter) expects a bad_alloc (or a derivation of it) or a nullptr (in case the nothrow was used).<br />
<br />
A programmer is able to specify the function to be call in case of OOM with the following function:
<br />
<br />
<script src="https://gist.github.com/kalman5/9a4685e0931d7a599b4d.js"></script>
the operator new will keep calling the specified function every time it tries to allocate memory and it doesn't succeeded. A programmer can exploit this mechanism in the following way:<br />
<ol>
<li>Allocate a the programming startup a bunch of memory reserving it for future uses.</li>
<li>Install the new handler that will free the reserved memory, in case the reserved memory was already release then throw bad_alloc. </li>
</ol>
<div>
The following code does exactly what described:</div>
<div>
<br />
<script src="https://gist.github.com/kalman5/f503e9e8be898e5483ad.js"></script></div>
<div>
<br />
issuing a ulimit -v 100000 before to run it (in order to decrease the memory that can be used), the output is the following:<br />
<br />
<blockquote class="tr_bq">
<blockquote class="tr_bq">
FREEING SOME MEMORY</blockquote>
<blockquote class="tr_bq">
SUCCEEDED</blockquote>
<blockquote class="tr_bq">
NO MORE MEMORY TO FREE</blockquote>
<blockquote class="tr_bq">
terminate called after throwing an instance of 'std::bad_alloc'</blockquote>
<blockquote class="tr_bq">
what(): std::bad_alloc</blockquote>
<blockquote class="tr_bq">
Aborted (core dumped)</blockquote>
</blockquote>
<br />
<br /></div>
As you can see at least once we were able to free some memory and the first allocation after the OOM condition was able to allocate memory due the fact some was freed by us, unfortunately there were no more space on the second call.
You have no excuse anymore to have a crash due to OOM condition, what you can do at least is to free the memory, launch a warning, writing in the logs, send a message to a pager or whatever action that soon the memory will be over for real!
Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com1tag:blogger.com,1999:blog-3197434572108789628.post-4270421583615986612014-05-21T15:30:00.002-07:002014-05-22T04:59:19.246-07:00Prevent exceptions from leaving destructors. Now!Any seasoned C++ programmer should now that permitting an exception to leave the destructor is bad practice, googling for "throw exception destructor" it leads to enough results convincing yourself that is a bad practice (see for example Meyers's "More effective C++" Item 11). Most of the arguments are: "if an object is destroyed during a stack unwinding then throwing an exception it triggers the terminate function" or "if an STL container is being destroyed it start to destroy all his contained elements and given the fact the STL containers do not expect an exception being thrown then it will not complete the destruction of the remaining objects".
<br></br>
If you still are not convinced by those arguments then I hope you will buy at least the following.
Let's look at a possible implementation of an unique ptr (apart the -> and * operators):
<br></br>
<script src="https://gist.github.com/kalman5/4f9bab4b1e9f127446ce.js"></script>
and a possible use:
<br></br>
<script src="https://gist.github.com/kalman5/fb5a00d72a1693f37480.js"></script>
As you can see the AutoPtr::reset() deletes the stored pointer and then is not able to nullify it due the throw, as soon as the "a" instance goes out of scope due the stack unwinding then ~AutoPtr deletes again thePointer.
A possible implementation of reset can be the following:
<br></br>
<script src="https://gist.github.com/kalman5/861f6396282136b55ad1.js"></script>
but unfortunately it not saves you! Indeed in c++11 specification you can "find" the following:
<blockquote>12.4.3: A declaration of a destructor that does not have an exception-specification is implicitly considered to have
the same exception-specification as an implicit declaration (15.4).</blockquote>
and again:
<blockquote>Whenever an exception is thrown and the search for a handler (15.3) encounters the outermost block of a
function with an exception-specification that does not allow the exception, then,
— if the exception-specification is a dynamic-exception-specification, the function std::unexpected() is
called (15.5.2),
— otherwise, the function std::terminate() is called (15.5.1).</blockquote>
that means that throwing an exception from a DTOR terminates your program and it doesn't matter if a stack unwinding
is going on or not. <br></br> This simple example <br></br>
<script src="https://gist.github.com/kalman5/b0d915060194f4f112ee.js"></script>
does generate a crash if compiled in c++11 mode with gcc (4.8 and 4.9) and clang (3.5) while with intel icc 14.01 doens't call the std::unexpected either the std::terminate (time to fill an icc bug?)Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com3tag:blogger.com,1999:blog-3197434572108789628.post-91902174346869478192014-03-09T14:43:00.000-07:002014-03-09T15:04:39.014-07:00ref-qualifiersC++11 introduced the ability to "ref-qualifier" methods. The most known qualifier is the const one:
<br></br>
<script src="https://gist.github.com/kalman5/9451810.js"></script>
however now is also possible to ref-qualify *this
<br></br>
<script src="https://gist.github.com/kalman5/9455164.js"></script>
let see how this can be of any use. Immagine to have a factory building heavy objects and
returning them by copy this way:
<br></br>
<script src="https://gist.github.com/kalman5/9455286.js"></script>
in the following scenario we can avoid an useless copy:
<br></br>
<script src="https://gist.github.com/kalman5/9455314.js"></script>
we can avoid the copy if Jumbo is movable overloading the method getJumboByCopy in case
the object on which I'm calling it is a temporary:
<br></br>
<script src="https://gist.github.com/kalman5/9455397.js"></script>
To be honest the example shows a scenario with other problems than the one mentioned (for instance
if the object Jumbo is so big why permitting the copy then?) but I hope you got the idea.
Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com4tag:blogger.com,1999:blog-3197434572108789628.post-50510342367322281222014-02-16T02:02:00.001-08:002014-02-16T07:56:10.610-08:00The under-evaluated delete specifierAs you should know by now in C++11 we are able disable certain signatures in our classes. Most of the time this is used to disable copy constructor and assignment operators
<br />
<br />
<script src="https://gist.github.com/kalman5/9031575.js"></script>
<br />
this way is much better than the old way where the programmer had to declare private both member and then not implement them getting an error either at compile time or either at linking time.<br /><br />
The delete specifier can disable some automatic overloads consider indeed the following code<br /><br />
<script src="https://gist.github.com/kalman5/9031664.js"></script>
it works perfectly but if we do not want that automatic conversion (note that explicit can not be used here) then the delete specifier can come in handy
<br /><br />
<script src="https://gist.github.com/kalman5/9031724.js"></script>
A typical mistake in C++ is to store a reference (or a pointer for the matter) to a temporary object leading to a disaster. This mistake is one of the argument java guys put on the table when they are arguing against C++.<br />
<br />
The following class is a perfectly working class but used wrongly can store a reference to a temporary object<br /><br />
<script src="https://gist.github.com/kalman5/9031779.js"></script>
<br />
The delete specifier can help us again indeed we can disable the constructor with a rvalue reference and avoid such use:<br /><br />
<script src="https://gist.github.com/kalman5/9031943.js"></script>
<br/>
now the code above will lead to a compilation error in case we are trying to build the class with a temporary string, you should note that a "const &&" is needed, indeed without the const specifier passing a "const std::string foo()" will not led to a compilation error<br/><br/>
Time to add to my coding rules a new rule!Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com3tag:blogger.com,1999:blog-3197434572108789628.post-8480037728438289012014-01-05T09:18:00.001-08:002018-12-14T01:45:43.920-08:00A bad workman always blames his tools (Miguel de Icaza inside)Recently Miguel de Icaza revealed in his blog the fact that they regret the decision to develop Moonlight in C++, you can read about it in here:<br />
<br />
<a href="http://tirania.org/blog/archive/2014/Jan-04.html">http://tirania.org/blog/archive/2014/Jan-04.html</a><br />
<br />
My first thought was, wow he found a way to communicate with a parallel universe where a "Miguel de Icaza" took the decision to got for C and he is now comparing the two projects.<br />
<br />
Anyway, he is Miguel de Icaza after all, he is behind the Gnome and Mono projects. Chapeau!<br />
I was curious to look their code base and after having opened some sources here and there I was horrified and I mean it. <br />
<br />
Issues I found (just opening a few files not doing a full code inspection and without any static analysis check):<br />
<br />
<ul>
<li>CTORs are not using initialization list</li>
<li>Arguments of CTORs and functions are not getting parameters as const reference copying basically all the arguments passed</li>
<li>Methods that should be marked as const are not(as example KeyTime::HasPercent), this means that const correctness is not used around in the code.</li>
<li>classes not meant to be modified after the construction do not have their member marked const</li>
<li>Not all classes have all their member initialized</li>
<li>Classes have their destructor marked as virtual even when not needed and also if the DTOR is marked as virtual why the CopyCTOR is not implemented or then "disabled"?</li>
<li>List reimplemented from scratch and instead to make the List template the List is a standarad bidirectional implementation with Node hosting only the next/prev pointers and a virtual destructor (nice vptr overhead when not needed) then a derived class from Node a template GenericNode. The user of this class has to create his own class Inheriting from GenericNode (see EventObjectNode).</li>
<li>brush.cpp: You accept first a possibile division by zero and then you fix the result</li>
</ul>
<blockquote class="tr_bq">
<blockquote class="tr_bq">
double sx = sw / width;</blockquote>
<blockquote class="tr_bq">
double sy = sh / height;</blockquote>
<blockquote class="tr_bq">
if (width == 0)</blockquote>
<blockquote class="tr_bq">
<span class="Apple-tab-span" style="white-space: pre;"> </span>sx = 1.0;</blockquote>
<blockquote class="tr_bq">
if (height == 0)</blockquote>
<blockquote class="tr_bq">
<span class="Apple-tab-span" style="white-space: pre;"> </span>sy = 1.0;</blockquote>
</blockquote>
<br />
<ul>
<li>collection.cpp the following statement looks suspicious: </li>
</ul>
<br />
<blockquote class="tr_bq">
if (n == 0 || n % 1 == 1) {...}</blockquote>
<br />
<div>
General remarks on the code:</div>
<div>
<ul>
<li>I haven't seen a single class not copyable</li>
<li>on around 250K line of code just a mere 128 asserts</li>
<li>Variables assigned twice without use the first assigned value (see as example CornerRadius::FromStr implementation).</li>
<li>Scope variables can be reduced </li>
<li>Pointer cast C-Styled</li>
<li>Unused variables</li>
</ul>
<br />So dear Miguel de Icaza, please fix your code base then we can talk about performances and memory efficiency. Unless your regret has to be read as the following: "We regret to have chosen C++ without having a deep knowledge of it and without any best practice to follow".</div>
<div>
<br /></div>
<br />
<br />
<br />Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com3tag:blogger.com,1999:blog-3197434572108789628.post-92075269056851255762013-08-28T13:45:00.001-07:002020-01-25T01:37:43.483-08:00STL is not thread safe, get over itSTL is not thread safe, get over it. Why it should be after all? STL is plain C++ code, compiler doesn't even know the existence of the STL, it's just C++ code released with your compiler, what the STL has in common with the C++ language is the fact that it's standardized. Given the fact that C++ and STL is standardized you can expect to have STL implemented on every platform with same guarantees enforced by the standard. You are not even obliged to use the STL deployed with your compiler indeed there are various version out there, see the roguewave ones for example (<a href="http://www.roguewave.com/">http://www.roguewave.com</a>).<br />
Let's take as example the std::list, let's suppose for a moment that the stl implementation it's thread safe,<br />
some problem arise:<br />
<br />
<ol>
<li>If it's not needed to be thread safe you will get extra not needed overhead</li>
<li>What shall do a thread calling a std::list::pop_back on an empty list?</li>
<ul>
<li>Waiting for a std::list::push?</li>
<li>Returning with an "error"?</li>
<li>Throwing an exception?</li>
<li>Waiting for a certain ammount of seconds that an entry is available?</li>
</ul>
<li>It should be used in an enviroment with a single producer/single consumer, in this case it's possible to implement it without locks.</li>
<li>Shall be multiple readers permitted?</li>
</ol>
<div>
Yes sure you can solve all the points above with a policy template, but just imagine your users complains.</div>
<div>
<br /></div>
<div>
Well, again, get over it, STL standardized is not thread safe you have to create your thread safe list embedding a real std::list, after all making a wrapper around an STL container is a so easy exercise that if you find difficult to do it yourself then you have to ask: "Am I ready to start with multithreading programming" even if someone provides me an out of the shelf std::list thread safe?</div>
<div>
Consider that two different instance of STL containers can be safely manipulated by different threads.</div>
<div>
<br /></div>
<div>
PS: I have written about std::list instead of the most widely used std::vector because std::vector for his own characteristics has to be used in a more "static" way with respect to an std::list and using an std::vector as a container used by multiple threads (producer/consumer) is a plain wrong choice. </div>
<div>
<br /></div>
<div>
<br /></div>
<div>
<br /></div>
<div>
<br /></div>
<div>
<br /></div>
<div>
<br /></div>
Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com0tag:blogger.com,1999:blog-3197434572108789628.post-23346871332499575752013-06-13T15:07:00.001-07:002013-06-14T03:39:28.873-07:00Code inspecting Stroustrup (TC++PL4) (2nd issue)Since I learned C++ reading one of the first edition of TC++PL and even if I'm programming using C++ since 2000 or so I'm reading the new TC++PL4 carefully as if this language is totally new to me. It seems my last post about an error found on this book will be not the unique and here we are again.<br />
In 5.3.4.1 he introduces conditions and how two threads can interact each other communicating using events, it's presented the classical producer / consumer interaction exchanging <span style="color: blue;">Messages </span>trough a queue, and this is the poor implementation proposed:<br />
<br />
<span style="color: blue;">class Message {</span><br />
<span style="color: blue;"> // ...</span><br />
<span style="color: blue;"> };</span><br />
<span style="color: blue;"><br /></span>
<span style="color: blue;"> queue<message> mqueue;</message></span><br />
<span style="color: blue;"> condition_variable mcond;</span><br />
<span style="color: blue;"> mutex mmutex;</span><br />
<span style="color: blue;"><br /></span>
<span style="color: blue;"> void consumer() </span><br />
<span style="color: blue;"> {</span><br />
<span style="color: blue;"> while(true) {</span><br />
<span style="color: blue;"> unique_lock<mutex> lck{mmutex};</mutex></span><br />
<span style="color: blue;"> while (mcond.wait(lck)) <i>/* do nothing */</i>;</span><br />
<span style="color: blue;"> </span><br />
<span style="color: blue;"> auto m = mqueue.front();</span><br />
<span style="color: blue;"> mqueue.pop();</span><br />
<span style="color: blue;"> lck.unlock();</span><br />
<span style="color: blue;"> // ... process m...</span><br />
<span style="color: blue;"> }</span><br />
<span style="color: blue;"> }</span><br />
<br />
<span style="color: blue;"> void producer() </span><br />
<span style="color: blue;"> {</span><br />
<span style="color: blue;"> while(true) {</span><br />
<span style="color: blue;"> Message m;</span><br />
<span style="color: blue;"> <i>// ... fill the message ...</i></span><br />
<span style="color: blue;"> unique_lock<mutex> lck{mmutex};</mutex></span><br />
<span style="color: blue;"> </span><span style="color: blue;">mqueue.push(m);</span><br />
<span style="color: blue;"> mcond.notify_one();</span><br />
<span style="color: blue;"> }</span><br />
<span style="color: blue;"> }</span><br />
<span style="color: blue;"><br /></span>
This implementation is affected by at least three issues:<br />
<br />
<ul>
<li>Unless very lucky the queue will grow indefinitely: that's because basically the consumer will wait at each cycle even if queue contains something, at the same time it has a chance (I repeat a "chance") to exit from the condition_variable::wait() only each time the producer puts something in the queue.</li>
<li>The consumer can miss the <span style="color: blue;">condition_variable::notify_one </span>event, indeed if the producer does the <span style="color: blue;">notify_one() </span>but the other thread hasn't yet executed the <span style="color: blue;">wait() </span>the consumer will block for no reason</li>
<li>The producer holds the <span style="color: blue;">unique_lock </span>for more time than needed, the <span style="color: blue;">mutex </span>has to only protect the queue not the condition as well</li>
</ul>
<div>
Let see how those producer / consumer should have been implemented:</div>
<div>
<br /></div>
<br />
<span style="color: blue;"> void consumer() </span><br />
<span style="color: blue;"> {</span><br />
<span style="color: blue;"> while(true) {</span><br />
<span style="color: blue;"> unique_lock<mutex> lck{mmutex};</mutex></span><br />
<span style="color: blue;"> while (mqueue.empty()) {<i> // the empty condition has to be recheck </i></span><i style="color: blue;">indeed the thread</i><br />
<span style="color: blue;"><i> // can get sporadic wakeup</i></span><i style="color: blue;"> without any thread doing a notify</i><br />
<span style="color: blue;"> </span><span style="color: blue;">mcond.wait(lck);</span><br />
<span style="color: blue;"> }</span><br />
<span style="color: blue;"> auto m = mqueue.front();</span><br />
<span style="color: blue;"> mqueue.pop();</span><br />
<span style="color: blue;"> lck.unlock();</span><br />
<span style="color: blue;"> // ... process m...</span><br />
<span style="color: blue;"> }</span><br />
<span style="color: blue;"> }</span><br />
<span style="color: blue;"><br /></span>
<span style="color: blue;"> void producer() </span><br />
<span style="color: blue;"> {</span><br />
<span style="color: blue;"> while(true) {</span><br />
<span style="color: blue;"> Message m;</span><br />
<span style="color: blue;"> </span><span style="color: blue;"> </span><i style="color: blue;">// ... fill the message ...</i><br />
<span style="color: blue;"><i> </i>{</span><br />
<span style="color: blue;"> unique_lock<mutex> lck{mmutex};</mutex></span><br />
<span style="color: blue;"> </span><span style="color: blue;">mqueue.push(m);</span><br />
<span style="color: blue;"> } // </span><i style="color: blue;">This extra scope is in here to release the mmutex asap</i><br />
<span style="color: blue;"> mcond.notify_one();</span><br />
<span style="color: blue;"> }</span><br />
<span style="color: blue;"> }</span><br />
<span style="color: blue;"><br /></span>
In my opinion this should have been the version of the producer/consumer in TC++PL4, as you can see<br />
with a simple extra scope and the right while(...) the issues reported in the bullets are solved.<br />
<br />
There is another problem, I have to admit that this issue most of the times is a minore one:<br />
<ul>
<li>The producer can issue <span style="color: blue;">notify_one() </span>even if not needed, and this can be a performance issue </li>
</ul>
to address it the producer has to "forecast" if the consumer can be in a blocked status, and this can happen only if after having acquired the mmutex the queue is empty, this is the final version of producer:<br />
<span style="color: blue;"><br /></span>
<span style="color: blue;"> void producer() </span><br />
<span style="color: blue;"> {</span><br />
<span style="color: blue;"> while(true) {</span><br />
<span style="color: blue;"> Message m;</span><br />
<span style="color: blue;"> bool notifyIsNeeded = false;</span><br />
<span style="color: blue;"> <i>// ... fill the message ...</i></span><br />
<span style="color: blue;"><i> </i>{</span><br />
<span style="color: blue;"> unique_lock<mutex> lck{mmutex};</mutex></span><br />
<span style="color: blue;"> if (mqueue.empty()) {</span><br />
<span style="color: blue;"> </span><span style="color: blue;">notifyIsNeeded</span><span style="color: blue;"> = true;</span><br />
<span style="color: blue;"> }</span><br />
<span style="color: blue;"> </span><span style="color: blue;">mqueue.push(m);</span><br />
<span style="color: blue;"> } // </span><i style="color: blue;">This extra scope is in here to release the mmutex asap</i><br />
<span style="color: blue;"><i> </i>if ( </span><span style="color: blue;">notifyIsNeeded</span><span style="color: blue;"> ) {</span><br />
<span style="color: blue;"> mcond.notify_one();</span><br />
<span style="color: blue;"> }</span><br />
<span style="color: blue;"> }</span><br />
<span style="color: blue;"> }</span><br />
<span style="color: blue;"><br /></span>
Writing correct code is not easy and writing correct multi-threaded is damn hard.<br />
<span style="color: blue;"><br /></span>
<span style="color: blue;"><br /></span>
<span style="color: blue;"><br /></span>Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com1tag:blogger.com,1999:blog-3197434572108789628.post-23053028665809730682013-06-09T09:27:00.002-07:002013-06-09T11:31:46.853-07:00Code inspecting Stroustrup (TC++PL4)TC++PL4 is now on my desk and carefully reading it I have to say that even Stroustrup makes stupid mistakes in his classes implementations.<br />
<br />
He illustrates a typical Vector implementation (pag. 73):<br />
<br />
<span style="color: blue;">class Vector {</span><br />
<span style="color: blue;"> private:</span><br />
<span style="color: blue;"> double* elem;</span><br />
<span style="color: blue;"> int sz;</span><br />
<span style="color: blue;"> public:</span><br />
<span style="color: blue;"> Vector(int s);</span><br />
<span style="color: blue;"> ~Vector() { delete [] elem; }</span><br />
<span style="color: blue;"> </span><br />
<span style="color: blue;"> Vector(const Vector& a);</span><br />
<span style="color: blue;"> Vector& operator=(const Vector& a);</span><br />
<span style="color: blue;"><br /></span>
<span style="color: blue;"> double& operator[](int i);</span><br />
<span style="color: blue;"> const double& operator[](int i) const;</span><br />
<span style="color: blue;"><br /></span>
<span style="color: blue;"> int size() const;</span><br />
<span style="color: blue;"> };</span><br />
<span style="color: blue;"><br /></span>
and given the fact this class needs a copy constructor implemented he "implements" it (pag. 74):<br />
<br />
<span style="color: blue;"> Vector::Vector(const Vector& a)<br /> :elem{new double[sz]},<br /> sz{a.sz}<br /> {<br /> for (int i = 0; i != sz; ++i) <br /> elem[i] = a.elem[i];<br /> }</span><br />
<br />
as you can see the elem vector is built with a size retrieved from a not yet initialized variable, being it defined at page 74 I had my last hope flipping the page and checking at page 73 if "<span style="color: blue;">int sz</span>" was declared before "<span style="color: blue;">double* elem</span>" but it was not the case.<br />
<br />
I'm sad.<br />
<br />
<br />
<br />
<br />Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com2tag:blogger.com,1999:blog-3197434572108789628.post-41220376324082631582010-11-01T12:36:00.000-07:002010-11-01T12:37:24.824-07:00Temporary objectsMost believe that temporary objects are const, well they are "almost const". It is possible indeed call on a temporary object a non const member. Nifty exception stated in the standard: "<span class="Apple-style-span" style="font-family: sans-serif; font-size: 13px; line-height: 19px;">Section 3.10.10 in C++ ISO/IEC 14882:1998".</span><br />
<span class="Apple-style-span" style="font-family: sans-serif; font-size: small;"><span class="Apple-style-span" style="font-size: 13px; line-height: 19px;">This exception permits to implement a movable constructor (through a proxy) while waiting for </span></span><span class="Apple-style-span" style="font-family: sans-serif; font-size: 13px; line-height: 19px;"><a class="external text" href="http://www.artima.com/cppsource/rvalue.html" rel="nofollow" style="background-attachment: initial; background-clip: initial; background-color: initial; background-image: url(http://bits.wikimedia.org/skins-1.5/vector/images/external-link-ltr-icon.png?2); background-origin: initial; background-position: 100% 50%; background-repeat: no-repeat no-repeat; color: #3366bb; padding-bottom: 0px; padding-left: 0px; padding-right: 13px; padding-top: 0px; text-decoration: none;">Rvalue references</a> in </span><span class="Apple-style-span" style="font-family: sans-serif; font-size: 13px; line-height: 19px;">C++0x language standard.</span><br />
<span class="Apple-style-span" style="font-family: sans-serif; font-size: small;"><span class="Apple-style-span" style="font-size: 13px; line-height: 19px;"><br />
</span></span><br />
<span class="Apple-style-span" style="font-family: sans-serif; font-size: 13px; line-height: 19px;"><br />
</span><br />
<span class="Apple-style-span" style="font-family: sans-serif; font-size: 13px; line-height: 19px;"><br />
</span>Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com0tag:blogger.com,1999:blog-3197434572108789628.post-18302775503652774972010-02-12T02:40:00.001-08:002010-02-14T01:32:45.544-08:00Asserts vs ExceptionsI still see people with the doubt: "shall I use an assert or throw an exception?".<br />Let me whine a bit about the usage of asserts first. Overall people code badly and I mean it, hence assertions are not used enough. Having say that, "assert" and "exception" have different<br />meanings and behaves.<br /><br />A failure assert just terminates the normal program execution (reporting source file name and line position where the error occurred), an exception thrown has a chance to be caught. Asserts disappear/vanish when NDEBUG is defined (usually in production code); this fact is a newbie oversight and what they claim about the massive usage of assert is the overhead introduced by asserts. If I wasn't clear: "Asserts disappear in production code". What didn't you get from: "Asserts disappear in production code"?<br /><br />Those two facts already give an hint on when to use an assert and when an exception. If the error can be managed (by user or code itself) then an exception must be used, if the error has no chance to be managed by anyone then assert is your friend. This is not the only rule to follow indeed as said: "Asserts disappear in production code" and then in production code that error will not be "detected", errors that you expect to happen in production code can not be spotted by an assert.<br />Asserts are meant to detect coding error and there is no reason to happen in production code. When writing a method there are some assumption about the internal class status (pre-condition), better to check those first in order to not do something bad, at the end of the same method better check that the internal status of class is in a consistent status (post-condition).<br />Unfortunately, there are a great deal of people that are using exceptions for case that really ought to be assertions. Throwing exceptions instead of wrapping simple pre/post-conditions into a simple assertion macro is an hint of the fact that you're coding badly. Asserts are used to protect by coding error: using a null pointer for example; exception are to protect by normal program life: server not available, file not writable, etc.<br /><br />It should be always possible to write a unit test that is able to make an exception to be thrown, if you are not able to then it means that that piece of code is a dead code; to the other side it should be impossible do the same with assert, if you are able to write an unit test that is able to make an assert fail then most likely that assert should be an exception, or of course the code is wrong.<br /><br />A nice rule that drives the lazy bone programmers to use more assert is: who ever write a code that provides a set of data that asserted then the same person is responsible to fix it. This rule drives library writer to protect them self by an incorrect usage of their library at the cost to fix even the code using the library.<br /><br />If you have missed it: "Asserts disappear in production code".Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com0tag:blogger.com,1999:blog-3197434572108789628.post-84797846263188887842008-09-18T04:06:00.000-07:002008-09-22T04:14:00.417-07:00Assignment operatorToday I went through a piece of code similar to this (what matters here is how the assignment operator was written):<br /><br /><pre><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">class</span><span style="color: rgb(0, 0, 0);"> Test {</span><br /><span style="color: rgb(0, 0, 0);"> Test()</span><br /><span style="color: rgb(0, 0, 0);"> :theId(</span><span style="color: rgb(0, 0, 255);">0</span><span style="color: rgb(0, 0, 0);">), theName()</span><br /><span style="color: rgb(0, 0, 0);"> {}</span><br /><br /><span style="color: rgb(0, 0, 0);"> Test& </span><span style="font-weight: bold; color: rgb(0, 0, 0);">operator</span><span style="color: rgb(0, 0, 0);">=(</span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> Test& aRhs) {</span><br /><span style="color: rgb(0, 0, 0);"> theID = aRhs.theID;</span><br /><span style="color: rgb(0, 0, 0);"> theName = aRhs.theName;</span><br /><br /><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">return</span><span style="color: rgb(0, 0, 0);"> *</span><span style="font-weight: bold; color: rgb(0, 0, 0);">this</span><span style="color: rgb(0, 0, 0);">;</span><br /><span style="color: rgb(0, 0, 0);"> }</span><br /><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">private</span><span style="color: rgb(0, 0, 0);">:</span><br /><span style="color: rgb(0, 0, 0);"> </span><span style="color: rgb(128, 0, 0);">int</span><span style="color: rgb(0, 0, 0);"> theID;</span><br /><span style="color: rgb(0, 0, 0);"> std::string theName;</span><br /><br /><span style="color: rgb(0, 0, 0);">};</span></pre><br /><div style="text-align: justify;">as it is, the code works and I have nothing to say at first glance. However , who knows how the class will be extended in the future?<br /><br />Imagine that in a future version some members are added:<br /></div><pre><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">class</span><span style="color: rgb(0, 0, 0);"> Test {</span><br /><span style="color: rgb(0, 0, 0);"> Test()</span><br /><span style="color: rgb(0, 0, 0);"> :theId(</span><span style="color: rgb(0, 0, 255);">0</span><span style="color: rgb(0, 0, 0);">), theName(), theHammer(), theHeap(</span><span style="color: rgb(0, 0, 255);">0</span><span style="color: rgb(0, 0, 0);">)</span><br /><span style="color: rgb(0, 0, 0);"> {}</span><br /><br /><span style="color: rgb(0, 0, 0);"> Test& </span><span style="font-weight: bold; color: rgb(0, 0, 0);">operator</span><span style="color: rgb(0, 0, 0);">=(</span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> Test& aRhs) {</span><br /><span style="color: rgb(0, 0, 0);"> theID = aRhs.theID;</span><br /><span style="color: rgb(0, 0, 0);"> theName = aRhs.theName;</span><br /><span style="color: rgb(0, 0, 0);"> theHammer = aRhs.theHammer; </span><span style="font-style: italic; color: rgb(128, 128, 128);">//This is going to be a bottleneck</span><br /><span style="color: rgb(0, 0, 0);"> theHeap = aRhs.theHeap; </span><span style="font-style: italic; color: rgb(128, 128, 128);">//This is wrong</span><br /><br /><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">return</span><span style="color: rgb(0, 0, 0);"> *</span><span style="font-weight: bold; color: rgb(0, 0, 0);">this</span><span style="color: rgb(0, 0, 0);">;</span><br /><span style="color: rgb(0, 0, 0);"> }</span><br /><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">private</span><span style="color: rgb(0, 0, 0);">:</span><br /><span style="color: rgb(0, 0, 0);"> </span><span style="color: rgb(128, 0, 0);">int</span><span style="color: rgb(0, 0, 0);"> theID;</span><br /><span style="color: rgb(0, 0, 0);"> std::string theName;</span><br /><span style="color: rgb(0, 0, 0);"> HugeClass theHammer; </span><span style="font-style: italic; color: rgb(128, 128, 128);">//Extra member with huge footprint</span><br /><span style="color: rgb(0, 0, 0);"> HeapClass* theHeap; </span><span style="font-style: italic; color: rgb(128, 128, 128);">//Extra member on heap</span><br /><span style="color: rgb(0, 0, 0);">};</span></pre><div style="text-align: justify;">as you can see with these two extra members following the same code line as the previous version the code becomes invalid. The objection I get is: "if one day someone adds those two members then they will take care to write it correctly", unfortunately in the real world it doesn't work like this. The average programmer will just add those two extra members following the code line already in place, following the rule: "after all if the code works for the already present members why shouldn't it be the same for the two extra members I have been told to add?"<br /></div><br /><div style="text-align: justify;">The trick to avoid problems like this is to write the right code, from the start thinking of what will happen in the future, when possible of course. Usually the change to make is just a matter of a few lines of code and a two line comment in order to warn the future coders.<br /><br />The original well written class would have been:</div><pre><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">class</span><span style="color: rgb(0, 0, 0);"> Test {</span><br /><span style="color: rgb(0, 0, 0);"> Test()</span><br /><span style="color: rgb(0, 0, 0);"> :theId(</span><span style="color: rgb(0, 0, 255);">0</span><span style="color: rgb(0, 0, 0);">), theName()</span><br /><span style="color: rgb(0, 0, 0);"> {}</span><br /><br /><span style="color: rgb(0, 0, 0);"> Test& </span><span style="font-weight: bold; color: rgb(0, 0, 0);">operator</span><span style="color: rgb(0, 0, 0);">=(</span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> Test& aRhs) {</span><br /><br /><span style="color: rgb(0, 0, 0);"> </span><span style="font-style: italic; color: rgb(128, 128, 128);">//Check for a self assignment</span><br /><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">if</span><span style="color: rgb(0, 0, 0);"> (</span><span style="font-weight: bold; color: rgb(0, 0, 0);">this</span><span style="color: rgb(0, 0, 0);"> != &aRhs) {</span><br /><span style="color: rgb(0, 0, 0);"> theID = aRhs.theID;</span><br /><span style="color: rgb(0, 0, 0);"> theName = aRhs.theName;</span><br /><span style="color: rgb(0, 0, 0);"> }</span><br /><br /><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">return</span><span style="color: rgb(0, 0, 0);"> *</span><span style="font-weight: bold; color: rgb(0, 0, 0);">this</span><span style="color: rgb(0, 0, 0);">;</span><br /><span style="color: rgb(0, 0, 0);"> }</span><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">private</span><span style="color: rgb(0, 0, 0);">:</span><br /><span style="color: rgb(0, 0, 0);"> </span><span style="color: rgb(128, 0, 0);">int</span><span style="color: rgb(0, 0, 0);"> theID;</span><br /><span style="color: rgb(0, 0, 0);"> std::string theName;</span><br /><span style="color: rgb(0, 0, 0);">};</span></pre><br />At this point the objection is: why check for a self assignment when the event is never going to happen? Who will ever write:<br /><pre><br /><span style="color: rgb(0, 0, 0);">Test t;</span><br /><br /><span style="color: rgb(0, 0, 0);">t=t;</span></pre>well, the code is valid so be sure someone will, also it is not always easy to spot a self assignment, consider this:<br /><pre><br /><span style="color: rgb(0, 0, 0);">t[j] = t[i];</span><br /></pre><br />or even:<br /><pre><br /><span style="color: rgb(0, 0, 0);">Test t;</span><br /><span style="color: rgb(0, 0, 0);">Test &a = t;</span><br /><span style="color: rgb(0, 0, 0);">...</span><br /><span style="color: rgb(0, 0, 0);">t = a;<br /></span></pre><br /><div style="text-align: justify;">The real question here is: "Why was the assignment operator in that class was ever implemented?" The question makes a point, the operator was not needed at all, in that case the right thing to do is to remove it.<br /><br />Let's then suppose the class is the one with the two extra members, the class is managing dynamically allocated memory then the operator <span style="font-weight: bold;">must</span> be implemented. The almost correct version is:<br /></div><br /><br /><pre><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">class</span><span style="color: rgb(0, 0, 0);"> Test {</span><br /><span style="color: rgb(0, 0, 0);"> Test()</span><br /><span style="color: rgb(0, 0, 0);"> :theId(</span><span style="color: rgb(0, 0, 255);">0</span><span style="color: rgb(0, 0, 0);">), theName(), theHammer(), theHeap(</span><span style="font-weight: bold; color: rgb(0, 0, 0);">new</span><span style="color: rgb(0, 0, 0);"> HeapClass)</span><br /><span style="color: rgb(0, 0, 0);"> {}</span><br /><br /><span style="color: rgb(0, 0, 0);"> Test& </span><span style="font-weight: bold; color: rgb(0, 0, 0);">operator</span><span style="color: rgb(0, 0, 0);">=(</span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> Test& aRhs) {</span><br /><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">if</span><span style="color: rgb(0, 0, 0);"> (</span><span style="font-weight: bold; color: rgb(0, 0, 0);">this</span><span style="color: rgb(0, 0, 0);"> != &aRhs) {</span><br /><span style="color: rgb(0, 0, 0);"> theID = aRhs.theID;</span><br /><span style="color: rgb(0, 0, 0);"> theName = aRhs.theName;</span><br /><span style="color: rgb(0, 0, 0);"> theHammer = aRhs.theHammer; </span><br /><br /><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">delete</span><span style="color: rgb(0, 0, 0);"> theHeap;</span><br /><span style="color: rgb(0, 0, 0);"> theHeap = </span><span style="font-weight: bold; color: rgb(0, 0, 0);">new</span><span style="color: rgb(0, 0, 0);"> HeapClass(*aRhs.theHeap);</span><br /><span style="color: rgb(0, 0, 0);"> }</span><br /><br /><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">return</span><span style="color: rgb(0, 0, 0);"> *</span><span style="font-weight: bold; color: rgb(0, 0, 0);">this</span><span style="color: rgb(0, 0, 0);">;</span><br /><span style="color: rgb(0, 0, 0);"> }</span><br /><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">private</span><span style="color: rgb(0, 0, 0);">:</span><br /><span style="color: rgb(0, 0, 0);"> </span><span style="color: rgb(128, 0, 0);">int</span><span style="color: rgb(0, 0, 0);"> theID;</span><br /><span style="color: rgb(0, 0, 0);"> std::string theName;</span><br /><span style="color: rgb(0, 0, 0);"> HugeClass theHammer; </span><span style="font-style: italic; color: rgb(128, 128, 128);">//Extra member with huge footprint</span><br /><span style="color: rgb(0, 0, 0);"> HeapClass* theHeap; </span><span style="font-style: italic; color: rgb(128, 128, 128);">//Extra member on heap</span><br /><span style="color: rgb(0, 0, 0);">};</span></pre><br /><br /><div style="text-align: justify;">I wrote "almost correct" because the assignment operator is correct but not exception safe, imagine what will happen if an exception is thrown, if that is the case then the class will be left in an inconsistent state. The goal is not an easy one, in order to make an assignment operator exception safe before modifying the internal state it is better to create a temporary object and then swap it with the internal state with operations that do not throw exceptions. This is achieved using the Pimpl idiom, moving all the internal state of Test inside another class and then leaving inside the class Test a pointer to this new class, it is better to use a boost::shared_ptr in this case to avoid exception catches:</div><br /><br /><pre><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">class</span><span style="color: rgb(0, 0, 0);"> TestImpl {</span><br /><br /><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">friend</span><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">class</span><span style="color: rgb(0, 0, 0);"> Test;</span><br /><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">private</span><span style="color: rgb(0, 0, 0);">:</span><br /><span style="color: rgb(0, 0, 0);"> TestImpl()</span><br /><span style="color: rgb(0, 0, 0);"> :theID(</span><span style="color: rgb(0, 0, 255);">0</span><span style="color: rgb(0, 0, 0);">), theName(), theHammer(), theHeap(</span><span style="font-weight: bold; color: rgb(0, 0, 0);">new</span><span style="color: rgb(0, 0, 0);"> HeapClass)</span><br /><span style="color: rgb(0, 0, 0);"> {}</span><br /><br /><span style="color: rgb(0, 0, 0);"> TestImpl(</span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> TestImpl& aRhs)</span><br /><span style="color: rgb(0, 0, 0);"> :theId(aRhs.theID), theName(aRhs.theName),</span><br /><span style="color: rgb(0, 0, 0);"> theHammer(aRhs.theHammer), theHeap(</span><span style="font-weight: bold; color: rgb(0, 0, 0);">new</span><span style="color: rgb(0, 0, 0);"> HeapClass(aRhs.theHeap))</span><br /><span style="color: rgb(0, 0, 0);"> {}</span><br /><br /><span style="color: rgb(0, 0, 0);"> </span><span style="color: rgb(128, 0, 0);">int</span><span style="color: rgb(0, 0, 0);"> theID;</span><br /><span style="color: rgb(0, 0, 0);"> std::string theName;</span><br /><span style="color: rgb(0, 0, 0);"> HugeClass theHammer; </span><span style="font-style: italic; color: rgb(128, 128, 128);">//Extra member with huge footprint</span><br /><span style="color: rgb(0, 0, 0);"> HeapClass* theHeap; </span><span style="font-style: italic; color: rgb(128, 128, 128);">//Extra member on heap</span><br /><span style="color: rgb(0, 0, 0);">};</span><br /><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">class</span><span style="color: rgb(0, 0, 0);"> Test {</span><br /><br /><span style="color: rgb(0, 0, 0);"> Test()</span><br /><span style="color: rgb(0, 0, 0);"> :theImplementation(</span><span style="font-weight: bold; color: rgb(0, 0, 0);">new</span><span style="color: rgb(0, 0, 0);"> TestImpl)</span><br /><span style="color: rgb(0, 0, 0);"> {}</span><br /><br /><span style="color: rgb(0, 0, 0);"> Test& </span><span style="font-weight: bold; color: rgb(0, 0, 0);">operator</span><span style="color: rgb(0, 0, 0);">=(</span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> Test& aRhs) {</span><br /><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">if</span><span style="color: rgb(0, 0, 0);"> (</span><span style="font-weight: bold; color: rgb(0, 0, 0);">this</span><span style="color: rgb(0, 0, 0);"> != &aRhs) {</span><br /><span style="color: rgb(0, 0, 0);"> boost::shared_ptr<TestImpl> tmp(</span><span style="font-weight: bold; color: rgb(0, 0, 0);">new</span><span style="color: rgb(0, 0, 0);"> TestImpl(*aRhs.theImplementation));</span><br /><br /><span style="color: rgb(0, 0, 0);"> std::swap(theImplementation, tmp);</span><br /><span style="color: rgb(0, 0, 0);"> }</span><br /><br /><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">return</span><span style="color: rgb(0, 0, 0);"> *</span><span style="font-weight: bold; color: rgb(0, 0, 0);">this</span><span style="color: rgb(0, 0, 0);">;</span><br /><span style="color: rgb(0, 0, 0);"> }</span><br /><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">private</span><span style="color: rgb(0, 0, 0);">:</span><br /><span style="color: rgb(0, 0, 0);"> boost::shared_ptr<TestImpl> theImplementation;</span><br /><span style="color: rgb(0, 0, 0);">};</span></pre><br />As you can see writing right code is not easy, and this becomes more difficult if you want to write exception safe code. What I suggest is, to remove assignment operator and copy constructor and write those only if really needed:<br /><br /><pre><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">class</span><span style="color: rgb(0, 0, 0);"> Test {</span><br /><br /><span style="color: rgb(0, 0, 0);"> Test()</span><br /><span style="color: rgb(0, 0, 0);"> :theID(</span><span style="color: rgb(0, 0, 255);">0</span><span style="color: rgb(0, 0, 0);">), theName(), theHammer(), theHeap(</span><span style="font-weight: bold; color: rgb(0, 0, 0);">new</span><span style="color: rgb(0, 0, 0);"> HeapClass)</span><br /><span style="color: rgb(0, 0, 0);"> {}</span><br /><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">private</span><span style="color: rgb(0, 0, 0);">:</span><br /><span style="color: rgb(0, 0, 0);"> Test& </span><span style="font-weight: bold; color: rgb(0, 0, 0);">operator</span><span style="color: rgb(0, 0, 0);">=(</span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> Test& aRhs); </span><span style="font-style: italic; color: rgb(128, 128, 128);">//disabled (do not even implement it)</span><br /><span style="color: rgb(0, 0, 0);"> Test(</span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> Test& aRhs); </span><span style="font-style: italic; color: rgb(128, 128, 128);">//disabled (do not even implement it)</span><br /><br /><br /><span style="color: rgb(0, 0, 0);"> </span><span style="color: rgb(128, 0, 0);">int</span><span style="color: rgb(0, 0, 0);"> theID;</span><br /><span style="color: rgb(0, 0, 0);"> std::string theName;</span><br /><span style="color: rgb(0, 0, 0);"> HugeClass theHammer; </span><span style="font-style: italic; color: rgb(128, 128, 128);">//Extra member with huge footprint</span><br /><span style="color: rgb(0, 0, 0);"> HeapClass* theHeap; </span><span style="font-style: italic; color: rgb(128, 128, 128);">//Extra member on heap</span><br /><span style="color: rgb(0, 0, 0);">};</span></pre>Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com0tag:blogger.com,1999:blog-3197434572108789628.post-90556370282139346112008-06-19T07:31:00.000-07:002013-04-04T13:12:52.575-07:00Threading mess (2)!I got some comments about my last post (<a href="http://cpp-today.blogspot.it/2008/06/threading-mess.html" target="_blank">threading-mess</a>) about the fact that the showed solution was just detecting the scenario of two or more threads entering at the same time a critical section (in the last post example the method <span style="font-style: italic;">Shared::foo</span>). What it doesn't answer is the real question: "is this class during its life being used by more than a single thread, or more specifically, is a certain section of code used by more than a thread"? Indeed if you remember after a SCOPED_LOCK leaves his scope, it "releases" the stored current ID thread allowing another thread to enter it.<br />
<br />
I have added a nested Watch class to ThreadCollisionWarning class that detects also if a critical section is ever used by two different threads ( for example you can detect if a given class is constructed and destroyed within the same thread).<br />
<br />
The code is the following:<br />
<br />
<pre style="background-color: #eeeeee; border: 1px dashed; color: black; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; font-size: 12px; overflow: auto;">
<span style="color: green;">#ifndef THREAD_COLLISION_WARNING</span>
<span style="color: green;">#define THREAD_COLLISION_WARNING</span>
<span style="color: green;">#include <stdexcept></span>
<span style="color: green;">#ifdef NDEBUG</span>
<span style="color: green;">#define THREAD_WATCH(obj)</span>
<span style="color: green;">#define SCOPED_WATCH(obj)</span>
<span style="color: green;">#define WATCH(obj)</span>
<span style="color: green;">#else</span>
<span style="color: green;">#define THREAD_WATCH(obj) ThreadCollisionWarning _##obj;</span>
<span style="color: green;">#define SCOPED_WATCH(obj) ThreadCollisionWarning::ScopedWatch scoped_watch_##obj(_##obj);</span>
<span style="color: green;">#define WATCH(obj) ThreadCollisionWarning::Watch watch_##obj(_##obj);</span>
<span style="color: green;">#endif</span>
<span style="color: black; font-weight: bold;">class</span><span style="color: black;"> ThreadCollisionWarning {</span>
<span style="color: black; font-weight: bold;">public</span><span style="color: black;">:</span>
<span style="color: black;"> ThreadCollisionWarning()</span>
<span style="color: black;"> :theActiveThread(</span><span style="color: blue;">0</span><span style="color: black;">)</span>
<span style="color: black;"> { }</span>
<span style="color: black;"> ~ThreadCollisionWarning() { }</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">class</span><span style="color: black;"> Watch {</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">public</span><span style="color: black;">:</span>
<span style="color: black;"> Watch(ThreadCollisionWarning& aTCW)</span>
<span style="color: black;"> :theWarner(aTCW)</span>
<span style="color: black;"> { theWarner.enter_self(); }</span>
<span style="color: black;"> ~Watch() { }</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">private</span><span style="color: black;">:</span>
<span style="color: black;"> ThreadCollisionWarning& theWarner;</span>
<span style="color: black;"> };</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">class</span><span style="color: black;"> ScopedWatch {</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">public</span><span style="color: black;">:</span>
<span style="color: black;"> ScopedWatch(ThreadCollisionWarning& aTCW)</span>
<span style="color: black;"> :theWarner(aTCW)</span>
<span style="color: black;"> { theWarner.enter(); }</span>
<span style="color: black;"> ~ScopedWatch() { theWarner.leave(); }</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">private</span><span style="color: black;">:</span>
<span style="color: black;"> ThreadCollisionWarning& theWarner;</span>
<span style="color: black;"> };</span>
<span style="color: black; font-weight: bold;">private</span><span style="color: black;">:</span>
<span style="color: black;"> </span><span style="color: maroon;">void</span><span style="color: black;"> enter_self() { </span>
<span style="color: black;"> </span><span style="color: grey; font-style: italic;">//If the active thread is 0 then I'll write the current thread ID</span>
<span style="color: black;"> </span><span style="color: grey; font-style: italic;">//if two or more threads arrive here only one will success to write on theActiveThread </span>
<span style="color: black;"> </span><span style="color: grey; font-style: italic;">//the current thread ID</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">if</span><span style="color: black;"> (! __sync_bool_compare_and_swap(&theActiveThread, </span><span style="color: blue;">0</span><span style="color: black;">, pthread_self())) { </span>
<span style="color: black;"> </span><span style="color: grey; font-style: italic;">//Last chance! may be is the thread itself calling from a critical section</span>
<span style="color: black;"> </span><span style="color: grey; font-style: italic;">//another critical section</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">if</span><span style="color: black;"> (!__sync_bool_compare_and_swap(&theActiveThread, pthread_self(), theActiveThread)) {</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">throw</span><span style="color: black;"> std::runtime_error(</span><span style="color: #dd0000;">"Thread Collision"</span><span style="color: black;">);</span>
<span style="color: black;"> }</span>
<span style="color: black;"> }</span>
<span style="color: black;"> }</span>
<span style="color: black;"> </span><span style="color: maroon;">void</span><span style="color: black;"> enter() { </span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">if</span><span style="color: black;"> (!__sync_bool_compare_and_swap(&theActiveThread, </span><span style="color: blue;">0</span><span style="color: black;">, pthread_self())) {</span>
<span style="color: black;"> </span><span style="color: grey; font-style: italic;">//gotcha! another thread is trying to use the same class</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">throw</span><span style="color: black;"> std::runtime_error(</span><span style="color: #dd0000;">"Thread Collision"</span><span style="color: black;">);</span>
<span style="color: black;"> }</span>
<span style="color: black;"> }</span>
<span style="color: black;"> </span><span style="color: maroon;">void</span><span style="color: black;"> leave() { </span>
<span style="color: black;"> __sync_fetch_and_xor(&theActiveThread, theActiveThread);</span>
<span style="color: black;"> }</span>
<span style="color: black;"> pthread_t theActiveThread;</span>
<span style="color: black;">};</span>
<span style="color: green;">#endif</span>
</pre>
<br />
<br />
The nested Watch class (used by WATCH macro) just during his constructor initializes theActiveThread member with the current id thread if it isn't still initialized, in case it gives another chance to check if the active thread is itself.<br />
<br />
So let's see some examples of use:<br />
<br />
Case #1: Check that one thread ever uses some critical section (recursion allowed)<br />
<br />
<pre style="background-color: #eeeeee; border: 1px dashed; color: black; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; font-size: 12px; overflow: auto;">
<span style="color: black; font-weight: bold;">struct</span><span style="color: black;"> Shared {</span>
<span style="color: black;"> </span><span style="color: maroon;">void</span><span style="color: black;"> foo() { </span>
<span style="color: black;"> WATCH(CriticaSectionA);</span>
<span style="color: black;"> bar();</span>
<span style="color: black;"> }</span>
<span style="color: black;"> </span><span style="color: maroon;">void</span><span style="color: black;"> bar() {</span>
<span style="color: black;"> WATCH(CriticaSectionA);</span>
<span style="color: black;"> }</span>
<span style="color: black;"> THREAD_WATCH(CriticaSectionA);</span>
<span style="color: black;">};</span></pre>
<br />
<br />
Case #2: Check that a class is constructed and destroyed inside the same thread<br />
<br />
<pre style="background-color: #eeeeee; border: 1px dashed; color: black; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; font-size: 12px; overflow: auto;">
<span style="color: black; font-weight: bold;">struct</span><span style="color: black;"> Shared {</span>
<span style="color: black;"> Shared() {</span>
<span style="color: black;"> WATCH(CTOR_DTOR_SECTION);</span>
<span style="color: black;"> ...</span>
<span style="color: black;"> }</span>
<span style="color: black;"> ~Shared() { </span>
<span style="color: black;"> WATCH(CTOR_DTOR_SECTION);</span>
<span style="color: black;"> ...</span>
<span style="color: black;"> }</span>
<span style="color: black;"> THREAD_WATCH(CTOR_DTOR_SECTION);</span>
<span style="color: black;">};</span></pre>
<br />
note that doing so the Shared destructor can throw an exception, so do not use this in a production code (put the WATCH between a try-catch and just notify it in some way).<br />
<br />
Case #3: Two or more different threads can enter a critical section but in exclusive way (useful to check if external sync mechanism are working).<br />
<br />
<pre style="background-color: #eeeeee; border: 1px dashed; color: black; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; font-size: 12px; overflow: auto;">
<span style="color: black; font-weight: bold;">struct</span><span style="color: black;"> Shared {</span>
<span style="color: black;"> foo() {</span>
<span style="color: black;"> SCOPED_WATCH(CriticalSectionA);</span>
<span style="color: black;"> }</span>
<span style="color: black;"> THREAD_WATCH(CriticalSectionA);</span>
<span style="color: black;">};</span></pre>
Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com0tag:blogger.com,1999:blog-3197434572108789628.post-27997672122948600042008-06-18T08:19:00.000-07:002017-05-08T13:26:57.782-07:00Threading mess!Software development requires discipline, you know what I mean: brainstorming, coding rules, code inspections, pair programming. Unfortunately all these activities for the management are a waste of time so at the end you end up to just act as a "code monkey"; to rub salt to the wound "multithread programming" requires ten time the discipline you need in a single thread environment. I've recently stepped in a project of medium size, and at the easy question: "are those class instances shared between two or more threads" the response was: "no... wait... yes, well I'm not sure... I don't know...". Riiiight.<br />
Let's see a quick technique that should permit to detect (at runtime, sigh!) if two or more threads are using concurrently a class.<br />
<br />
Suppose we have the following class:<br />
<br />
<pre style="background-color: #eeeeee; border: 1px dashed; color: black; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; font-size: 12px; overflow: auto;"><span style="color: black; font-weight: bold;">struct</span><span style="color: black;"> Shared {</span>
<span style="color: black;"> </span><span style="color: maroon;">void</span><span style="color: black;"> foo() { ... }</span>
<span style="color: black;">};</span></pre>
<br />
and we are unsure if two threads are calling the Shared::foo() at same time. One way is to add a mutex to the class Shared and then attempt a "try lock" as first thing to do inside the foo and raise an error in case the try lock fails.<br />
<br />
Something like:<br />
<br />
<pre style="background-color: #eeeeee; border: 1px dashed; color: black; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; font-size: 12px; overflow: auto;"><span style="color: black; font-weight: bold;">class</span><span style="color: black;"> Shared {</span>
<span style="color: black;"> </span><span style="color: maroon;">void</span><span style="color: black;"> foo() {</span>
<span style="color: black;"> TryScopedLock aLock(theMutex);</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">if</span><span style="color: black;"> (!aLock) { </span><span style="color: black; font-weight: bold;">throw</span><span style="color: black;"> std::runtime_error(</span><span style="color: #dd0000;">"BOOM"</span><span style="color: black;">); }</span>
<span style="color: black;"> ...</span>
<span style="color: black;"> }</span>
<span style="color: black; font-weight: bold;">private</span><span style="color: black;">:</span>
<span style="color: black;"> </span><span style="color: maroon;">volatile</span><span style="color: black;"> mutex theMutex;</span>
<span style="color: black;">};</span></pre>
<br />
this approach works but it will slow down your software, hiding other problems around and, most of all, introduces useless synchronization; a mutex lock is not exactly a cheap operation.<br />
<br />
The idea is to use the technique above but without using a lock, GCC gives us some functions for atomic memory access, and we can use for example:<br />
<br />
<pre style="background-color: #eeeeee; border: 1px dashed; color: black; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; font-size: 12px; overflow: auto;"><code>bool __sync_bool_compare_and_swap (</code><var>type</var><code> *ptr, </code><var>type</var><code> oldval </code><var>type</var><code> newval, ...)</code></pre>
<br />
for our very purpose. That function assigns at *ptr the value newval only if the current value of *ptr is oldval, it returns true if the comparison is successful and newval was written. We can use it to store the threadId when we enter the critical section, "zeroing" the value when we exit.<br />
<br />
Basically I wrote a class that store (with an atomic operation) the threadID of the thread entering the critical section, and when it leaves forgets about the threadID. This was the result:<br />
<br />
<pre face="Andale Mono,Lucida Console,Monaco,fixed,monospace" size="12px" style="background-color: #eeeeee; border: 1px dashed; color: black; overflow: auto;"><span style="color: green;">#ifndef THREAD_COLLISION_WARNING</span>
<span style="color: green;">#define THREAD_COLLISION_WARNING</span>
<span style="color: green;">#include <stdexcept></span>
<span style="color: green;">#ifdef NDEBUG</span>
<span style="color: green;">#define THREAD_WATCH(obj)</span>
<span style="color: green;">#define SCOPED_WATCH(obj)</span>
<span style="color: green;">#else</span>
<span style="color: green;">#define THREAD_WATCH(obj) ThreadCollisionWarning _##obj;</span>
<span style="color: green;">#define SCOPED_WATCH(obj) ThreadCollisionWarning::ScopedWatch scoped_watch_##obj(_##obj);</span>
<span style="color: green;">#endif</span>
<span style="color: black; font-weight: bold;">class</span><span style="color: black;"> ThreadCollisionWarning {</span>
<span style="color: black; font-weight: bold;">public</span><span style="color: black;">:</span>
<span style="color: black;"> ThreadCollisionWarning()</span>
<span style="color: black;"> :theActiveThread(</span><span style="color: blue;">0</span><span style="color: black;">)</span>
<span style="color: black;"> { }</span>
<span style="color: black;"> ~ThreadCollisionWarning() { }</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">class</span><span style="color: black;"> ScopedWatch {</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">public</span><span style="color: black;">:</span>
<span style="color: black;"> ScopedWatch(ThreadCollisionWarning& aTCW)</span>
<span style="color: black;"> :theWarner(aTCW)</span>
<span style="color: black;"> { theWarner.enter(); }</span>
<span style="color: black;"> ~ScopedWatch() { theWarner.leave(); }</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">private</span><span style="color: black;">:</span>
<span style="color: black;"> ThreadCollisionWarning& theWarner;</span>
<span style="color: black;"> };</span>
<span style="color: black; font-weight: bold;">private</span><span style="color: black;">:</span>
<span style="color: black;"> </span><span style="color: maroon;">void</span><span style="color: black;"> enter() { </span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">if</span><span style="color: black;"> (!__sync_bool_compare_and_swap(&theActiveThread, </span><span style="color: blue;">0</span><span style="color: black;">, pthread_self())) {</span>
<span style="color: black;"> </span><span style="color: grey; font-style: italic;">//gotcha! another thread is trying to use the same class</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">throw</span><span style="color: black;"> std::runtime_error(</span><span style="color: #dd0000;">"Thread Collision"</span><span style="color: black;">);</span>
<span style="color: black;"> }</span>
<span style="color: black;"> }</span>
<span style="color: black;"> </span><span style="color: maroon;">void</span><span style="color: black;"> leave() { </span>
<span style="color: black;"> __sync_fetch_and_xor(&theActiveThread, theActiveThread);</span>
<span style="color: black;"> }</span>
<span style="color: black;"> pthread_t theActiveThread;</span>
<span style="color: black;">};</span>
<span style="color: green;">#endif</span>
</pre>
<br />
The class <span style="font-style: italic;">ThreadCollisionWarning</span> has the responsibility to store the thread using the class (or more in general entering a critical section) while the nested class <span style="font-style: italic;">ScopedWatch</span> is used to notify the entering and the leaving the critical section. Look the implementation of the two ThreadCollisionWarning::enter and ThreadCollisionWarning::leave, the former stores the thread Id only if the old value was 0 the latter just zeroes it. The macros simplify the usage.<br />
<br />
So there we go, the class Shared becomes then:<br />
<br />
<pre style="background-color: #eeeeee; border: 1px dashed; color: black; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; font-size: 12px; overflow: auto;"><span style="color: black; font-weight: bold;">struct</span><span style="color: black;"> Shared {</span>
<span style="color: black;"> </span><span style="color: maroon;">void</span><span style="color: black;"> foo(</span><span style="color: maroon;">char</span><span style="color: black;"> aC) { </span>
<span style="color: black;"> SCOPED_WATCH(Shared)</span>
<span style="color: black;"> ...</span>
<span style="color: black;"> }</span>
<span style="color: black;"> THREAD_WATCH(Shared)</span>
<span style="color: black;">};</span></pre>
<br />
using SCOPED_WATCH we just check that two threads are not using the method foo concurrently.<br />
<br />
Of course the implementation above is not by any mean a complete solution to the problem I exposed at the beginning, it helps and it can be a good start point to create a better tool to detect if someone messed around.Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com0tag:blogger.com,1999:blog-3197434572108789628.post-59887723582192708562008-05-31T00:56:00.001-07:002013-10-31T13:48:34.611-07:00False Sharing hits again!You can ask what "false sharing" is? False sharing is an annoying effect that occurs when two processors apparently do not share any resource but due to undergoing hardware architecture they actually do. For example consider two threads writing in not overlapping memory locations, they do not need any synchronization so happily you are driven to think that you are able to split your data in order to implement a lock less algorithm. Unfortunately you hold a sparkling "centrino duo" processor in where both cores do share the L2 cache and then the data you partition in memory can be mapped on the same cache line. The same scenario is triggered on L1 caches, indeed due to coherency protocols if a thread write to a cache line then the cache line referring the same memory is invalidated on the other processor (cache trashing).<br />
<br />
Consider for example the following case:<br />
<br />
<pre style="background-color: #eeeeee; border: 1px dashed; color: black; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; font-size: 12px; overflow: auto;">
char a[10];
char b[10];
start_thread_that_works_on_a;
start_thread_that_works_on_b;
</pre>
<br />
<br />
Very likely that 20 bytes will lie on contiguous memory location and then will be mapped inside the same single cache line, so each time a thread works on his own vector it invalidates the cache line for the other thread and then the hardware has to write and fetch the cache line in and from memory even if it is not strictly needed.<br />
<br />
I've had to work on an algorithm that had that very issue and the following code, even if useless, exploits the same problem:<br />
<br />
<pre style="background-color: #eeeeee; border: 1px dashed; color: black; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; font-size: 12px; overflow: auto;">
<span style="color: green;">#include <iostream></span>
<span style="color: green;">#include <boost/thread.hpp></span>
<span style="color: black; font-weight: bold;">class</span><span style="color: black;"> threadS {</span>
<span style="color: black; font-weight: bold;">public</span><span style="color: black;">:</span>
<span style="color: black;"> threadS(</span><span style="color: maroon;">unsigned</span><span style="color: black;"> </span><span style="color: maroon;">char</span><span style="color: black;"> *aVector, </span><span style="color: maroon;">unsigned</span><span style="color: black;"> </span><span style="color: maroon;">int</span><span style="color: black;"> aVSize) </span>
<span style="color: black;"> :theVector(aVector),</span>
<span style="color: black;"> theVSize(aVSize)</span>
<span style="color: black;"> { }</span>
<span style="color: black;"> </span><span style="color: maroon;">void</span><span style="color: black;"> </span><span style="color: black; font-weight: bold;">operator</span><span style="color: black;">()() {</span>
<span style="color: black;"> </span><span style="color: maroon;">unsigned</span><span style="color: black;"> </span><span style="color: maroon;">long</span><span style="color: black;"> </span><span style="color: maroon;">long</span><span style="color: black;"> myCounter = </span><span style="color: blue;">100000000</span><span style="color: black;">;</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">while</span><span style="color: black;">(--myCounter) {</span>
<span style="color: black;"> </span><span style="color: black; font-weight: bold;">for</span><span style="color: black;"> (</span><span style="color: maroon;">int</span><span style="color: black;"> i=</span><span style="color: blue;">0</span><span style="color: black;">; i<</span><span style="color: blue;">10</span><span style="color: black;">; ++i) {</span>
<span style="color: black;"> ++theVector[i];</span>
<span style="color: black;"> }</span>
<span style="color: black;"> }</span>
<span style="color: black;"> }</span>
<span style="color: black; font-weight: bold;">private</span><span style="color: black;">:</span>
<span style="color: black;"> </span><span style="color: maroon;">unsigned</span><span style="color: black;"> </span><span style="color: maroon;">char</span><span style="color: black;">* theVector; </span>
<span style="color: black;"> </span><span style="color: maroon;">unsigned</span><span style="color: black;"> </span><span style="color: maroon;">int</span><span style="color: black;"> theVSize; </span>
<span style="color: black;">};</span>
<span style="color: maroon;">int</span><span style="color: black;"> main() </span>
<span style="color: black;">{</span>
<span style="color: black;"> </span><span style="color: maroon;">unsigned</span><span style="color: black;"> </span><span style="color: maroon;">char</span><span style="color: black;"> vectorA[</span><span style="color: blue;">10</span><span style="color: black;">]; </span>
<span style="color: black;"> </span><span style="color: maroon;">unsigned</span><span style="color: black;"> </span><span style="color: maroon;">char</span><span style="color: black;"> vectorB[</span><span style="color: blue;">10</span><span style="color: black;">];</span>
<span style="color: black;"> std::cout << std::hex;</span>
<span style="color: black;"> std::cout << </span><span style="color: #dd0000;">"A:["</span><span style="color: black;"> << (</span><span style="color: maroon;">int</span><span style="color: black;">)&vectorA[</span><span style="color: blue;">0</span><span style="color: black;">] << </span><span style="color: #dd0000;">"-"</span><span style="color: black;"> << (</span><span style="color: maroon;">int</span><span style="color: black;">)&vectorA[</span><span style="color: blue;">9</span><span style="color: black;">] << </span><span style="color: #dd0000;">"]"</span><span style="color: black;"> << std::endl;</span>
<span style="color: black;"> std::cout << </span><span style="color: #dd0000;">"B:["</span><span style="color: black;"> << (</span><span style="color: maroon;">int</span><span style="color: black;">)&vectorB[</span><span style="color: blue;">0</span><span style="color: black;">] << </span><span style="color: #dd0000;">"-"</span><span style="color: black;"> << (</span><span style="color: maroon;">int</span><span style="color: black;">)&vectorB[</span><span style="color: blue;">9</span><span style="color: black;">] << </span><span style="color: #dd0000;">"]"</span><span style="color: black;"> << std::endl;</span>
<span style="color: black;"> threadS threadA(vectorA, </span><span style="color: blue;">10</span><span style="color: black;">);</span>
<span style="color: black;"> threadS threadB(vectorB, </span><span style="color: blue;">10</span><span style="color: black;">);</span>
<span style="color: black;"> boost::thread_group tg;</span>
<span style="color: black;"> tg.create_thread(threadA);</span>
<span style="color: black;"> tg.create_thread(threadB);</span>
<span style="color: black;"> tg.join_all();</span>
<span style="color: black;">}</span></pre>
<br />
<br />
<br />
You should be able to compile and link it with:<br />
<br />
g++ main.cpp -o false_sharing -lboost_thread -O3<br />
<br />
Let see what that codes does.<br />
The class threadS stores the vector on which it will operate. The thread body (the operator()) just increases all the vector elements. As you can see I used the boost thread library to start two threads: threadA and threadB.<br />
<br />
On my system I obtain an execution time that goes from 6 to 8 seconds and the following boundaries for the two vectors:<br />
<br />
B:[bfa7d010-bfa7d019] - A:[bfa7d01a-bfa7d023]<br />
<br />
as you can see vectorB and vectorA are at contiguous memory locations.<br />
<br />
How to eliminate the false sharing in this case? The goal is to have both threads working on an underling different cache line, we can achieve this goal separating both data with some extra bytes. Declaring the two vector bigger than we need it's a dirty and quick way to do it, executing the same program with the following vector declaration:<br />
<br />
<pre face="Andale Mono,Lucida Console,Monaco,fixed,monospace" size="12px" style="background-color: #eeeeee; border: 1px dashed; color: black; overflow: auto;">
<span style="color: black;"> </span><span style="color: maroon;">unsigned</span><span style="color: black;"> </span><span style="color: maroon;">char</span><span style="color: black;"> vectorA[</span><span style="color: blue;">100</span><span style="color: black;">]; </span>
<span style="color: black;"> </span><span style="color: maroon;">unsigned</span><span style="color: black;"> </span><span style="color: maroon;">char</span><span style="color: black;"> vectorB[</span><span style="color: blue;">100</span><span style="color: black;">];</span></pre>
<br />
<br />
I'm able to obtain an execution time that goes from 1 to 1.5 seconds.<br />
<br />
Same problem would happen with a single vector "unsigned char vector[1000]" but with a thread working on elements [0,10] and another thread working on elements [11,20]. I wrote a simple program that creates two threads,one performs writes at locations [0,10] and the other that performs writes at [z,z+10] with z inside the interval [11,100]. The following graph shows the execution time while the bytes of separation between the two data increase; z=11 means that data have a 0 bytes separation.<br />
<br />
<a href="http://4.bp.blogspot.com/_xxyADQu8yXQ/SEF_lhh2sgI/AAAAAAAAAAM/nUC3Twfw5nE/s1600-h/latency.png" onblur="try {parent.deselectBloggerImageGracefully();} catch(e) {}"><img alt="" border="0" id="BLOGGER_PHOTO_ID_5206582926849257986" src="http://4.bp.blogspot.com/_xxyADQu8yXQ/SEF_lhh2sgI/AAAAAAAAAAM/nUC3Twfw5nE/s320/latency.png" style="cursor: pointer; display: block; margin: 0px auto 10px; text-align: center;" /></a><br />
<br />
As you can see as soon the two data "false shared" have 51 bytes separation the execution time collapses from ~8 secs to ~1.8 secs, so just wasting 51 bytes I'm able to obtain a x5 speed up, nice isn't it?Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com0tag:blogger.com,1999:blog-3197434572108789628.post-11593319205791655932007-11-26T07:30:00.000-08:002008-06-18T13:53:22.348-07:00A local variable is "local" after allI have found a code that "sounds" like this:<br /><br /><br /><pre style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;font-family:Andale Mono,Lucida Console,Monaco,fixed,monospace;font-size:12px;"><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">struct</span><span style="color: rgb(0, 0, 0);"> StructureC {</span><br /><span style="color: rgb(0, 0, 0);"> </span><span style="color: rgb(128, 0, 0);">char</span><span style="color: rgb(0, 0, 0);"> * theString;</span><br /><span style="color: rgb(0, 0, 0);">};</span><br /><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">class</span><span style="color: rgb(0, 0, 0);"> ClassCPP : </span><span style="font-weight: bold; color: rgb(0, 0, 0);">public</span><span style="color: rgb(0, 0, 0);"> StructureC {</span><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">public</span><span style="color: rgb(0, 0, 0);">:</span><br /><span style="color: rgb(0, 0, 0);"> ClassCPP();</span><br /><span style="color: rgb(0, 0, 0);"> ClassCPP(</span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> ClassCPP& aClass);</span><br /><br /><span style="color: rgb(0, 0, 0);"> ~ClassCPP();</span><br /><span style="color: rgb(0, 0, 0);">};</span><br /><br /><span style="color: rgb(0, 0, 0);">ClassCPP::ClassCPP() {</span><br /><span style="color: rgb(0, 0, 0);"> std::string myString = foo();</span><br /><br /><span style="color: rgb(0, 0, 0);"> theString = myString.c_str();</span><br /><span style="color: rgb(0, 0, 0);">}</span><br /><br /><span style="color: rgb(0, 0, 0);">ClassCPP::~ClassCPP() {</span><br /><span style="color: rgb(0, 0, 0);">}</span><br /><br /><span style="color: rgb(0, 0, 0);">ClassCPP::ClassCPP(</span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> ClassCPP& aClass) {</span><br /><span style="color: rgb(0, 0, 0);"> theString = aClass.theString;</span><br /><span style="color: rgb(0, 0, 0);">}</span><br /></pre><br /><br />someone failed here, badly!<br /><br />Let's see.<br /><br />Of course the code above is not useful, it was simplified and extracted from a real case; StructureC can not be changed at all.<br /><br />std::string has the method c_str() that returns a null terminated sequence of characters (same content as std::string) and it points to an internal location of std::string, when the scope of myString (note how I use the prefix "my" for local variables) ends, any reference to its internal status is then not valid, unfortunately theString is a member of the object being constructed. The code can "work" but then you are very lucky if it does.<br /><br />The correct way is to allocate memory in the constructor and then copy the character sequence:<br /><br /><pre style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;font-family:Andale Mono,Lucida Console,Monaco,fixed,monospace;font-size:12px;"><br /><span style="color: rgb(0, 0, 0);">ClassCPP::ClassCPP() {</span><br /><span style="color: rgb(0, 0, 0);"> std::string myString = foo();</span><br /><br /><span style="color: rgb(0, 0, 0);"> theString = </span><span style="font-weight: bold; color: rgb(0, 0, 0);">new</span><span style="color: rgb(0, 0, 0);"> </span><span style="color: rgb(128, 0, 0);">char</span><span style="color: rgb(0, 0, 0);">[myString.size()+</span><span style="color: rgb(0, 0, 255);">1</span><span style="color: rgb(0, 0, 0);">]; </span><span style="font-style: italic; color: rgb(128, 128, 128);">//+1 to store the null termination</span><br /><span style="color: rgb(0, 0, 0);"> strcpy(theString, myString.c_str());</span><br /><span style="color: rgb(0, 0, 0);">}</span></pre><br /><br />so, you think that's all, don't you?<br /><br />Still some errors left.<br /><br /><span style="font-style: italic;">Copy</span> constructor shall be rewritten, if we leave it unchanged like this<br /><br /><pre style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;font-family:Andale Mono,Lucida Console,Monaco,fixed,monospace;font-size:12px;"><br /><span style="color: rgb(0, 0, 0);">ClassCPP::ClassCPP(</span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> ClassCPP&amp; aClass) {</span><br /><span style="color: rgb(0, 0, 0);"> theString = aClass.theString;</span><br /><span style="color: rgb(0, 0, 0);">}</span><br /></pre><br /><br />then as soon we copy an object of type ClassCPP we will have two instances that are pointing to the same memory area (the one that contains the string).<br /><br />So it'd be better to write it in the correct way:<br /><br /><pre style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;font-family:Andale Mono,Lucida Console,Monaco,fixed,monospace;font-size:12px;"><br /><span style="color: rgb(0, 0, 0);">ClassCPP::ClassCPP(</span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> ClassCPP&amp; aClass) {</span><br /><span style="color: rgb(0, 0, 0);"> theString = </span><span style="font-weight: bold; color: rgb(0, 0, 0);">new</span><span style="color: rgb(0, 0, 0);"> </span><span style="color: rgb(128, 0, 0);">char</span><span style="color: rgb(0, 0, 0);">[strlen(aClass.theString)+</span><span style="color: rgb(0, 0, 255);">1</span><span style="color: rgb(0, 0, 0);">];</span><br /><span style="color: rgb(0, 0, 0);"> strcpy(theString, aClass.theString); </span><br /><span style="color: rgb(0, 0, 0);">}</span></pre><br /><br />I could have used a strdup but strdup uses malloc to allocate memory.<br /><br />Having done dynamic allocation of memory the destructor can not be void, we need to release the memory allocated:<br /><br /><pre style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;font-family:Andale Mono,Lucida Console,Monaco,fixed,monospace;font-size:12px;"><br /><span style="color: rgb(0, 0, 0);">ClassCPP::~ClassCPP() {</span><br /><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">delete</span><span style="color: rgb(0, 0, 0);"> []theString;</span><br /><span style="color: rgb(0, 0, 0);">}</span></pre><br />still some problems left, incredible how many errors can be done in a few lines of code!<br /><br /><span style="font-style: italic;">Assignment </span>operator shall be written as well but in this case we can declare it<br />private and not implement it:<br /><br /><pre style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;font-family:Andale Mono,Lucida Console,Monaco,fixed,monospace;font-size:12px;"><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">class</span><span style="color: rgb(0, 0, 0);"> ClassCPP : </span><span style="font-weight: bold; color: rgb(0, 0, 0);">public</span><span style="color: rgb(0, 0, 0);"> StructureC {</span><br /><span style="color: rgb(0, 0, 0);"> ...</span><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">private</span><span style="color: rgb(0, 0, 0);">:</span><br /><span style="color: rgb(0, 0, 0);"> </span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> ClassCPP &amp; </span><span style="font-weight: bold; color: rgb(0, 0, 0);">operator</span><span style="color: rgb(0, 0, 0);">=(</span><span style="color: rgb(128, 0, 0);">const</span><span style="color: rgb(0, 0, 0);"> ClassCPP&amp;);</span><br /><span style="color: rgb(0, 0, 0);">};</span></pre><br /><br />in this way we can check if someone needs it (the original implementation didn't<br />have it implemented so I guess the intention was: "I don't need it") and if necessary<br />implement it.<br /><br />The problems are not over yet, look at the following usage of that code:<br /><br /><pre style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;font-family:Andale Mono,Lucida Console,Monaco,fixed,monospace;font-size:12px;"><br /><code><br /><span style="color: rgb(0, 0, 0);">StructureC *a = </span><span style="font-weight: bold; color: rgb(0, 0, 0);">new</span><span style="color: rgb(0, 0, 0);"> ClassCPP;</span><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">delete</span><span style="color: rgb(0, 0, 0);"> a;</span></code></pre><br /><br />as you can see deleting an instance of ClassCPP through a pointer to its base class will not call the ClassCPP destructor. Then we need to declare the destructor of StructureC virtual but given the fact we can not change StructureC then we need avoiding someone being able to build a ClassCPP in the heap memory. This can be done declaring and not implementing the operator new in the private part of class.<br /><br /><pre style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;font-family:Andale Mono,Lucida Console,Monaco,fixed,monospace;font-size:12px;"><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">class</span><span style="color: rgb(0, 0, 0);"> ClassCPP : </span><span style="font-weight: bold; color: rgb(0, 0, 0);">public</span><span style="color: rgb(0, 0, 0);"> StructureC {</span><br /><span style="color: rgb(0, 0, 0);"> ...</span><br /><span style="font-weight: bold; color: rgb(0, 0, 0);">private</span><span style="color: rgb(0, 0, 0);">:</span><br /><span style="color: rgb(0, 0, 0);"> ...</span><br /><span style="color: rgb(0, 0, 0);"> </span><span style="color: rgb(128, 0, 0);">void</span><span style="color: rgb(0, 0, 0);"> * </span><span style="font-weight: bold; color: rgb(0, 0, 0);">operator</span><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">new</span><span style="color: rgb(0, 0, 0);">(std::size_t);</span><br /><span style="color: rgb(0, 0, 0);"> </span><span style="color: rgb(128, 0, 0);">void</span><span style="color: rgb(0, 0, 0);"> * </span><span style="font-weight: bold; color: rgb(0, 0, 0);">operator</span><span style="color: rgb(0, 0, 0);"> </span><span style="font-weight: bold; color: rgb(0, 0, 0);">new</span><span style="color: rgb(0, 0, 0);">(std::size_t, </span><span style="color: rgb(128, 0, 0);">void</span><span style="color: rgb(0, 0, 0);"> *);</span><br /><span style="color: rgb(0, 0, 0);">};</span></pre><br /><br />as you can see in order to avoid any mistake better to disable the in-place operator<br />new as well.Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com0tag:blogger.com,1999:blog-3197434572108789628.post-43518443319088559092006-12-15T09:19:00.000-08:002008-06-03T02:46:26.084-07:00Exceptions (part 2)So we have seen on my last post that throwing exceptions and functions call have some similarities, we have also seen that throw an object always means copy it even if we catch for reference, as all copies in C++ are based on the static type then even in the exceptions environments the objects thrown are base on the static type.<br /><br /><br />Not always throw an exception assures you to avoid memory leakages even using "automatic objects", consider this example:<br /><br /><pre face="Andale Mono,Lucida Console,Monaco,fixed,monospace" size="12px" style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;"><br /><code><br />class Foo {<br /> public:<br /> Foo();<br /> ~Foo()<br /> private:<br /> AType* aPointer;<br /> BType* bPointer;<br />};<br /><br />Foo::Foo()<br />:aPointer(new AType),<br /> bPointer(new BType)<br />{ }<br /></code><br /></pre><br />we know that the initializer list order depends on the order declaration in the definition of class,<br />so in this case aPointer is initialized first then bPointer. What happens if the "new BType" throws an exception? Well, given the fact aPointer is a plain pointer then we will have memory leakage. So a first thought can be to use not plain pointers but something like "smart pointer".<br />So a first approach can be the following:<br /><br /><pre style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); font-size: 12px; line-height: 14px; width: 100%;"><br /><code><br />class Foo {<br /> public:<br /> Foo();<br /> ~Foo()<br /> private:<br /> std::auto_ptr aPointer;<br /> std::auto_ptr<btype> bPointer;<br />};<br /><br />Foo::Foo()<br />:aPointer(new AType),<br /> bPointer(new BType)<br />{ }<br /></code><br /></pre> <br />well this is still not safe. Let see the constructor execution sequence:<br /><br />1) Constructor AType is executed (new AType)<br />2) Constructor BType is executed (new BType)<br />3) Constructor std::auto_ptr<atype> is executed ( aPointer( ... ) )<br />4) Constructor std::auto_ptr<btype> is executed ( bPointer( ... ) )<br /><br />do you see know where the problem is? If still "new BType" throws an exception the address of memory allocated by new AType was still not saved anywhere; unfortunately the correct way to solve this problem is the following:<br /><br /><br /><pre style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); font-size: 12px; line-height: 14px; width: 100%;"><br /><code><br />class Foo {<br /> public:<br /> Foo();<br /> ~Foo()<br /> private:<br /> std::auto_ptr<atype> aPointer;<br /> std::auto_ptr<btype> bPointer;<br />};<br /><br />Foo::Foo()<br />:aPointer(),<br /> bPointer()<br />{<br /> aPointer = std::auto_ptr<atype>(new AType);<br /> bPointer = std::auto_ptr<btype>(new BType);<br />}<br /></code><br /></pre> <br /><br />throwing an exception can also leave the object in an inconsistent state, consider the following class (do not consider the fact that the class is useless):<br /><br /><pre style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); font-size: 12px; line-height: 14px; width: 100%;"><br /><code><br />class Foo {<br /> public:<br /> Foo()<br /> :theStorage()<br /> { }<br /><br /> addInt(int anInteger) {<br /> theStorage.push_back(anInteger);<br /> }<br /><br /> void sumOne() {<br /> int i;<br /> for (i=0; i < theStorage.size(); ++i) {<br /> theStorage[i] += 1;<br /> if (i==2) {<br /> throw std::runtime_error("OPS!");<br /> }<br /> }<br /><br /> private:<br /> std::vector<int> theStorage;<br />};<br /></code><br /></pre> <br /><br />and his usage:<br /><br /><pre style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); font-size: 12px; line-height: 14px; width: 100%;"><br /><code><br />Foo aFoo;<br /><br />aFoo.addInt(0);<br />aFoo.addInt(1);<br />aFoo.addInt(2);<br />aFoo.addInt(3);<br /></code><br /></pre> <br /><br />at this point calling:<br /><br /><pre style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; font-family: Andale Mono,Lucida Console,Monaco,fixed,monospace; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); font-size: 12px; line-height: 14px; width: 100%;"><br /><code><br />aFoo.sumOne();<br /></code><br /></pre> <br /><br />will throw an exception leaving aFoo with partial updated elements, and from user point of view the aFoo is in an inconsistent state, so the sumOne() function shows here another problem that can break the exception safety of a class. The solution on this kind of problems is to work on a copy of internal class state and then make a swap between the internal state and the modified state.Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com0tag:blogger.com,1999:blog-3197434572108789628.post-14695928478685545462006-11-28T03:19:00.000-08:002008-06-03T02:39:07.269-07:00Exceptions (part 1)As you already know a modern way to deal with "errors" in C++ is the exception handling; however you need to be careful on using this mechanism. Let see how it works and some tips as well.<br />The exception mechanism is based on the <span style="font-style: italic;"><span style="font-weight: bold;">try - catch </span></span>blocks:<br /><br /><br /><pre face="Andale Mono,Lucida Console,Monaco,fixed,monospace" size="12px" style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;"><br /><code><br />try {<br /> //some code in here that we <span style="font-style: italic;">attempt </span>to execute<br />}<br />catch (...) {<br /> // this block is the error handling, the code in this <br /> // block is executed if the code in the<br /> // try block above have thrown an exception<br />}<br /></code><br /></pre><br /><br />easy and net.<br /><br />Let see how throw an exception so we can see more in depth what this mechanism offers, how use it, what to avoid.<br />An exception is thrown with a <span style="font-style: italic;"><span style="font-weight: bold;">throw:</span></span><span><span><br /><br /><pre face="Andale Mono,Lucida Console,Monaco,fixed,monospace" size="12px" style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;"><br /><code><br />throw A;<br /></code><br /></pre><br /><br />where A is the type of the object thrown, in that case we are throwing an object of type A inizialized with his default constructor, we could have done:<br /><br /><pre face="Andale Mono,Lucida Console,Monaco,fixed,monospace" size="12px" style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;"><br /><code><br />throw A(3, "foo");<br /></code><br /></pre><br /><br />or even:<br /><br /><pre face="Andale Mono,Lucida Console,Monaco,fixed,monospace" size="12px" style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;"><br /><code><br />A a(3, "foo");<br />throw a;<br /></code><br /></pre><br /><br />The catch(...) { } handler is supposed to handle all kind of exception that code inside the try block throws, in this way we lose the kind of exception thrown so it's a bit reductive because the error handler doesn't have a clue on what is going on; fortunately is possible to specify wich kind of exceptions we want to manage (we as well ignore some).<br />Specify which kind of exception we want manage is done in this way:<br /><br /><pre face="Andale Mono,Lucida Console,Monaco,fixed,monospace" size="12px" style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;"><br /><code><br />catch(Foo f) {<br />}<br /></code><br /></pre><br /><br />we can have multiple catch blocks after a try:<br /><br /><br /><pre face="Andale Mono,Lucida Console,Monaco,fixed,monospace" size="12px" style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;"><br /><code><br />try {<br /> //code we are attempting to execute<br />}<br />catch(Foo f) {<br /> // error handler in case a Foo type was thrown<br />}<br />catch(Bar b) {<br /> // error handler in case a Bar type was thrown<br />}<br /></code><br /></pre><br /><br />if the code in the try block thrown an exception that is not Foo and neither Bar then is like we are not using the try-catch blocks ( apart the introduction of the try's extra scope ).<br /><br />We can obtain the same behaviour (logging for example we were not able to catch any expected exception) in this way:<br /><br /><pre face="Andale Mono,Lucida Console,Monaco,fixed,monospace" size="12px" style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;"><br /><code><br />try {<br /> //code we are attempting to execute<br />}<br />catch(Foo f) {<br /> // error handler in case a Foo type was thrown<br />}<br />catch(Bar b) {<br /> // error handler in case a Bar type was thrown</span></span><br />}<br />catch(...) {<br /> // log the event in here<br /> throw; // this throws again the same exception.<br />}<br /></code><br /></pre><br /><br />There is still something behind all this. As you have seen the <span style="font-weight: bold;">catch </span>blocks are very similar to a function declaration where the arguments are passed by value.<br />The <span style="font-weight: bold;">catch </span>blocks can have indeed all kind of parameters:<br /><br />catch( T )<br />catch( T & )<br />catch( T * )<br />catch( const T )<br />catch( const T & )<br />catch( const T * )<br /><br />so you can think of throwing an exception have same effect of calling a function, but is not.<br />Consider this code:<br /><br /><pre face="Andale Mono,Lucida Console,Monaco,fixed,monospace" size="12px" style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;"><br /><code><br />void foo()<br />{<br /> ...<br /> A aLocalObject;<br /> ...<br /><br /> throw aLocalObject <br />}<br /></code><br /></pre><br /><br />and the call of foo is inside a try block:<br /><br /><pre face="Andale Mono,Lucida Console,Monaco,fixed,monospace" size="12px" style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;"><br /><code><br />try {<br /> foo();<br />}<br />catch( A anException) {<br /><br />}<br /></code><br /></pre><br /><br /><br />in this case, as the catch "signature" suggests, the anException is a copy of aLocalObject so<br />no problem with it, but what if we catch by reference?<br /><br /><pre face="Andale Mono,Lucida Console,Monaco,fixed,monospace" size="12px" style="border: 1px dashed rgb(153, 153, 153); padding: 5px; overflow: auto; color: rgb(0, 0, 0); background-color: rgb(238, 238, 238); line-height: 14px; width: 100%;"><br /><code><br />catch( A & anException ) {<br />}<br /></code><br /></pre><br /><br />in this case we can think to have a reference to a destroyed object ( when an exception is thrown is not like a function call and all the local variable on that scope are destroyed ), well this is not the case indeed even if you catch an exception by reference the c++ runtime support will perform a copy of the object thrown, and this happens <span style="font-weight: bold;">always</span>, you can not avoid this copy even if the object will not be destroyed going out of scope ( a static variable for example ).<br /><br />So in case of:<br /><br />catch( A ) { }<br /><br />you have 2 copies performed, in case of:<br /><br />catch ( A & )<br /><br />you have just one copy. For this very reason is not possible to modify the object thrown, because you have on the catch block a copy of it.<br /><br />Next week the second and last part about exceptions.Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com0tag:blogger.com,1999:blog-3197434572108789628.post-27219321611082107932006-11-22T05:51:00.000-08:002008-06-03T02:46:02.633-07:00operator new - new operatorAt first shot this two entities can appear to be the same; however they are not.<br />Let see what they are, what you can change and the safe rules to handle them.<br /><br />First of all let see what happens when you write something like: <br /><br /><div style="text-align: center;"> C * pC = new C;<br /></div><ul><li>Enough memory is allocated to contain the object requested</li><li>The constructor of C is called to initialize the object the lays in the memory allocated<br /></li></ul>This described is the<span style="font-weight: bold;"> new</span> <span style="font-weight: bold;">operator </span>behave and you can not change the way he acts.<br /><br />The first point in the sequence above is the only think you can change, the way the memory is allocated, the <span style="font-weight: bold;">new operator </span>uses for this task what is called: <span style="font-weight: bold;">operator new</span>.<br /><br />So the pseudo code for <span style="font-style: italic;">C * pC = new C;</span> could be:<br /><ol><li>Call <span style="font-weight: bold;">operator new</span></li><li><span style="font-weight: bold;"><span style="font-weight: bold;"><span style="font-weight: bold;"></span></span></span><span style="font-weight: bold;"><span style="font-weight: bold;"><span style="font-weight: bold;"></span></span></span>Construct an object of the type request at the location returned from previous step</li></ol>The operator new signature is something like this:<br /><br /> void * operator new(size_t);<br /><br />so if you want change the way the <span style="font-weight: bold;">new operator</span> allocates the memory for your type then you need to rewrite the <span style="font-weight: bold;">operator new</span>.<br />As you already know when you specify a name in a scope ( for example a method name in a class ) this will hide the same name in the scopes that are containing your actual ( the base class scope for example ). So rewriting your <span style="font-weight: bold;">operator new</span> what you do is to hide the other forms of <span style="font-weight: bold;">operator new. </span>For instance these forms are:<br /><ol><li>void * operator new(std::size_t, std::nothrow_t) throw();</li><li>void * operator new(std::size_t, void *);</li></ol>the former is the <span style="font-style: italic;">nothrow new </span>the latter is the <span style="font-style: italic;">in place new. </span>Actualy you can break more than this if you define your own operator new, something like:<br /><ul><li>void * operator new(std::size_t, T);</li></ul>remember in this case that first argument of <span style="font-weight: bold;">operator new </span>shall be always std::size_t, in this case you will hide not only the "less common" <span style="font-weight: bold;">operator new</span> version but also the plain new one. I quoted <span style="font-style: italic;">less common </span>because in reality the STL does heavy usage of <span style="font-style: italic;">in place new.</span>Gaetanohttp://www.blogger.com/profile/03637292910769902384noreply@blogger.com0