On Tue, Oct 12, 2010 at 6:37 PM, James Robinson <jam...@google.com> wrote:
> On Tue, Oct 12, 2010 at 3:46 PM, Maciej Stachowiak <m...@apple.com> wrote: > >> >> On Oct 12, 2010, at 2:37 PM, Darin Fisher wrote: >> >> On Tue, Oct 12, 2010 at 1:20 PM, Maciej Stachowiak <m...@apple.com> wrote: >> >> Hmm, I've found weak pointer abstractions to be very useful. The issue >> with reference counting is that it is "easy" to introduce memory leaks, and >> has been mentioned, it is sometimes nice to have deterministic object >> destruction. >> >> >> We used to have lots of problems with leaking refcounted objects back when >> most refcounting was done manually, but this has become much less common as >> we deployed smart pointers. >> >> >> It is also nice to avoid having to have explicit clear() functions and >> then add checks to every other method to assert that they are not called >> after clear(). >> >> >> Well, if you have a weak pointer you have to check weak pointer validity, >> so you don't save any checks. I don't think explicit close calls are that >> bad. I think it's actually a better pattern when you hold significant >> resources other than just some memory (e.g. VRAM, file handles, sockets, >> limited kernel resources, window server handles...). That way, cleaning up >> your external resources does not become dependent on your ownership strategy >> for the object. >> >> >> In the Chromium code base, we have a helper for weak pointers: >> >> http://src.chromium.org/viewvc/chrome/trunk/src/base/weak_ptr.h?view=markup >> >> We tend to use this in cases in which: >> >> 1) there are many consumers interested in holding a back pointer to some >> shared resource, and >> 2) we'd like the shared resource to die at some predictable time. >> >> Without a utility like this, the alternative is to make the shared >> resource notify each of the consumers about the impending destruction of the >> shared resource. >> >> It is true that WeakPtr<T> adds a null check in front of each method call >> made by the consumers, but that runtime cost is often justified in exchange >> for reduced code complexity (i.e., eliminating the need to notify consumers >> when the shared resource dies). >> >> >> The cost isn't just the null checks. You need either storage for all the >> backpointers, or the smaller storage overhead for a refcounted handle. But >> then almost by definition you have more overhead than refcounting, since you >> have a refcounted object plus more work. >> >> I do think weak pointers have their uses, but only in relatively unusual >> cases (e.g. when you'd have a cycle otherwise). It doesn't sound like a huge >> win in this case. >> > > Another other advantage of weak pointers over refcounted pointers is that > with a combination of OwnPtr<>/WeakPtr<> it that the ownership is explicit > and easy to examine locally. By this I mean if I have: > > class A { > OwnPtr<T> m_t; > }; > > class B { > private: > WeakPtr<T> m_t; > }; > > it's immediately and locally obvious which object is responsible for the > lifetime of the T. With the alternative: > > class A { > RefPtr<T> m_t; > }; > > class B { > RefPtr<T> m_t; > }; > ^^^ Looks like a memory leak too :-) One then hopes for up-to-date documentation about how the memory leak is broken. -Darin > > there's no way to know which class drives the lifetime of the T without > reading through the definitions of both A and B (and any other class that > holds a RefPtr). This makes it much harder to review changes to A or to B > without carefully reading through many mostly-unrelated classes. This is > especially true when > > A concrete example of this is LayerRendererChromium which used to have an > OwnPtr<GraphicsContext3D> member variable, took a > PassOwnPtr<GraphicsContext3D> parameter in its create() factory function and > exposed a GraphicsContext3D* getter. The lifetime of the GraphicsContext3D > associated with the compositor was crystal clear just by examining > LayerRendererChromium.h - it took exclusive ownership of the context at > creation time and retained exclusive ownership for the lifetime of the > LayerRendererChromium. Now that GraphicsContext3D is ref counted, it's not > nearly as obvious whether the LayerRendererChromium is supposed to be the > exclusive owner of the underlying context or whether callers might extend > its lifetime (especially since the GraphicsContext3D* getter now allows > callers to grab their own references). Ambiguous and non-local ownership > semantics makes code harder to review and reason about. > > - James > > >> Regards, >> Maciej >> >> >
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
