Thread: Smart pointer class

  1. #16
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    Quote Originally Posted by Elysia View Post
    Then it's impossible to send the class through a const reference since it can only call const functions.
    You're confusing const pointers with pointers-to-const. It's the same for smart pointers as for raw pointers: there's a difference between "CFoo const *" and "CFoo * const", and there's a difference between "smart_pointer<CFoo const>" and "const smart_pointer<CFoo>".

    Shrug. You never know.
    Look at it this way: on a 32-bit system, your class is currently probably 7*sizeof(void*) = 28 bytes large. To have the refcount rise to 2^31 (the overflow point), you'd need 2^31 smart pointers occupying 2^31 * 28 bytes, that's 2^32 * 7 bytes, and that's more than a 32-bit CPU is capable of addressing. So yes, you do know.
    In the 64-bit case, it might work in theory. Your class is 56 bytes large (that's huge, by the way - another reason to get rid of all that runtime stuff; a refcounting smart pointer should be 16 bytes large), which is 2^33 * 7 bytes. This does fit into the 2^40 bytes an Athlon64 can physically address.
    However, the stack is still limited way below that even in 64-bit systems. And these smart pointers are intended for stack usage - you're actually trying to block heap allocation.

    I'm not going to have 1000 classes that does the same thing. Splitting the class would be difficult or very tiresome as I'd have to rewrite a lot of code (since calling Release calls InternalDelete which calls InternalDetach, which Locks, deletes memory and removes pointer from map).
    Bzuh? If you followed my advice, you'd be using Boost's smart pointers and not writing a single line of code.

    Ah, unless I make virtual functions and override those with non-locking and non-pointer looking-uping.
    No. Bad. You can use base classes for implementation sharing, but no virtuals. Virtuals are for runtime decisions. You should get rid of those.

    Not nonsense in my view. Try assigning a lot of pointers to the class a lot of the time and you'll see a performance hit.
    1) You shouldn't be assigning a lot of pointers to the class a lot of the time. If you do, you've got a logical problem in your code. You're busy juggling pointers when you should be doing work.
    2) What the hell are you doing that takes so long? Oh, I know. You're looking stuff up in your various global maps.

    No, that was a bug. Initially, I just wanted to delete m_pInfo, but later realized that I had to delete m_pInfo->pdwRefCount, so I made a new function to handle that and called it but forgot to remove delete m_pInfo.
    My point exactly.

    That's really stupid. I don't see why I can't use realloc, though. Even though you disagree, it can work, as I have used it in the past and is has worked fine. I don't know the implications of doing it, however.
    It's a necessary consequence of encapsulation. You don't know the internals of a class. You can't know if it's safe to copy it to someplace else. The class might be holding a pointer to its own members. (Objects using the small buffer optimization often do that.) You have to call the class's copy constructor to create an instance somewhere else. And because realloc is a C function, it doesn't know that. It doesn't even know that such things as copy constructors exist.
    For that matter, you don't know if operator new is even implemented in terms of malloc. An implementation is free not to do so. A programmer is free to override the implementation's default implementation. Calling realloc assumes that malloc allocated the block in the first place.
    It's pretty much a time bomb waiting to blow up in your face.

    It simply doesn't work that way. Instead of looking up twice (I agree that could be done with arguments), but caching wouldn't work that way.
    What caching?

    Now that's something that should make it into the standard: no weird automatic conversions. If a function takes a bool, then it takes a bool, end of story. Not some int or any other kind of data type that the compiler can think of making up.
    No chance. The standard won't break backwards compatibility, and there's actually code out there that relies on these conversions.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  2. #17
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by CornedBee View Post
    You're confusing const pointers with pointers-to-const. It's the same for smart pointers as for raw pointers: there's a difference between "CFoo const *" and "CFoo * const", and there's a difference between "smart_pointer<CFoo const>" and "const smart_pointer<CFoo>".
    Code:
    void foo(const pp<int>& p)
    {
    	int n = *p; // No workie because operator * and operator T* isn't const.
    }
    int main()
    {
    	ppnew<int> p(50);
    	foo(p);
    	return 0;
    }
    Look at it this way: on a 32-bit system, your class is currently probably 7*sizeof(void*) = 28 bytes large. To have the refcount rise to 2^31 (the overflow point), you'd need 2^31 smart pointers occupying 2^31 * 28 bytes, that's 2^32 * 7 bytes, and that's more than a 32-bit CPU is capable of addressing. So yes, you do know.
    In the 64-bit case, it might work in theory. Your class is 56 bytes large (that's huge, by the way - another reason to get rid of all that runtime stuff; a refcounting smart pointer should be 16 bytes large), which is 2^33 * 7 bytes. This does fit into the 2^40 bytes an Athlon64 can physically address.
    However, the stack is still limited way below that even in 64-bit systems. And these smart pointers are intended for stack usage - you're actually trying to block heap allocation.
    Ah well, but as the saying goes: What isn't broken, don't fix. It can remain unsigned and the critical section will take care of the locking, so it shouldn't be a problem.

    Bzuh? If you followed my advice, you'd be using Boost's smart pointers and not writing a single line of code.
    No chance

    No. Bad. You can use base classes for implementation sharing, but no virtuals. Virtuals are for runtime decisions. You should get rid of those.
    No. Good. Polymorphism is good. If I don't use Virtuals, I'll have to tear the class apart and re-write already existing code when creating new ones. THAT is bad.

    1) You shouldn't be assigning a lot of pointers to the class a lot of the time. If you do, you've got a logical problem in your code. You're busy juggling pointers when you should be doing work.
    2) What the hell are you doing that takes so long? Oh, I know. You're looking stuff up in your various global maps.
    And that's why I added time critical - it doesn't mess with the maps!

    My point exactly.
    Point was that I forgot to remove a line - I never forgot what function did what! And it's better than copying around all the code all the time.

    It's a necessary consequence of encapsulation. You don't know the internals of a class. You can't know if it's safe to copy it to someplace else. The class might be holding a pointer to its own members. (Objects using the small buffer optimization often do that.) You have to call the class's copy constructor to create an instance somewhere else. And because realloc is a C function, it doesn't know that. It doesn't even know that such things as copy constructors exist.
    For that matter, you don't know if operator new is even implemented in terms of malloc. An implementation is free not to do so. A programmer is free to override the implementation's default implementation. Calling realloc assumes that malloc allocated the block in the first place.
    It's pretty much a time bomb waiting to blow up in your face.
    I just learned it doesn't call copy constructors... Yes, I know what that means. I never thought of it. Look like we're going to have to do a some good ol' memory copying to new memory, which is expensive and stupid when you can simply resize an existing memory block. Bah.
    BTW, there's no C++ safe copying method? memcpy and CopyMemory are obviously bad.

    What caching?
    Tut-tut. m_PInfoTemp caches the lookup for pointer p so it doesn't have to look up twice or more if it's going to lookup p again.

    No chance. The standard won't break backwards compatibility, and there's actually code out there that relies on these conversions.
    Doesn't necessarily mean make it default behaviour. Add something along the lines of explicit or mutable or another keyword for it. Safe for everyone.
    Last edited by Elysia; 10-31-2007 at 04:11 PM.

  3. #18
    Officially An Architect brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,396
    Quote Originally Posted by Elysia View Post
    Doesn't necessarily mean make it default behaviour. Add something along the lines of explicit or mutable or another keyword for it. Safe for everyone.
    My suggestion of wrapping up types inside objects with explicit constructors would accomplish exactly what you are talking about.

  4. #19
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    It doesn't work like it should:
    Code:
    pp<CRect> foo()
    {
    	return NULL; // error C2664: 'CMemoryManager<T>::CMemoryManager(const CMemoryManager<T> &)' : cannot convert parameter 1 from 'int' to 'const CMemoryManager<T> &'
    
    }
    int main()
    {
    	foo();
    	return 0;
    }
    
    explicit CMemoryManager(int/* nNull*/);
    Remove explicit and it works. But it also works to return a bool >_< Which I why I used PPNULL in the first place.

  5. #20
    The larch
    Join Date
    May 2006
    Posts
    3,573
    Quote Originally Posted by Elysia View Post
    I don't get what you mean?
    I mean I'm quite perplexed by the resize methods (seeing how each allocates):
    Code:
    	void Resize(size_t dwNewSize) throw(...) 
    	{
    		//...
    		const void* pTemp = realloc(p, dwNewSize); 
    		//...
    	}
    	void ResizeElements(DWORD dwNewSize) throw(...) 
    	{
    		//...
    		const void* pTemp = realloc(p, sizeof(*p) * dwNewSize); 
    		//...
    	}
    Even if realloc would work here, this seems like an accident waiting to happen...

    But if you need to resize dynamic arrays, wouldn't it be wiser to use a std::vector in the first place?

    It helps catch bugs at design time and not create weird bugs in your code when the compiler calls the entirely wrong function. I was frustrated before when I patched
    Code:
    myclass = 50;
    ...to the bool constructor that took a param that said if you wanted to create a new object when creating the class or not!
    It should have failed by then (code has since been revised).
    This is a case that making the constructor explicit should catch.

    However, in your code I see you using the explicit keyword with a constructor that takes two parameters. I can't see how that constructor could be called implicitly otherwise.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  6. #21
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by anon View Post
    I mean I'm quite perplexed by the resize methods (seeing how each allocates):
    Code:
    	void Resize(size_t dwNewSize) throw(...) 
    	{
    		//...
    		const void* pTemp = realloc(p, dwNewSize); 
    		//...
    	}
    	void ResizeElements(DWORD dwNewSize) throw(...) 
    	{
    		//...
    		const void* pTemp = realloc(p, sizeof(*p) * dwNewSize); 
    		//...
    	}
    Even if realloc would work here, this seems like an accident waiting to happen...

    But if you need to resize dynamic arrays, wouldn't it be wiser to use a std::vector in the first place?
    Yes, they should be removed (or maybe renamed). Or maybe I should rename them to ResizeC? Err, but C doesn't support classes. Hmmm. Maybe I should deprecate arrays and demand the use of vectors instead... That might be a good idea when I think about it.

    This is a case that making the constructor explicit should catch.
    Doesn't work. See above ^

    However, in your code I see you using the explicit keyword with a constructor that takes two parameters. I can't see how that constructor could be called implicitly otherwise.
    Better safe than sorry.

  7. #22
    The larch
    Join Date
    May 2006
    Posts
    3,573
    Look like we're going to have to do a some good ol' memory copying to new memory, which is expensive and stupid when you can simply resize an existing memory block. Bah.
    You seem to think that realloc magically resizes an existing memory block. It can only do so because it overallocates in the first place. And if there simply isn't enough room to expand it will move everything to a new place.
    Code:
    #include <iostream>
    #include <cstdlib>
    
    int main()
    {
        int size = 2;
        int* p = (int*)malloc(sizeof (int) * size);
        std::cout << p << '\n';
        for (int i = 0; i != 10; ++i) {
            size += 128;
            p = (int*)realloc(p, size* sizeof (int));
            std::cout << p << '\n';
        }
        free(p);
    }
    Output:
    0x3d37e8
    0x3d3850
    0x3d3850
    0x3d3850
    0x3d3e60
    0x3d3e60
    0x3d3e60
    0x3d3e60
    0x3d3e60
    0x3d4e70
    0x3d4e70
    As you can see the memory address is changing (everything gets moved at these places). Actually it is not different how std::vector resizes itself (by overallocating), except that std::vector works properly with user-defined objects.

    BTW, there's no C++ safe copying method? memcpy and CopyMemory are obviously bad.
    Yes, these are bad. A good way (performance-wise) is to overallocate like std::vector does (which makes me ask, why a MemoryManager or SmartPointer should be managing dynamic arrays).
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  8. #23
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by anon View Post
    ...(which makes me ask, why a MemoryManager or SmartPointer should be managing dynamic arrays).
    Because the app should not have access to the pointer and should in NO circuimstance delete or re-allocate memory in that memory because it's owned by the CLASS and NOT the function.

  9. #24
    The larch
    Join Date
    May 2006
    Posts
    3,573
    Quote Originally Posted by Elysia View Post
    Because the app should not have access to the pointer and should in NO circuimstance delete or re-allocate memory in that memory because it's owned by the CLASS and NOT the function.
    But that's the case with normal usage of std::vector. Sure you can obtain the pointer (address of first element) and try to delete it but you'd need to be rather dumb to do that. I guess functions that need to re-allocate memory, should take a vector as an argument, not a raw pointer.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  10. #25
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    The only problem I can see right now are buffers. Arrays full of chars or BYTES. A vector won't suit for that purpose and I need to resize the darn thing, but I guess the task of a buffer should fall on a different class.

  11. #26
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    >> Arrays full of chars or BYTES. A vector won't suit for that purpose and I need to resize the darn thing, but I guess the task of a buffer should fall on a different class.

    Why won't a vector be suitable for that?

  12. #27
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Would it? I don't see vector as the sort of class that holds buffers.

  13. #28
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    I would imagine it would be great for that, but maybe our understandings of "buffer" are different.

  14. #29
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Quote Originally Posted by Elysia View Post
    The only problem I can see right now are buffers. Arrays full of chars or BYTES. A vector won't suit for that purpose and I need to resize the darn thing, but I guess the task of a buffer should fall on a different class.
    A vector<unsigned char> is perfect for that...

    What's wrong with vector::reserve(), vector::resize() or just letting the vector grow on its own?

  15. #30
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    You really should read this book... It answers virtually all the questions that we're talking about.

    Chapter 76. Use vector by default. Otherwise, choose an appropriate container.

    If you doubt this advice, ask yourself if you really have a compelling reason not to use the only standard container that guarantees all of the following properties. vector alone is:

    • Guaranteed to have the lowest space overhead of any container (zero bytes per object).
    • Guaranteed to have the fastest access speed to contained elements of any container.
    • Guaranteed to have inherent locality of reference, meaning that objects near each other in the container are guaranteed to be near each other in memory, which is not guaranteed by any other standard container.
    • Guaranteed to be layout-compatible with C, unlike any other standard container. (See Items 77 and Chapter 78)
    • Guaranteed to have the most flexible iterators (random access iterators) of any container.
    • Almost certain to have the fastest iterators (pointers, or classes with comparable performance that often compile away to the same speed as pointers when not in debug mode), faster than those of all other containers.
    Chapter 77. Use vector and string instead of arrays

    Chapter 78. Use vector (and string::c_str) to exchange data with non-C++ APIs

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Message class ** Need help befor 12am tonight**
    By TransformedBG in forum C++ Programming
    Replies: 1
    Last Post: 11-29-2006, 11:03 PM
  2. Direct3D problem
    By cboard_member in forum Game Programming
    Replies: 10
    Last Post: 04-09-2006, 03:36 AM
  3. My Window Class
    By Epo in forum Game Programming
    Replies: 2
    Last Post: 07-10-2005, 02:33 PM
  4. base class pointer problems
    By ... in forum C++ Programming
    Replies: 3
    Last Post: 11-16-2003, 11:27 PM
  5. Warnings, warnings, warnings?
    By spentdome in forum C Programming
    Replies: 25
    Last Post: 05-27-2002, 06:49 PM