Home » Developing U++ » U++ Developers corner » Core chat... (reference counted pointers etc.)
| Core chat... (reference counted pointers etc.) [message #12298] |
Tue, 23 October 2007 23:48  |
 |
mirek
Messages: 13975
Registered: November 2005
|
Ultimate Member |
|
|
EDIT: by fudadmin - This topic is a pure gem and one day should go into documentation. I am making it sticky.
| mdelfede wrote on Tue, 23 October 2007 15:47 |
| luzr wrote on Tue, 23 October 2007 19:45 |
Well, I think At is extremely effective way how to solve many problems. And similar issue existis even in the plain C
a[i++] = a[i];
it is simply something you have to care about...
|
Well, in your example, you can see that you're doing it wrong.
In theide bug, you can't.... or, at least, you can't if you don't know upp vector internals, what's normally the matter.
|
Wait a moment - this issue is quite rigirously documented. There is precisely defined which container methods invalidate references.
| Quote: |
Usually people see Vector<> as a blackbox, they don't know (at least, I didn't know) about invalid references caused by At().
|
http://www.ultimatepp.org/src$Core$Vector$en-us.html
Anyway, as long as you know what At does, I guess this is quite obvious - but I agree that it is a potential source of bugs. There are several gotachas in U++ like this; I guess we should have some document created...
| Quote: |
BTW, I still think that c++ standard is missing many useful constructs, one of the most useful ot them is the 'property' one, which borland introduced as a language extension in their compilers.
An object-oriented language without properties is an empty wine glass, I think 
Just an example of it :
class C
{
private:
int iVal;
void set_iVal(int _i) { i = _iVal; }
int get_iVal(void) { return iVal; }
public:
__property i = { read = getIval, write = setIval };
};
C c ;
c.i = 5; // calls set_Ival(5)
int a = c.i; // calls a = get_Ival()
That's what I call 'clean object-oriented language'.
|
I am still not quite sure why people insist on such verbose syntactic sugar (it is really nothing else).
IME/IMO, there is usually much more properties to set than get and setting properties is much more convenient U++/C++ way...
Mirek
[Updated on: Wed, 16 February 2011 11:15] Report message to a moderator |
|
|
|
|
|
|
|
|
|
| Re: Core chat... [message #12317 is a reply to message #12298] |
Wed, 24 October 2007 17:46  |
mdelfede
Messages: 1307
Registered: September 2007
|
Ultimate Contributor |
|
|
| luzr wrote on Tue, 23 October 2007 23:48 |
I am still not quite sure why people insist on such verbose syntactic sugar (it is really nothing else).
IME/IMO, there is usually much more properties to set than get and setting properties is much more convenient U++/C++ way...
|
Well, if you think so, you can have object-oriented code in plain C also, look in GTK code to have just an example.... or even in assembler.
That doesn't mean 'clean object programming' in my way of thinking.
Please understand that I'm not criticizing UPP, I still think it's one of the best coded toolkits.
What I do criticise is the lack of some very useful constructs in C++ that could make code much cleaner to write and to mantain.
Constructs that don't cause loss of performance, as properties.
| Quote: |
Wait a moment - this issue is quite rigirously documented. There is precisely defined which container methods invalidate references.
|
Yes, all is documented somewhere, you did know that but... still the bug was there. In my thinking, for example, a function that has the main purpose of accessing array element should *not* as a side effect invalidate references. I know that write
a.SetSize(100);
a[99] = 10;
is one line longer than
but in former case you clearly separate array dimensioning from accessing, in latter not. And more, you have a sure cause of nasty bugs on latter one. Such constructs are (IMHO) acceptable in dynamic languages such visual basic, not in C++.
You where complaining about shared ownership of objects (and you're right about it, if you use pointers), but in case of At() function you do have a case that is very similar, because you use [] operator that you think operates in an object that is invalidated by the At() function as a side effect.
Ok, it's documented, but then also realloc() is documented, but I've seen MANY times code like this :
int * a = malloc(100);
realloc(a, 200);
*a = 10;
and that's MUCH less subtle bug than yours with At().
Ciao
Max
|
|
|
|
| Re: Core chat... [message #12318 is a reply to message #12317] |
Wed, 24 October 2007 18:23 |
|
mirek
Messages: 13975
Registered: November 2005
|
Ultimate Member |
|
|
| mdelfede wrote on Wed, 24 October 2007 11:46 |
| luzr wrote on Tue, 23 October 2007 23:48 |
I am still not quite sure why people insist on such verbose syntactic sugar (it is really nothing else).
IME/IMO, there is usually much more properties to set than get and setting properties is much more convenient U++/C++ way...
|
Well, if you think so, you can have object-oriented code in plain C also, look in GTK code to have just an example.... or even in assembler.
That doesn't mean 'clean object programming' in my way of thinking.
Please understand that I'm not criticizing UPP, I still think it's one of the best coded toolkits.
What I do criticise is the lack of some very useful constructs in C++ that could make code much cleaner to write and to mantain.
Constructs that don't cause loss of performance, as properties.
|
Well, in any case, GTK code is much more verbose than C++. U++ "modifiers" are less verbose than properties.
| Quote: |
Wait a moment - this issue is quite rigirously documented. There is precisely defined which container methods invalidate references.
|
Yes, all is documented somewhere, you did know that but... still the bug was there. In my thinking, for example, a function that has the main purpose of accessing array element should *not* as a side effect invalidate references.
[/quote]
I agree. However, the main purpose of At is not accessing array element, but to create one if it does not exist yet.
| Quote: |
I know that write
a.SetSize(100);
a[99] = 10;
is one line longer than
|
But that is not the correct equivalent.
| Quote: |
if(a.GetCount() < 100)
a.SetCount(100)
a[99] = 10;
|
is the equivalent. OK, that would be something to live with, but consider composition:
Vector< Vector<int> > a;
a.At(10).At(20) = 10;
- that is the real reason to provide such utility method.
| Quote: |
You where complaining about shared ownership of objects (and you're right about it, if you use pointers), but in case of At() function you do have a case that is very similar, because you use [] operator that you think operates in an object that is invalidated by the At() function as a side effect.
Ok, it's documented, but then also realloc() is documented, but I've seen MANY times code like this :
int * a = malloc(100);
realloc(a, 200);
*a = 10;
and that's MUCH less subtle bug than yours with At().
|
Well, I guess there is always instrinsict conflict between performance, convenience and safety. "At", and in fact, the whole concept of "inplace creation" (instances get created in container, reference is returned) is nothing new, it is part of U++ design since the very beginning and proved to be very useful.
In any case, I believe I would make more bugs in "unrolling"
a.At(x).At(y) = ...
code than using such utility method.
Mirek
[Updated on: Wed, 24 October 2007 18:23] Report message to a moderator |
|
|
|
| Re: Core chat... [message #12319 is a reply to message #12298] |
Wed, 24 October 2007 19:49 |
sergei
Messages: 94
Registered: September 2007
|
Member |
|
|
| luzr wrote on Tue, 23 October 2007 23:48 |
| mdelfede wrote on Tue, 23 October 2007 15:47 |
| luzr wrote on Tue, 23 October 2007 19:45 |
Well, I think At is extremely effective way how to solve many problems. And similar issue existis even in the plain C
a[i++] = a[i];
it is simply something you have to care about...
|
Well, in your example, you can see that you're doing it wrong.
In theide bug, you can't.... or, at least, you can't if you don't know upp vector internals, what's normally the matter.
|
|
Could someone please explain the workings of a[i++] = a[i]? I guess something here goes wrong because i++ probably gets evaluated earlier than rvalue (it it?), but I'm not sure why/what. And does this happen with Vector too (reference on left evaluated before rvalue)?
P.S. on-topic - I consider At() behaviour just fine, since if you don't want to allocate anything, just use [].
|
|
|
|
|
|
| Re: Core chat... [message #12321 is a reply to message #12318] |
Wed, 24 October 2007 20:24 |
mdelfede
Messages: 1307
Registered: September 2007
|
Ultimate Contributor |
|
|
| luzr wrote on Wed, 24 October 2007 18:23 |
Well, I guess there is always instrinsict conflict between performance, convenience and safety.
|
Speaking about performance, I agree... it shouldn't traded with convenience. With safety.... hmmm... there I have some reserve.
But, in modern compilers, doing a 3 line construct or an equivalent 1 liner At() should be the same in terms of performance.
BTW, I really see few practical usage of At(); normally (about 100% of cases) you know in advance your needed array size, and, if you don't, I don't really see the point of increasing (and such copying the whole stuff) the array on a 1 element basis; se that example :
Array<int> a(1);
for(int i = 2, i < 1000; i++)
a.At(i) = i;
of course that works, but that means, in the worst case of a badly written Array code, 998 realloc() calls, with 998 buffer copy, memory releases and allocations, ecc ecc.
In a better and more realistic case, if the array is grown in chunks of 10 elements, you'd still have to do it 98 times. That's no efficiency neither good code.
Of course there's a better solution (here I added 1 member functions to Array class) :
Array<int> a(1);
for(int i = 2, i < 1000; i++)
{
a.CheckSize(i, 1000);
a[i] = i;
}
Where the CheckSize(minSize, increment) is a member function that checks the actual/required size of array and, if too small, make it to grow of a specified size (here 1000).
That'd be an huge preformance gain with a small line of code added.
| Quote: |
"At", and in fact, the whole concept of "inplace creation" (instances get created in container, reference is returned) is nothing new, it is part of U++ design since the very beginning and proved to be very useful.
In any case, I believe I would make more bugs in "unrolling"
a.At(x).At(y) = ...
code than using such utility method.
|
Here I'd let the At() the sole purpose of element accessor and put somewhere else the array resize. I don't see nothing bad on a.At().At().At(), as they don't change array sizes.
Ciao
Max
|
|
|
|
| Re: Core chat... [message #12322 is a reply to message #12319] |
Wed, 24 October 2007 21:46 |
|
mirek
Messages: 13975
Registered: November 2005
|
Ultimate Member |
|
|
| sergei wrote on Wed, 24 October 2007 13:49 |
| luzr wrote on Tue, 23 October 2007 23:48 |
| mdelfede wrote on Tue, 23 October 2007 15:47 |
| luzr wrote on Tue, 23 October 2007 19:45 |
Well, I think At is extremely effective way how to solve many problems. And similar issue existis even in the plain C
a[i++] = a[i];
it is simply something you have to care about...
|
Well, in your example, you can see that you're doing it wrong.
In theide bug, you can't.... or, at least, you can't if you don't know upp vector internals, what's normally the matter.
|
|
Could someone please explain the workings of a[i++] = a[i]? I guess something here goes wrong because i++ probably gets evaluated earlier than rvalue (it it?), but I'm not sure why/what. And does this happen with Vector too (reference on left evaluated before rvalue)?
|
The problem is that the order of evaluation is left to the compiler, it is undefined (by C/C++ standard). Therefore, such statement has undefined semantics.
Mirek
[Updated on: Wed, 24 October 2007 22:06] Report message to a moderator |
|
|
|
| Re: Core chat... [message #12323 is a reply to message #12321] |
Wed, 24 October 2007 22:05 |
|
mirek
Messages: 13975
Registered: November 2005
|
Ultimate Member |
|
|
| mdelfede wrote on Wed, 24 October 2007 14:24 |
| luzr wrote on Wed, 24 October 2007 18:23 |
Well, I guess there is always instrinsict conflict between performance, convenience and safety.
|
But, in modern compilers, doing a 3 line construct or an equivalent 1 liner At() should be the same in terms of performance.
|
Nobody argues that...
| Quote: |
BTW, I really see few practical usage of At();
|
Well, in that case, just do not use it:)
Anyway, it is used about 100 times in the uppsrc. There are many cases when it is the right thing to do - if you do not want to duplicate the code.
| Quote: |
normally (about 100% of cases) you know in advance your needed array size
|
Wrong, in 99% you do not know the GetCount of resulting Vector or Array. Or you do not WANT to know it - I mean, sure, it is e.g. possible to count all elements in the file first, then create the array of the right size, the reopen the file and load them in. But load them in into dynamic array is much more simple...
| Quote: |
, and, if you don't, I don't really see the point of increasing (and such copying the whole stuff) the array on a 1 element basis; se that example :
Array<int> a(1);
for(int i = 2, i < 1000; i++)
a.At(i) = i;
of course that works, but that means, in the worst case of a badly written Array code, 998 realloc() calls, with 998 buffer copy, memory releases and allocations, ecc ecc.
|
Well but that is completely different issue altogether... But be sure that Vector code is not badly written
| Quote: |
In a better and more realistic case, if the array is grown in chunks of 10 elements, you'd still have to do it 98 times. That's no efficiency neither good code.
|
Growing by static chunks is very stupid method. You always need exponential growth - this is the same for NTL and STL. In that case, the total number of copying stuff is amortized constant - both for STL and NTL (but for NTL, unlike STL, the copy of Vector element is performed by raw binary move, which can be much faster).
| Quote: |
Where the CheckSize(minSize, increment) is a member function that checks the actual/required size of array and, if too small, make it to grow of a specified size (here 1000).
|
Works well speed wise only for small array sizes - but for small ones, it wastes memory. What you really need is exponential growth.
| Quote: |
That'd be an huge preformance gain with a small line of code added.
|
Current At method is more optimal.
| Quote: |
Here I'd let the At() the sole purpose of element accessor and put somewhere else the array resize. I don't see nothing bad on a.At().At().At(), as they don't change array sizes.
|
Why should you duplicate operator[] with a method?
BTW, if you want to study easy to undestand practical examples of using At, look at ArrayCtrl::SetDisplay or Switch::Set.
Mirek
|
|
|
|
| Re: Core chat... [message #12324 is a reply to message #12323] |
Wed, 24 October 2007 23:36 |
mdelfede
Messages: 1307
Registered: September 2007
|
Ultimate Contributor |
|
|
| luzr wrote on Wed, 24 October 2007 22:05 |
Well, in that case, just do not use it:)
|
Impossible, if some library that I use do use it 
| Quote: |
Well but that is completely different issue altogether... But be sure that Vector code is not badly written
|
I never thought that That was only an example of how it can be made different. Your Vector::Checksize() can also be made working exponentially, just drop the second argument and change code inside it... even more easy to use.
a.CheckSize(i);
a[i] = i;
in previous example. The matter doesn't change. All you spare with At() is a line of code at a cost of the danger of hidden bugs.....
| Quote: |
Growing by static chunks is very stupid method. You always need exponential growth - this is the same for NTL and STL. In that case, the total number of copying stuff is amortized constant - both for STL and NTL (but for NTL, unlike STL, the copy of Vector element is performed by raw binary move, which can be much faster).
|
I told you that I'm not the boggest fan of STL
| Quote: |
Current At method is more optimal.
|
More than my example with linear/constant growth, ok.
But with exponential growth, all you spare is a line of code.
| Quote: |
Why should you duplicate operator[] with a method?
|
You shouldn't. If the only purpose of At() is allow creating elements on the fly just before accessing them, I see on it no true benefit, besides some 20 keystrokes less typing.
| Quote: |
BTW, if you want to study easy to undestand practical examples of using At, look at ArrayCtrl::SetDisplay or Switch::Set.
|
I'll look for it next days, thanx !
Ciao
Max
|
|
|
|
|
|
| Re: Core chat... [message #12331 is a reply to message #12325] |
Thu, 25 October 2007 14:06 |
mdelfede
Messages: 1307
Registered: September 2007
|
Ultimate Contributor |
|
|
| luzr wrote on Thu, 25 October 2007 05:33 |
Consider composition a.At(x).At(y)... It can become tedious.
|
yes, but finding At() kind of bugs can be even more tedious...
There you could write :
a.CheckSize(x).CheckSize(y);
a[x][y];
if [][] operators can be joined, if not
a.CheckSize(x).CheckSize(y);
a.At(x).At(y)
You need only CheckSize to return a reference to array a as usual in upp.
| Quote: |
Also, this is not the only place you need be aware of the problem:
a.Add() = a[0];
|
That could be solved with a construct like
where Dup() should have an obvious function.or
a.Grow(1);
a.Last() = a[0]
for example, or something similar. The point is to avoid potentially dangerous cases.
Of course, all that cost something in term of code lines, (besides the Dup() example...) but nothing in terms of code speed, but avoids many possible caveats.
I *do not* criticize retourning references, that's needed for speed sakes, but I think a generic class like an array should make potentially dangerous constructs impossible.
Ciao
Max
|
|
|
|
| Re: Core chat... [message #12332 is a reply to message #12331] |
Thu, 25 October 2007 14:34 |
|
mirek
Messages: 13975
Registered: November 2005
|
Ultimate Member |
|
|
| mdelfede wrote on Thu, 25 October 2007 08:06 |
| luzr wrote on Thu, 25 October 2007 05:33 |
Consider composition a.At(x).At(y)... It can become tedious.
|
yes, but finding At() kind of bugs can be even more tedious...
|
Well, sometimes finding bugs IS tedious. But this one was the first At related I had to spend more than 10 minutes.
| Quote: |
There you could write :
a.CheckSize(x).CheckSize(y);
a[x][y];
if [][] operators can be joined, if not
a.CheckSize(x).CheckSize(y);
a.At(x).At(y)
You need only CheckSize to return a reference to array a as usual in upp.
|
Hey, think about it a little bit more. To make what you suggest work, CheckSize has to return a reference to contained element. So it behaves exactly the same as At.
| Quote: |
Of course, all that cost something in term of code lines, (besides the Dup() example...) but nothing in terms of code speed, but avoids many possible caveats.
|
Actually, would be a bit slower, as you get the variable address evaluated in the At once. Your version evaluates it twice (and in composition example, access the outer container twice too).
Mirek
|
|
|
|
| Re: Core chat... [message #12334 is a reply to message #12332] |
Thu, 25 October 2007 15:19 |
mdelfede
Messages: 1307
Registered: September 2007
|
Ultimate Contributor |
|
|
| luzr wrote on Thu, 25 October 2007 14:34 |
Well, sometimes finding bugs IS tedious. But this one was the first At related I had to spend more than 10 minutes.
|
That's because you're a guru of your UPP code, in my case that would have taken days, I guess.... I haven't seen it even when you pointed at the source....
| Quote: |
Hey, think about it a little bit more. To make what you suggest work, CheckSize has to return a reference to contained element. So it behaves exactly the same as At.
|
yes, of course, but in my way you can't do very few bad thins with that reference.... you could even write
a.CheckSize(100)[100] = 5;
whith no danger. Of course, you could even force it do do bad things such
a.CheckSize(100)[100] = a[10];
but then, you're forcing things to be buggy 
BTW, I still think that a CheckSize() function should return nothing, to avoid such caveats..... I'd rather
a.CheckSize(100);
a[100] = 5;
| Quote: |
Actually, would be a bit slower, as you get the variable address evaluated in the At once. Your version evaluates it twice (and in composition example, access the outer container twice too).
|
well, that depends of compiler code... usually "modern" compilers take care of avoiding double access when unneeded.
BTW, all that chat becomes question of personal taste... like commenting code. I usually prefere to write some more lines and have less "hidden" bugs possibilities.... as I usually comment about each code line in order to be able to know what I did even 1 year later. Other people like more to write a single-1000-chars line of code with no comment at all, and they can understand it even after 10 years.
I'd find more interesting a chat about reference counted objects.... I think I'll get back my old (and poorly written) array class, just to see the performance differences
It was done mainly to manage DispInterfaces arrays in a COM app managing Autocad, so no great speed requirements, but I guess it could be polished to be of some interest.
Just a last word, I *don't* think to propose you to change from pick_ to refcounted arrays (usually I hate when someone writes to me of changing core parts of my code !), I'm just curious of performance differences and possible caveats of this kind of solution.
Ciao
Max
|
|
|
|
|
|
|
|
|
|
| Re: Core chat... [message #12348 is a reply to message #12346] |
Thu, 25 October 2007 23:26 |
mdelfede
Messages: 1307
Registered: September 2007
|
Ultimate Contributor |
|
|
| luzr wrote on Thu, 25 October 2007 21:33 |
You can try. However, long time ago, such class template was part of U++. But there was no use for it. "pick" is confusing at first, but quite powerful concept.
|
I do find "pick" quite clear. You get the top performance at at the expense of some loss of easy to use stuff.
With deepcopy behaviour, you have the opposite.... large performance loss with the gain of ease to use.
I think using refcount objects is a compromise between both, you have the same easy to use as deepcopy behaviour at the expense of a *small* performance loss in respect to your pick mechanics, that I guess is mostly due to a double indirection accessing elements, when you don't need a deep copy.
In your class :
array<int> a, b;
a.At(1000) = 1;
b = a;
a[10] = 2; <==ERROR
b[10] = 2; <==OK
in my class :
array<int> a, b;
a.At(1000) = 1;
b = a; <== b and a share same memory area
a[10] = 2; <== here an automatic deep copy, ok
b[10] = 2; <== here no deep copy, just array access, ok
in usual deep copy behaviour :
array<int> a, b;
a.At(1000) = 1;
b = a; <== deep copy
a[10] = 2; <==OK
b[10] = 2; <==OK
in 95% of cases, my array behaves exactly as yours, but... in the rest 5%, you DO have to specify WithDeepCopyOption, in mine that's done automatically, with a 'small' performance penalty.
What I'm really curious about is how 'small' that is....
BTW, returning an array from a function, my class don't need a deep copy, as yours, as the origin array is released when function ends, leaving only the result reference to array.
array<int> a;
a = MyFunc(x);
array a gets 2 references to it for a while, just when MyFunc returns a value. Then, temporary from MyFunc get destroyed, leaving a with a single reference. Any subsequent access to a[] don't need a deep copy.
Max
|
|
|
|
Goto Forum:
Current Time: Sat Apr 20 07:42:29 CEST 2024
Total time taken to generate the page: 0.03839 seconds
|
Members
Pages
Help
Register
Login
Home
